New subject: [Bug 36731] Runes of Magic 'ClientUpdater.exe' crashes after a number of update cycles (mshtml environment setup contains stack buffer overflow)

14 Jun 2014


      https://bugs.winehq.org/show_bug.cgi?id=36731
Bug ID: 36731
           Summary: Runes of Magic 'ClientUpdater.exe' crashes after a
                    number of update cycles (mshtml environment setup
                    contains stack buffer overflow)
           Product: Wine
           Version: 1.7.20
          Hardware: x86
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: mshtml
          Assignee: wine-bugs@winehq.org
          Reporter: focht@gmx.net
Hello folks,
found during investigation of 'Runes of Magic' client updater.
There is a crash after a number of update cycles.
Unfortunately it's not easily traceable as it requires large downloads and many
client restarts (= hours) to reach the crash point.
I started the updater with a few debug channels (= reduced noise) which still
allowed me to do post-mortem analysis.
The launcher restarts itself after each update cycle.
--- snip ---
$ pwd
/home/focht/.wine/drive_c/Program Files/Runes of Magic
$ WINEDEBUG=+tid,+seh,+loaddll,+process,+mshtml wine ./launcher.exe
...
<hours, multiple updater restarts>
...
004f:trace:loaddll:load_builtin_dll Loaded L"C:\windows\system32\mshtml.dll"
at 0x7c090000: builtin
004f:trace:mshtml:DllGetClassObject (CLSID_HTMLDocument
{00000001-0000-0000-c000-000000000046} 0x3392a8)
004f:trace:mshtml:ClassFactory_AddRef (0x1c2c80) ref = 1
004f:trace:mshtml:HTMLDocument_Create ((nil) IID_IUnknown 0x1c19d4)
004f:trace:mshtml:load_gecko ()
004f:trace:mshtml:check_version "Wine Gecko 2.24"
004f:trace:mshtml:load_xul
(L"C:\windows\system32\gecko\2.24\wine_gecko\\xul.dll")
004f:trace:seh:raise_exception code=c0000005 flags=0 addr=0x7c130001
ip=7c130001 tid=004f
004f:trace:seh:raise_exception  info[0]=00000001
004f:trace:seh:raise_exception  info[1]=8d43ade4
004f:trace:seh:raise_exception  eax=00000001 ebx=006b0063 ecx=003389a0
edx=7bcda204 esi=00339330 edi=001c2de8
004f:trace:seh:raise_exception  ebp=005c0070 esp=00338c00 cs=0023 ds=002b
es=002b fs=0063 gs=006b flags=00010212
004f:trace:seh:call_stack_handlers calling handler at 0x4a97b0 code=c0000005
flags=0
004f:trace:seh:call_stack_handlers handler at 0x4a97b0 returned 1
004f:trace:seh:call_stack_handlers calling handler at 0x7bc9ecf7 code=c0000005
flags=0
wine: Unhandled page fault on write access to 0x8d43ade4 at address 0x7c130001
(thread 004f), starting debugger...
--- snip ---
I looked at the crash site and noticed it being in the middle of opcode
sequence.
--- snip ---
7C130000    45              INC EBP
7C130001    0889 4424108D   OR BYTE PTR DS:[ECX+8D102444],CL
7C130007    8313 B5         ADC DWORD PTR DS:[EBX],-4B
7C13000A    FA              CLI
--- snip ---
Decoded with proper opcode start addresses:
--- snip ---
7C12FFFF    8B45 08         MOV EAX,DWORD PTR SS:[EBP+8]
7C130002    894424 10       MOV DWORD PTR SS:[ESP+10],EAX
7C130006    8D83 13B5FAFF   LEA EAX,[EBX+FFFAB513]
--- snip ---
Partial stack dump with 'esp' = 0x00338c00 from exception context:
--- snip ---
...
00338BA8   C0000005  
00338BAC   00000000
00338BB0   00000000
00338BB4   7C130001 
00338BB8   00000002  
00338BBC   00000001  
00338BC0   8D43ADE4  äC
00338BC4   00650074  t e
00338BC8   0033006D  m 3
00338BCC   005C0032  2 \
00338BD0   00650067  g e
00338BD4   006B0063  c k
00338BD8   005C006F  o \
00338BDC   002E0032  2 .
00338BE0   00340032  2 4
00338BE4   0077005C  \ w
00338BE8   006E0069  i n
00338BEC   00000005  
00338BF0   00650067  g e
00338BF4   006B0063  c k
00338BF8   005C006F  o \
00338BFC   7C130000  
00338C00   00338F74  ; UNICODE "C:\windows\system32\gecko\2.24\wine_gecko"
00338C04   7C1FF480  
00338C08   7C19E234  ; ASCII "load_xul"
00338C0C   7C19CF48  ; ASCII "(%s)"
00338C10   7BCEC8C1  ; ASCII
"L"C:\windows\system32\gecko\2.24\wine_gecko\\xul.dll""
00338C14   7C19D4E8  ; ASCII "Wine Gecko 2.24"
00338C18   7C19E250  ; ASCII "check_version"
00338C1C   7C19D4C0  ; ASCII "%s"
00338C20   7BCEC8AF  ; ASCII ""Wine Gecko 2.24""
...
--- snip ---
Yes, looks pretty much like a stack buffer overflow.
A string buffer overwrote 'ebp', 'ebx' values (register save on stack for
'__x86_get_pc_thunk_bx') and parts of the return address.
The NULL terminator cancelled out the lower 16 bits of the return address.
The culprit: 'load_xul' -> 'set_environment'
Source:
http://source.winehq.org/git/wine.git/blob/0be56d27d2d4b22367313fa4c6f1e6586...
--- snip ---
439 static void set_environment(LPCWSTR gre_path)
440 {
441     WCHAR path_env[MAX_PATH], buf[20];
442     int len, debug_level = 0;
443
444     static const WCHAR pathW[] = {'P','A','T','H',0};
445     static const WCHAR warnW[] = {'w','a','r','n',0};
446     static const WCHAR xpcom_debug_breakW[] =
447    
{'X','P','C','O','M','_','D','E','B','U','G','_','B','R','E','A','K',0};
448     static const WCHAR nspr_log_modulesW[] =
449     {'N','S','P','R','_','L','O','G','_','M','O','D','U','L','E','S',0};
450     static const WCHAR debug_formatW[] = {'a','l','l',':','%','d',0};
451
452     /* We have to modify PATH as XPCOM loads other DLLs from this
directory. */
453     GetEnvironmentVariableW(pathW, path_env,
sizeof(path_env)/sizeof(WCHAR));
454     len = strlenW(path_env);
455     path_env[len++] = ';';
456     strcpyW(path_env+len, gre_path);
457     SetEnvironmentVariableW(pathW, path_env);
458
459     SetEnvironmentVariableW(xpcom_debug_breakW, warnW);
460
461     if(TRACE_ON(gecko))
462         debug_level = 5;
463     else if(WARN_ON(gecko))
464         debug_level = 3;
465     else if(ERR_ON(gecko))
466         debug_level = 2;
467
468     sprintfW(buf, debug_formatW, debug_level);
469     SetEnvironmentVariableW(nspr_log_modulesW, buf);
470 }
--- snip ---
'path_env' must have overflowed ... but how?
I used a JIT debugger to examine the process environment block at the time of
the crash since 'GetEnvironmentVariableW' reads from
'NtCurrentTeb()->Peb->ProcessParameters->Environment'.
--- snip ----
Address   UNICODE dump
...
00231EC0  m32\cmd.exe.PATH
00231EE0  =C:\windows\syst
00231F00  em32;C:\windows;
00231F20  C:\windows\syste
00231F40  m32\wbem;C:\wind
00231F60  ows\system32\gec
00231F80  ko\2.24\wine_gec
00231FA0  ko;C:\windows\s
00231FC0  ystem32\gecko\2.
00231FE0  24\wine_gecko;C
00232000  :\windows\system
00232020  32\gecko\2.24\wi
00232040  ne_gecko;C:\win
00232060  dows\system32\ge
00232080  cko\2.24\wine_ge
002320A0  cko;C:\windows\
002320C0  system32\gecko\2
002320E0  .24\wine_gecko.
00232100  TEMP=C:\users\fo
00232120  cht\Temp.TMP=C:\
00232140  users\focht\Temp
00232160  .windir=C:\windo
00232180  ws.ALLUSERSPROFI
002321A0  LE=C:\users\Publ
002321C0  ic.APPDATA=C:\us
...
--- snip ---
At the time 'gre_path' path was appended, the string from 'PATH' environment
variable had already grown near 'MAX_PATH' (260 characters) buffer limit.
'PATH' is of course not limited to 'MAX_PATH' since it contains a list of
paths.
A better option would be to query with 'GetEnvironmentVariableW( value, NULL,
0)' first and allocate the needed buffer from heap, including length for
'gre_path'.
Even with these things corrected there is still a general problem: at one point
it will overflow/being blocked from appending to 'PATH'.
Each newly created updater process inherits the process environment from parent
(client updater restarts itself each time). 
A more sophisticated thing to do would be to search for existing value and not
append if already present.
Wine Mono 'mscoree' component has a similar potential stack buffer overflow:
http://source.winehq.org/git/wine.git/blob/8cdcf470016f0655dfc8810f9d4d2f2d7...
Regards
-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.

[Bug 36731] New: Runes of Magic 'ClientUpdater.exe' crashes after a number of update cycles (mshtml environment setup contains stack buffer overflow)