On Monday, September 17, 2012 6:50:08 PM UTC-4, Dominique Pelle wrote:
> Raymond Ko <[email protected]> wrote:
>
>
>
> > Hello all,
>
> >
>
> > After compiling VIM with Visual Studio 2012,
>
> > apparently a buffer overflow was detected
>
> > and caused VIM to crash.
>
> ...snip...
>
> > mb_unescape() seems to meant for only
>
> > decoding individual characters, and stores
>
> > its results inside a static local array buf, which
>
> > is only meant to be MB_MAXBYTES + 1 big
>
> > (22 bytes).
>
> ..snip...
>
>
>
> I don't understand the function mb_unescape(),
>
> but what I can see that it can overflow buf[ ] by
>
> 1 bytes:
>
>
>
> mbytes.c:
>
>
>
> 3796 static char_u buf[MB_MAXBYTES + 1];
>
> 3802 for (n = 0; str[n] != NUL && m <= MB_MAXBYTES; ++n)
>
> ...
>
> 3828 else
>
> 3829 buf[m++] = str[n];
>
> 3830 buf[m] = NUL; <--- where it crashes
>
> ....
>
> 3838 }
>
>
>
> Notice that we may write 2 bytes within the for loop
>
> (at lines 3829 and line 3830 for example). So the
>
> check at line 3802 (m <= MB_MAXBYTES) is not
>
> enough to prevent overflows. If m is MB_MAXBYTES
>
> at line 3802, we enter the for loop, which may writes
>
> at buf[MB_MAXBYTES] at line 3829 then write at
>
> buf[MB_MAXBYTES+1] at line 3830 (overflow
>
> by 1 byte!)
>
>
>
> This overflow can be avoided by adding one more
>
> byte to buf:
>
>
>
> 3796 static char_u buf[MB_MAXBYTES + 2];
>
>
>
> But since I don't understand the function, I'm not
>
> sure whether that's the correct fix.
>
>
>
> Can you explain how to reproduce the bug?
>
>
>
> Regards
>
> -- Dominique
Here are the steps:
1. Get Visual Studio 2012. The Express edition should be using the same
compiler for the backend but I'm not sure.
2. Compile VIM. Not that the default Make_mvc.mak file doesn't work anymore
because the Windows SDK is no longer referenced by the vcvarsall.bat. Instead I
had to follow the instructions here: http://kalekold.net/index.php?post=23 .
Changes are noted in Make_mvc.vc11.diff (/Ox -> /Od for debug purposes). I am
using the following batch file to simplify the compile process (_msvc.bat.diff).
3. Install UltiSnips (http://www.vim.org/scripts/script.php?script_id=2715)
4. Use one of the snippets and it should crash (like the snippet "once<TAB>").
---
Reading your analysis, I realized that I had mentally parsed '&&' in the for
loop as '||' for some reason, and your reasoning should be correct. I guess I
was confused as to why a function that is designed to escape a multi-byte
character (from the description in the comments) would be passed a potentially
multi-byte string.
Changing it to +2 does prevent a crash, although like you said you don't know
what effect it would have.
Another screenshot:
http://i.imgur.com/jFz5b.png
According to the assembly, it seems to be checking to see that at most 0x16
bytes (MB_MAXBYTES + 1) is written, and if the invariant fails, it triggers the
"report range check failure" function, causing my crash.
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
diff -r 5884d056e1d1 src/Make_mvc.mak
--- a/src/Make_mvc.mak Wed Sep 12 20:21:43 2012 +0200
+++ b/src/Make_mvc.mak Mon Sep 17 19:33:50 2012 -0400
@@ -227,7 +227,7 @@
# Get all sorts of useful, standard macros from the Platform SDK.
-!include <Win32.mak>
+!include $(SDK_INCLUDE_DIR)\Win32.mak
# Flag to turn on Win64 compatibility warnings for VC7.x and VC8.
WP64CHECK = /Wp64
@@ -395,6 +395,9 @@
!if "$(_NMAKE_VER)" == "10.00.30319.01"
MSVCVER = 10.0
!endif
+!if "$(_NMAKE_VER)" == "11.00.50727.1"
+MSVCVER = 11.0
+!endif
!endif
# Abort bulding VIM if version of VC is unrecognised.
@@ -409,7 +412,7 @@
!endif
# Convert processor ID to MVC-compatible number
-!if ("$(MSVCVER)" != "8.0") && ("$(MSVCVER)" != "9.0") && ("$(MSVCVER)" != "10.0")
+!if ("$(MSVCVER)" != "8.0") && ("$(MSVCVER)" != "9.0") && ("$(MSVCVER)" != "10.0") && ("$(MSVCVER)" != "11.0")
!if "$(CPUNR)" == "i386"
CPUARG = /G3
!elseif "$(CPUNR)" == "i486"
@@ -440,10 +443,10 @@
!elseif "$(OPTIMIZE)" == "SPEED"
OPTFLAG = /O2
!else # MAXSPEED
-OPTFLAG = /Ox
+OPTFLAG = /Od
!endif
-!if ("$(MSVCVER)" == "8.0") || ("$(MSVCVER)" == "9.0") || ("$(MSVCVER)" == "10.0")
+!if ("$(MSVCVER)" == "8.0") || ("$(MSVCVER)" == "9.0") || ("$(MSVCVER)" == "10.0") || ("$(MSVCVER)" == "11.0")
# Use link time code generation if not worried about size
!if "$(OPTIMIZE)" != "SPACE"
OPTFLAG = $(OPTFLAG) /GL
@@ -908,7 +911,7 @@
# Report link time code generation progress if used.
!ifdef NODEBUG
-!if ("$(MSVCVER)" == "8.0") || ("$(MSVCVER)" == "9.0") || ("$(MSVCVER)" == "10.0")
+!if ("$(MSVCVER)" == "8.0") || ("$(MSVCVER)" == "9.0") || ("$(MSVCVER)" == "10.0") || ("$(MSVCVER)" == "11.0")
!if "$(OPTIMIZE)" != "SPACE"
LINKARGS1 = $(LINKARGS1) /LTCG:STATUS
!endif
diff -r 5884d056e1d1 src/_msvc.bat
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/src/_msvc.bat Mon Sep 17 20:22:11 2012 -0400
@@ -0,0 +1,24 @@
+@ECHO OFF
+PUSHD %~dp0
+
+SET SDK_INCLUDE_DIR=C:\Program Files\Microsoft SDKs\Windows\v7.1\Include
+SET VC_DIR="C:\Program Files (x86)\Microsoft Visual Studio 11.0\VC
+
+CALL %VC_DIR%\vcvarsall.bat" x86_amd64
+
+@ECHO ON
+
+SET VIM_CONFIG_OPTIONS=^
+FEATURES=HUGE GUI=yes OLE=yes MBYTE=yes ^
+IME=yes DYNAMIC_IME=yes GIME=yes ^
+PYTHON=C:\Python27 DYNAMIC_PYTHON=yes PYTHON_VER=27 ^
+SNIFF=yes CSCOPE=yes ICONV=yes GETTEXT=yes POSTSCRIPT=yes ^
+NETBEANS=yes ^
+DEBUG=lines OPTIMIZE=MAXSPEED CPU=AMD64 CPUNR=pentium4 WINVER=0x0500
+
+nmake -f Make_mvc.mak %VIM_CONFIG_OPTIONS% clean
+nmake -f Make_mvc.mak %VIM_CONFIG_OPTIONS%
+
+C:\Windows\System32\xcopy.exe /y /f gvim.exe "C:\Program Files (x86)\Vim\vim73\"
+C:\Windows\System32\xcopy.exe /y /f vimrun.exe "C:\Program Files (x86)\Vim\vim73\"
+C:\Windows\System32\xcopy.exe /y /f xxd\xxd.exe "C:\Program Files (x86)\Vim\vim73\"
