[Bug 36731] New: Runes of Magic 'ClientUpdater.exe' crashes after a number of update cycles (mshtml environment setup contains stack buffer overflow)
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(a)winehq.org Reporter: focht(a)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.
https://bugs.winehq.org/show_bug.cgi?id=36731 Anastasius Focht <focht(a)gmx.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download URL| |http://www.gamestar.de/_mis | |c/downloads/getDLUrl.cfm?dl | |Id=52301 -- 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.
http://bugs.winehq.org/show_bug.cgi?id=36731 Jacek Caban <jacek(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jacek(a)codeweavers.com --- Comment #1 from Jacek Caban <jacek(a)codeweavers.com> --- Patch sent: http://source.winehq.org/patches/data/105709 Thanks for the report. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=36731 --- Comment #2 from Austin English <austinenglish(a)gmail.com> --- (In reply to Jacek Caban from comment #1)
Patch sent: http://source.winehq.org/patches/data/105709
Thanks for the report.
https://source.winehq.org/git/wine.git/commitdiff/1bd0f4f065da6db368f57c63d9... -- 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.
https://bugs.winehq.org/show_bug.cgi?id=36731 Anastasius Focht <focht(a)gmx.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |1bd0f4f065da6db368f57c63d9b | |d89e98e1677b3 Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #3 from Anastasius Focht <focht(a)gmx.net> --- Hello folks, this is fixed by commit http://source.winehq.org/git/wine.git/commitdiff/1bd0f4f065da6db368f57c63d9b... Thanks Jacek 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.
https://bugs.winehq.org/show_bug.cgi?id=36731 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #4 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 1.7.23. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=36731 Anastasius Focht <focht(a)gmx.net> changed: What |Removed |Added ---------------------------------------------------------------------------- URL|http://www.gamestar.de/_mis | |c/downloads/getDLUrl.cfm?dl | |Id=52301 | Keywords|download | -- 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.
participants (2)
-
wine-bugs＠winehq.org -
WineHQ Bugzilla