[Bug 36731] New: Runes of Magic 'ClientUpdater.exe' crashes after a number of update cycles (mshtml environment setup contains stack buffer overflow)
wine-bugs at winehq.org
wine-bugs at winehq.org
Sat Jun 14 11:06:42 CDT 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 at winehq.org
Reporter: focht at 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/0be56d27d2d4b22367313fa4c6f1e65865c90438:/dlls/mshtml/nsembed.c#l439
--- 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/8cdcf470016f0655dfc8810f9d4d2f2d7a7d1b54:/dlls/mscoree/metahost.c#l117
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.
More information about the wine-bugs
mailing list