February 2008 - Wine-Devel - list.winehq.org

Re: shell32: Fix security issue in SHELL_execute.

by Alexandre Julliard

Robert Shearman <rob(a)codeweavers.com> writes: > execfunc should not be called on files which aren't supposed to be > executable. This doesn't work: ../../../tools/runtest -q -P wine -M shell32.dll -T ../../.. -p shell32_test.exe.so shlexec.c && touch shlexec.ok shlexec.c:971: Test failed: ShellExecuteEx(null, "C:\windows\temp\\test_shortcut_exe.lnk", null, null) failed: rc=31 err=2 shlexec.c:974: Test failed: argvA3 expected 'Lnk', got '(null)' shlexec.c:1010: Test failed: ShellExecuteEx(null, "C:\windows\temp\\test_shortcut_exe.lnk", ""C:\windows\temp\\nonexistent.shlexec"", null) failed: rc=31 err=2 shlexec.c:1010: Test failed: ShellExecuteEx(null, "C:\windows\temp\\test_shortcut_exe.lnk", ""C:\windows\temp\\nonexistent.noassoc"", null) failed: rc=31 err=2 shlexec.c:1010: Test failed: ShellExecuteEx(null, "C:\windows\temp\\test_shortcut_exe.lnk", ""C:\windows\temp\\test file.shlexec"", null) failed: rc=31 err=2 shlexec.c:1010: Test failed: ShellExecuteEx(null, "C:\windows\temp\\test_shortcut_exe.lnk", ""C:\windows\temp\/%nasty% $file.shlexec"", null) failed: rc=31 err=2 shlexec.c:1010: Test failed: ShellExecuteEx(null, "C:\windows\temp\\test_shortcut_exe.lnk", ""C:\windows\temp\\test file.noassoc"", null) failed: rc=31 err=2 shlexec.c:1062: Test failed: ShellExecuteEx(null, "Z:\home\julliard\wine\wine\dlls\shell32\tests\shell32_test.exe.so", "shlexec "C:\windows\temp\wt51f.tmp" Exec", null) returned 31 shlexec.c:1064: Test failed: argvA3 expected 'Exec', got '(null)' shlexec.c:1071: Test succeeded inside todo block: ShellExecute(null, "C:\windows\temp\\test file.noassoc", "shlexec "C:\windows\temp\wt51f.tmp" Exec", null) succeeded: rc=31 shlexec.c:1093: Test failed: ShellExecuteEx(null, "Z:\home\julliard\wine\wine\dlls\shell32\tests\shell32_test.exe.so", "shlexec "C:\windows\temp\wt51f.tmp" abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxy", null) returned 31 shlexec.c:1095: Test failed: argvA3 expected 'abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxy', got '(null)' shlexec.c:1102: Test succeeded inside todo block: ShellExecute(null, "C:\windows\temp\\test file.noassoc", "shlexec "C:\windows\temp\wt51f.tmp" abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxy", null) succeeded: rc=31 make: *** [shlexec.ok] Error 13 -- Alexandre Julliard julliard(a)winehq.org

Feb. 7, 2008

Re: msi 2: Handle carriage returns in MsiDatabaseImport

by Alexandre Julliard

"James Hawkins" <truiken(a)gmail.com> writes: > Hi, > > Fixes bug 11479. http://bugs.winehq.org/show_bug.cgi?id=11479 This doesn't work: ../../../tools/runtest -q -P wine -M msi.dll -T ../../.. -p msi_test.exe.so automation.c && touch automation.ok automation.c:276: Test failed: Expected ERROR_SUCCESS, got 1627 wine: Unhandled page fault on read access to 0x00000018 at address 0x6002a3da (thread 0009), starting debugger... Unhandled exception: page fault on read access to 0x00000018 in 32-bit code (0x6002a3da). Register dump: CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b EIP:6002a3da ESP:0033f400 EBP:0033f578 EFLAGS:00010206( - 00 - RIP1) EAX:00000018 EBX:60121844 ECX:00000018 EDX:0033f5b8 ESI:602adce0 EDI:0033f52d Stack dump: 0x0033f400: 00000000 4d430002 00132fac 00000004 0x0033f410: 600425f6 0033f44c 6065e20e 602adce0 0x0033f420: 0033f5b8 0033f5b8 00000001 7bc417fa 0x0033f430: 00000000 4d430002 00133004 00000004 0x0033f440: 00000010 0033f47c 0033f464 00000030 0x0033f450: 00134180 00110014 00000020 00000000 Backtrace: =>1 0x6002a3da vsnprintfW+0x3ca(str=0x33f67a, len=0x7fffffff, format=0x6065e20a, valist=0x33f5b8) [/home/julliard/wine/wine/libs/wine/string.c:376] in libwine.so.1 (0x0033f578) 2 0x6002a596 sprintfW+0x36(str=0x33f678, format=0x6065e20a) [/home/julliard/wine/wine/libs/wine/string.c:458] in libwine.so.1 (0x0033f5a8) 3 0x60610808 MSI_DatabaseImport+0x13e8(db=0x130ef8, folder=<is not available>, file=0x1329ca) [/home/julliard/wine/wine/dlls/msi/database.c:603] in msi (0x0033f7f8) 4 0x60610b29 MsiDatabaseImportW+0x129(handle=<register EDI not in topmost frame>, szFolder=0x12fcd0, szFilename=0x1329b0) [/home/julliard/wine/wine/dlls/msi/database.c:777] in msi (0x0033f828) 5 0x60610cce MsiDatabaseImportA+0x18e(handle=0x1, szFolder=0x605b43c0, szFilename=0x605a3db7) [/home/julliard/wine/wine/dlls/msi/database.c:803] in msi (0x0033f868) 6 0x60527631 test_Installer+0x571() [/home/julliard/wine/wine/dlls/msi/tests/automation.c:275] in msi_test (0x0033fbd8) 7 0x6052983b func_automation+0xebb() [/home/julliard/wine/wine/dlls/msi/tests/automation.c:2428] in msi_test (0x0033fe68) 8 0x60592cea run_test+0x14a(name=0x1103ec) [/home/julliard/wine/wine/dlls/msi/tests/../../../include/wine/test.h:389] in msi_test (0x0033fea8) 9 0x60593332 main+0x142(argc=<register ECX not in topmost frame>, argv=0x1103d0) [/home/julliard/wine/wine/dlls/msi/tests/../../../include/wine/test.h:437] in msi_test (0x0033fed8) 10 0x6059340b __wine_spec_exe_entry+0x5b(peb=0x7ffdf000) [/home/julliard/wine/wine/dlls/winecrt0/exe_entry.c:36] in msi_test (0x0033ff08) 11 0x7b8744ee start_process+0xee(arg=0x0) [/home/julliard/wine/wine/dlls/kernel32/process.c:883] in kernel32 (0x0033ffe8) 12 0x600288d7 wine_switch_to_stack+0x17() in libwine.so.1 (0x00000000) 0x6002a3da vsnprintfW+0x3ca [/home/julliard/wine/wine/libs/wine/string.c:376] in libwine.so.1: movzwl 0x0(%ecx),%edx 376 while (*striter) -- Alexandre Julliard julliard(a)winehq.org

Feb. 7, 2008

Re: 2/2 start.exe: Handle the process title argument (try3)

by Alexandre Julliard

Alexander Nicolaysen Sørnes <alex(a)thehandofagony.com> writes: > + vi.dwOSVersionInfoSize = sizeof(vi); > + GetVersionExW(&vi); > + > + /* On Windows 9x start does not have the process title argument. > + It works such that if the first parameter begins with a double quote ("), > + it is taken as the process title. Thus 'start "notepad" a' will work on > + Windows 9x but not NT variants */ > + if(argc > 1 && (vi.dwMajorVersion >= 5 || (vi.dwMajorVersion == 4 && vi.dwPlatformId == 2))) The right way to check for Win9x is GetVersion() & 0x80000000. > + { > + int cmdcount; > + WCHAR* cmdline = GetCommandLineW(); > + WCHAR** cmdargs = CommandLineToArgvW(cmdline, &cmdcount); > + WCHAR* pos = cmdline; > + > + pos += lstrlenW(cmdargs[0]); This won't work if the program name is quoted. -- Alexandre Julliard julliard(a)winehq.org

Feb. 7, 2008

Re: systray[3/4]: Better validate icon owner

by Robert Shearman

Reece Dunn wrote: > On 06/02/2008, Robert Shearman <rob(a)codeweavers.com> wrote: > >> PostMessage should correctly >> handle the case where icon->owner has been destroyed and adding a call >> to IsWindow just introduces a race condition. >> > > Is there a test case in the PostMessage tests to verify that behaviour? > No - I don't think the tests will tell us anything we don't already know, but don't let me stop you adding a test for "PostMessage((HWND)0xdeadbeef, WM_USER, 0, 0)". -- Rob Shearman

Feb. 7, 2008

Re: systray[3/4]: Better validate icon owner

by Robert Shearman

Kirill K. Smirnov wrote: >> I don't get why you need this change. PostMessage should correctly >> handle the case where icon->owner has been destroyed and adding a call >> to IsWindow just introduces a race condition. >> > > Just a testcase: > 1) run any wine app (to be sure that explorer is running), e.g winecfg > 2) run taskmgr > 3) select taskmgr process in tasklist and kill it. taskmgr window will > dissapear but tray icon not. > 4) move mouse cursor over taskmgr tray icon. Before this patch it will not > dissapear, but should. > This is pretty much the same test I did when I originally wrote the code. > The reason is that ERROR_INVALID_HANDLE is not the only value which indicates > the absence of owner window (ERROR_INVALID_WINDOW_HANDLE too). I think that > using robust and straightforward IsWindow() function is better than verify > last error. > The error value was changed in commit 19e7fab981a75cdf0a339db35d36d9c1a6e64494. The code should be changed to check for ERROR_INVALID_WINDOW_HANDLE now. > Is it acceptable to check return value of PostMessage only and do not check > last error? > Possibly, although I can think of other errors such as out-of-memory that probably shouldn't indicate that the parent has disappeared and the icon destroyed. > And what race condition introduces IsWindow() here? > In general, you shouldn't check whether something can be done before you try doing it. Just try doing it and cope with it failing. In this case, you're checking if the window is valid before you post a message to it. The window could be destroyed between IsWindow and PostMessage, in which case you'd have to detect the error by checking the result of the PostMessage call. After you've done this, the IsWindow call is redundant. -- Rob Shearman

Feb. 7, 2008

Re: [PATCH] dinput: Skip old mouse movement evenets.

by Alexandre Julliard

Vitaliy Margolen <wine-patches(a)kievinfo.com> writes: > @@ -280,6 +281,9 @@ static void dinput_mouse_hook( LPDIRECTINPUTDEVICE8A iface, WPARAM wparam, LPARA > { > POINT pt, pt1; > > + /* Skip old movement events */ > + if (This->last_event_time > hook->time) break; That's not an appropriate way to compare timestamps. -- Alexandre Julliard julliard(a)winehq.org

Feb. 7, 2008

Re: x11drv: implement X11DRV_AlphaRender for non bitmap sources

by Alexandre Julliard

Aric Stewart <aric(a)codeweavers.com> writes: > @@ -1569,12 +1570,14 @@ BOOL X11DRV_AlphaBlend(X11DRV_PDEVICE *devDst, INT xDst, INT yDst, INT widthDst, > > if (!devSrc->bitmap || GetObjectW( devSrc->bitmap->hbitmap, sizeof(dib), &dib ) != sizeof(dib)) > { > - FIXME("not a dibsection\n"); > - return FALSE; > + TRACE("not a dibsection\n"); > + bitmap = FALSE; It can be an invalid handle too, you can't assume that it's a valid bitmap just because it isn't a DIB. Also naming the boolean 'bitmap' for the DIB section case is very confusing, both are bitmaps. > > - if (xSrc < 0 || ySrc < 0 || widthSrc < 0 || heightSrc < 0 || xSrc + widthSrc > dib.dsBmih.biWidth > - || ySrc + heightSrc > abs(dib.dsBmih.biHeight)) > + if (xSrc < 0 || ySrc < 0 || widthSrc < 0 || heightSrc < 0 || (bitmap && xSrc + widthSrc > dib.dsBmih.biWidth) > + || (bitmap && ySrc + heightSrc > abs(dib.dsBmih.biHeight))) Testing the flag all over the place is ugly, you should separate the code completely, have one block for DIBs and one block for DDBs, with a single if at the top. -- Alexandre Julliard julliard(a)winehq.org

Feb. 7, 2008

Re: regression in worms2

by Olaf Leidinger

Hello! > The same bug happens in Patrician3.exe > If you apply the attached patch if should be ok. > > Please let me know, if that worked also for you. I already tried this work-around and it makes the game work. Alexandre Julliard also proposed a patch which works fine, please have a look at bug 11340. Ciao, Olaf -- Olaf Leidinger <leidola(a)newcon.de>

Feb. 7, 2008

Re: regression in worms2

by Dmitry Timoshkov

<luis.busquets(a)ilidium.com> wrote: > The same bug happens in Patrician3.exe > If you apply the attached patch if should be ok. That bug should be fixed in git. This is not an appropriate place to report the bugs, use bugzilla instead. -- Dmitry.

Feb. 7, 2008

regression in worms2

by luis.busquets＠ilidium.com

The same bug happens in Patrician3.exe If you apply the attached patch if should be ok. Please let me know, if that worked also for you.

Feb. 7, 2008