https://bugs.winehq.org/show_bug.cgi?id=38949
Bug ID: 38949 Summary: Free Falcon 5.x/6.x configuration editor segfaults on start Product: Wine Version: 1.7.47 Hardware: x86 URL: http://www.g4g.it/2013/11/24/free-falcon-v6-0/ OS: Linux Status: NEW Keywords: download, regression Severity: normal Priority: P2 Component: ntdll Assignee: wine-bugs@winehq.org Reporter: gyebro69@gmail.com CC: nerv@dawncrow.de Regression SHA1: 7e1c886fbfd362376b6aebe5381ab7d4433c3371 Distribution: ---
Created attachment 51896 --> https://bugs.winehq.org/attachment.cgi?id=51896 gdb backtrace from coredump
I'm getting only a 'Segmentation fault (core dumped)' message when trying to start the configuration tool for Free Falcon. Dmesg shows something like this: FFViper Config [1191]: segfault at e18d24fa ip 7bc9882a sp bfe70f30 error 5 in ntdll.dll.so[7bc11000+c4000]
The problem is present since
commit 7e1c886fbfd362376b6aebe5381ab7d4433c3371 Author: André Hentschel nerv@dawncrow.de Date: Tue Jul 7 19:50:25 2015 +0200
ntdll: Randomize security cookie when available.
To reproduce the problem download and unpack FF6d.7z, launch the installer.Start 'FFViper Config Editor.exe'. The configuration tool requires mfc42.dll, but the segfault occurs earlier.
Still present in wine-1.7.47-118-ga90592c
FF6d.7z (935M) sha1: a9b589245b9d5b1afafc0ba5857b8bbcbf7a5b27
https://bugs.winehq.org/show_bug.cgi?id=38949
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |focht@gmx.net Summary|Free Falcon 5.x/6.x |Free Falcon 5.x/6.x |configuration editor |configuration editor |segfaults on start |segfaults on start (loader | |must take invalid | |IMAGE_LOAD_CONFIG_DIRECTORY | |values into account)
--- Comment #1 from Anastasius Focht focht@gmx.net --- Hello folks,
confirming.
The main executable is UPX compressed:
--- snip --- -=[ ProtectionID v0.6.6.7 DECEMBER]=- (c) 2003-2015 CDKiLLER & TippeX Build 24/12/14-22:48:13 Ready... Scanning -> C:\FreeFalcon6\FFViper Config Editor.exe File Type : 32-Bit Exe (Subsystem : Win GUI / 2), Size : 563793 (089A51h) Byte(s) Compilation TimeStamp : 0x3F6E2EA1 -> Sun 21st Sep 2003 23:05:05 (GMT) [TimeStamp] 0x3F6E2EA1 -> Sun 21st Sep 2003 23:05:05 (GMT) | PE Header | - | Offset: 0x00000128 | VA: 0x00400128 | - [TimeStamp] 0xC7530092 -> Fri 20th Dec 2075 22:13:38 (GMT) | Export | - | Offset: 0x000275D4 | VA: 0x0048B1D4 | Probably invalid [TimeStamp] 0x9B474648 -> Sun 21st Jul 2052 01:52:08 (GMT) | LoadConfig | - | Offset: 0x0001A4F4 | VA: 0x0047E0F4 | - -> File has 290385 (046E51h) bytes of appended data starting at offset 042C00h [!] Warning - export data seems to be invalid [File Heuristics] -> Flag #1 : 00000000000000001100001100100111 (0x0000C327) [Entrypoint Section Entropy] : 7.92 (section #1) "UPX1 " | Size : 0x3A800 (239616) byte(s) [DllCharacteristics] -> Flag : (0x0000) -> NONE [SectionCount] 3 (0x3) | ImageSize 0xA7000 (684032) byte(s) [VersionInfo] Product Name : F4Patch Application [VersionInfo] Product Version : 5. 0. 0. 0 [VersionInfo] File Description : F4Patch Application [VersionInfo] File Version : 5. 0. 1. 8 [VersionInfo] Original FileName : F4Patch.EXE [VersionInfo] Internal Name : F4Patch [VersionInfo] Legal Copyrights : Copyright (C) 2000-2003 Joel Bierling [!] UPX 1.24 compressed ! upx internal version : 012 / compression method : 05 (M_NRV2D_LE32) - Level : 010 decompressed adler32 : 0x21EA0B85 / compressed adler32 : 0x5F45E02D uncompressed size : 0x0009CB38 (0641848) / compressed size : 0x0003A4D8 (0238808) original file size : 0x00098000 (0622592) / filter : 0x026 / ct0 0x17 / linkchecksum : 0x0DF - Scan Took : 0.436 Second(s) [0000001B4h (436) tick(s)] [558 of 573 scan(s) done] --- snip ---
Relevant part of trace log (early loader phase):
--- snip --- $ pwd /home/focht/.wine/drive_c/FreeFalcon6
$ WINEDEBUG=+tid,+seh,+relay,+server,+ntdll,+module,+virtual wine ./FFViper\ Config\ Editor.exe >>log.txt 2>&1 ... 0009:trace:module:load_native_dll Trying native dll L"C:\FreeFalcon6\FFViper Config Editor.exe" 0009: create_mapping( access=000f0005, attributes=00000000, protect=00000145, size=00000000, file_handle=0018, objattr={rootdir=0000,sd={},name=L""} ) 0009: create_mapping() = 0 { handle=001c } 0009:trace:virtual:NtMapViewOfSection handle=0x1c process=0xffffffff addr=(nil) off=000000000 size=0 access=20 0009: get_mapping_info( handle=001c, access=00000004 ) 0009: get_mapping_info() = 0 { size=000a7000, protect=325, header_size=4096, base=00400000, mapping=0020, shared_file=0000 } 0009: get_handle_fd( handle=001c ) 0009: *fd* 001c -> 26 0009: get_handle_fd() = 0 { type=1, cacheable=1, access=000f0005, options=00000020 } 0009:trace:virtual:VIRTUAL_DumpView View: 0x400000 - 0x4a6fff (anonymous) 0009:trace:virtual:VIRTUAL_DumpView 0x400000 - 0x4a6fff c-rWx 0009:trace:module:map_image mapped PE file at 0x400000-0x4a7000 0009:trace:module:map_image mapping section UPX0 at 0x401000 off 400 size 0 virt 63000 flags e0000080 0009:trace:module:map_image mapping section UPX1 at 0x464000 off 400 size 3a800 virt 3b000 flags e0000040 0009:trace:module:map_image clearing 0x49e800 - 0x49f000 0009:trace:module:map_image mapping section .rsrc at 0x49f000 off 3ac00 size 8000 virt 8000 flags c0000040 0009: *killed* exit_code=0 --- snip ---
Using 'strace':
--- snip --- ... 7159 execve("/home/focht/projects/wine/wine.repo/install/bin/wine", ["wine", "./FFViper Config Editor.exe"], ...) = 0 7159 brk(0) = 0x7c2ae000 7159 mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf77c9000 7159 readlink("/proc/self/exe", "/home/focht/projects/wine/wine.r"..., 4096) = 52 ... 7159 open("/home/focht/projects/wine/wine.repo/install/bin/../lib/libwine.so.1", O_RDONLY|O_CLOEXEC) = 3 7159 read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0000@\0\0004\0\0\0"..., 512) = 512 ... 7159 pread64(9, "MZ\220\0\3\0\0\0\4\0\0\0\377\377\0\0\270\0\0\0\0\0\0\0@\0\0\0\0\0\0\0"..., 85, 0) = 85 7159 _llseek(9, 85, [85], SEEK_SET) = 0 7159 rt_sigprocmask(SIG_BLOCK, [HUP INT USR1 USR2 ALRM CHLD IO], [HUP INT USR1 USR2 ALRM CHLD IO], 8) = 0 7159 writev(3, [{"M\0\0\0\f\0\0\0\0\0\0\0\5\0\17\0\0\0\0\0E\1\0\0\0\0\0\0\0\0\0\0"..., 64}, {"\0\0\0\0\0\0\0\0\0\0\0\0", 12}], 2) = 76 7159 read(5, "\0\0\0\0\0\0\0\0\24\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 64) = 64 7159 rt_sigprocmask(SIG_SETMASK, [HUP INT USR1 USR2 ALRM CHLD IO], NULL, 8) = 0 7159 rt_sigprocmask(SIG_BLOCK, [HUP INT USR1 USR2 ALRM CHLD IO], [HUP INT USR1 USR2 ALRM CHLD IO], 8) = 0 7159 write(3, "O\0\0\0\0\0\0\0\0\0\0\0\24\0\0\0\4\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 64) = 64 7159 read(5, "\0\0\0\0\0\0\0\0\0p\n\0\0\0\0\0E\1\0\0\0\20\0\0\0\0@\0\0\0\0\0"..., 64) = 64 7159 rt_sigprocmask(SIG_SETMASK, [HUP INT USR1 USR2 ALRM CHLD IO], NULL, 8) = 0 7159 rt_sigprocmask(SIG_BLOCK, [HUP INT USR1 USR2 ALRM CHLD IO], [HUP INT USR1 USR2 ALRM CHLD IO], 8) = 0 7159 rt_sigprocmask(SIG_BLOCK, [HUP INT USR1 USR2 ALRM CHLD IO], [HUP INT USR1 USR2 ALRM CHLD IO], 8) = 0 7159 write(3, "*\0\0\0\0\0\0\0\0\0\0\0\24\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 64) = 64 7159 read(5, "\0\0\0\0\0\0\0\0\1\0\0\0\1\0\0\0\5\0\17\0 \0\0\0\0\0\0\0\0\0\0\0"..., 64) = 64 7159 rt_sigprocmask(SIG_SETMASK, [HUP INT USR1 USR2 ALRM CHLD IO], NULL, 8) = 0 7159 recvmsg(4, {msg_name(0)=NULL, msg_iov(1)=[{"\24\0\0\0", 4}], msg_controllen=16, [{cmsg_len=16, cmsg_level=SOL_SOCKET, cmsg_type=SCM_RIGHTS, [10]}], msg_flags=MSG_CMSG_CLOEXEC}, MSG_CMSG_CLOEXEC) = 4 7159 fcntl64(10, F_SETFD, FD_CLOEXEC) = 0 7159 rt_sigprocmask(SIG_SETMASK, [HUP INT USR1 USR2 ALRM CHLD IO], NULL, 8) = 0 7159 rt_sigprocmask(SIG_BLOCK, [HUP INT USR1 USR2 ALRM CHLD IO], [HUP INT USR1 USR2 ALRM CHLD IO], 8) = 0 7159 mmap2(0x400000, 684032, PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x400000 7159 fstat64(10, {st_dev=makedev(0, 82), st_ino=432290, st_mode=S_IFREG|0664, st_nlink=1, st_uid=1000, st_gid=1000, st_blksize=4096, st_blocks=1104, st_size=563793, st_atime=2015/07/19-12:20:28, st_mtime=2015/07/19-12:20:03, st_ctime=2015/07/19-12:20:03}) = 0 7159 mmap2(0x400000, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED, 10, 0) = 0x400000 7159 mmap2(0x464000, 239616, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x464000 7159 pread64(10, "o\373\377\377\213\316\350\27\5ad3\322\307\6\350\220G\0\211V`\270p7\16F\\5d\277,"..., 239616, 1024) = 239616 7159 mmap2(0x49f000, 32768, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x49f000 7159 pread64(10, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\n\0\1\0\0\0`\0\0\200\2\0\0\0p\3\0\200"..., 32768, 240640) = 32768 7159 --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_ACCERR, si_addr=0xe18d24fa} --- 7159 +++ killed by SIGSEGV (core dumped) +++ --- snip ---
Dump of optional header table.
--- snip --- Magic: 0x010B (HDR32_MAGIC) MajorLinkerVersion: 0x07 MinorLinkerVersion: 0x0A -> 7.10 SizeOfCode: 0x0003B000 SizeOfInitializedData: 0x00008000 SizeOfUninitializedData: 0x00063000 AddressOfEntryPoint: 0x0009E4E0 BaseOfCode: 0x00064000 BaseOfData: 0x0009F000 ImageBase: 0x00400000 SectionAlignment: 0x00001000 FileAlignment: 0x00000200 MajorOperatingSystemVersion: 0x0004 MinorOperatingSystemVersion: 0x0000 -> 4.00 MajorImageVersion: 0x0000 MinorImageVersion: 0x0000 -> 0.00 MajorSubsystemVersion: 0x0004 MinorSubsystemVersion: 0x0000 -> 4.00 Win32VersionValue: 0x00000000 SizeOfImage: 0x000A7000 SizeOfHeaders: 0x00001000 CheckSum: 0x00000000 Subsystem: 0x0002 (WINDOWS_GUI) DllCharacteristics: 0x0000 SizeOfStackReserve: 0x00100000 SizeOfStackCommit: 0x00001000 SizeOfHeapReserve: 0x00100000 SizeOfHeapCommit: 0x00001000 LoaderFlags: 0x00000000 NumberOfRvaAndSizes: 0x00000010
DataDirectory (16) RVA Size ------------- ---------- ---------- ExportTable 0x0008B1D0 0x0000005E ("UPX1") ImportTable 0x000A6B04 0x00000304 (".rsrc") Resource 0x0009F000 0x00007B04 (".rsrc") Exception 0x00000000 0x00000000 Security 0x00000000 0x00000000 Relocation 0x00000000 0x00000000 Debug 0x00000000 0x00000000 Copyright 0x00000000 0x00000000 GlobalPtr 0x00000000 0x00000000 TLSTable 0x00000000 0x00000000 LoadConfig 0x0007E0F0 0x00000048 ("UPX1") BoundImport 0x00000000 0x00000000 IAT 0x00000000 0x00000000 DelayImport 0x00088D0C 0x00000040 ("UPX1") COM 0x00000000 0x00000000 Reserved 0x00000000 0x00000000 --- snip ---
Section header table:
--- snip --- 1. item: Name: UPX0 VirtualSize: 0x00063000 VirtualAddress: 0x00001000 SizeOfRawData: 0x00000000 PointerToRawData: 0x00000400 PointerToRelocations: 0x00000000 PointerToLinenumbers: 0x00000000 NumberOfRelocations: 0x0000 NumberOfLinenumbers: 0x0000 Characteristics: 0xE0000080 (UNINITIALIZED_DATA, EXECUTE, READ, WRITE)
2. item: Name: UPX1 VirtualSize: 0x0003B000 VirtualAddress: 0x00064000 SizeOfRawData: 0x0003A800 PointerToRawData: 0x00000400 PointerToRelocations: 0x00000000 PointerToLinenumbers: 0x00000000 NumberOfRelocations: 0x0000 NumberOfLinenumbers: 0x0000 Characteristics: 0xE0000040 (INITIALIZED_DATA, EXECUTE, READ, WRITE)
3. item: Name: .rsrc VirtualSize: 0x00008000 VirtualAddress: 0x0009F000 SizeOfRawData: 0x00008000 PointerToRawData: 0x0003AC00 PointerToRelocations: 0x00000000 PointerToLinenumbers: 0x00000000 NumberOfRelocations: 0x0000 NumberOfLinenumbers: 0x0000 Characteristics: 0xC0000040 (INITIALIZED_DATA, READ, WRITE) --- snip ---
Whoops ... the load configuration directory RVA points into UPX1 section.
Dumping the load config directory:
--- snip ---
Characteristics: 0x3F1FA11E TimeDateStamp: 0x9B474648 MajorVersion: 0xE297 MinorVersion: 0xE3CB -> 58007.58315 GlobalFlagsClear: 0xC3E77F4D GlobalFlagsSet: 0x168A3A5E CriticalSectionDefaultTimeout: 0x56DBC9D5 DeCommitFreeBlockThreshold: 0xCA3A402F DeCommitTotalFreeThreshold: 0xB7ED893F LockPrefixTable: 0x0C27171E MaximumAllocationSize: 0xD84A7A5F VirtualMemoryThreshold: 0xEDC8D34F ProcessHeapFlags: 0x44EBDB05 ProcessAffinityMask: 0xFBDA5727 CSDVersion: 0x649F Reserved: 0x8BA6 EditList: 0x36F61998 SecurityCookie: 0xE18D24FA ... --- snip ---
The segfault address (strace) is 0xe18d24fa -> SecurityCookie "address" ;-)
The content of some PE image directories is invalid before the UPX decompressor stub is run. You must take this possibility into account before trying to access these data structures.
https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/ntdll/virtual.c#l1321
Regards
https://bugs.winehq.org/show_bug.cgi?id=38949
Sebastian Lackner sebastian@fds-team.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |sebastian@fds-team.de See Also| |https://bugs.winehq.org/sho | |w_bug.cgi?id=38895
https://bugs.winehq.org/show_bug.cgi?id=38949
--- Comment #2 from Sebastian Lackner sebastian@fds-team.de --- (In reply to Anastasius Focht from comment #1)
The content of some PE image directories is invalid before the UPX decompressor stub is run. You must take this possibility into account before trying to access these data structures.
Validating if the SecurityCookie is inside of the module should fix this, however I wonder if this is really a complete fix. Do we theoretically also need relocation of the SecurityCookie pointer?
https://bugs.winehq.org/show_bug.cgi?id=38949
--- Comment #3 from André H. nerv@dawncrow.de --- (In reply to Sebastian Lackner from comment #2)
(In reply to Anastasius Focht from comment #1)
The content of some PE image directories is invalid before the UPX decompressor stub is run. You must take this possibility into account before trying to access these data structures.
Validating if the SecurityCookie is inside of the module should fix this, however I wonder if this is really a complete fix. Do we theoretically also need relocation of the SecurityCookie pointer?
Checking if the pointer is sane is one thing, but what about the decompressed exe, it also needs a random cookie value...
https://bugs.winehq.org/show_bug.cgi?id=38949
--- Comment #4 from Anastasius Focht focht@gmx.net --- Hello folks,
--- quote --- Validating if the SecurityCookie is inside of the module should fix this, however I wonder if this is really a complete fix. Do we theoretically also need relocation of the SecurityCookie pointer? --- quote ---
I think that should be taken care by the PE compressor code itself. Whoever messes with load config directory has to emit/keep relocation entries for:
* LockPrefixTable (VA) * EditList (VA) * SecurityCookie (VA) * SEHandlerTable (VA)
I would assume the PE compressors generate metadata about the original file sections, which includes relocations. When the compressor creates the final PE, those relocation entries get stripped and are instead stored in internal table. The uncompressor stub has to implement OS loader behaviour by processing those internally stored relocation entries on its own, which should also include load config VAs before passing control to the real application entry point.
Of course the security cookie can't be randomized by OS loader this way (retaining the original value as emitted by VC++ compiler /GS) but that's a minor drawback. The app will do a "once" randomization of the security cookie on its own as part of CRT startup.
Regards
https://bugs.winehq.org/show_bug.cgi?id=38949
--- Comment #5 from André H. nerv@dawncrow.de --- Created attachment 51899 --> https://bugs.winehq.org/attachment.cgi?id=51899 ntdll: Don't touch SecurityCookie when the pointer is outside of the image
What about this patch?
https://bugs.winehq.org/show_bug.cgi?id=38949
--- Comment #6 from Sebastian Lackner sebastian@fds-team.de --- (In reply to Anastasius Focht from comment #4)
I think that should be taken care by the PE compressor code itself. Whoever messes with load config directory has to emit/keep relocation entries for:
- LockPrefixTable (VA)
- EditList (VA)
- SecurityCookie (VA)
- SEHandlerTable (VA)
When the PE compressor code does the relocation, then yes. But Wine also contains relocation code, thats what I was concerned about.
(In reply to André H. from comment #5)
Created attachment 51899 [details] ntdll: Don't touch SecurityCookie when the pointer is outside of the image
What about this patch?
You are truncating the pointer on 64-bit, you'll have to use something like DWORD_PTR or ULONG_PTR (or alternatively do the test with pointer types). Besides that, I think it would be better to check:
(ULONG_PTR)ptr >= loadcfg->SecurityCookie && loadcfg->SecurityCookie <= (ULONG_PTR)ptr + total_size - sizeof(ULONG_PTR)
https://bugs.winehq.org/show_bug.cgi?id=38949
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |6e66c12c68c6b35ec6ff037e032 | |979fb1dacbe26 Status|NEW |RESOLVED Resolution|--- |FIXED
--- Comment #7 from Anastasius Focht focht@gmx.net --- Hello folks,
this is fixed by commit https://source.winehq.org/git/wine.git/commitdiff/6e66c12c68c6b35ec6ff037e03...
Thanks Sebastian
Regards
https://bugs.winehq.org/show_bug.cgi?id=38949
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #8 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 1.7.49.
-
wine-bugs＠winehq.org