View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002820 | FSSCP | public | 2013-03-23 04:33 | 2013-04-13 04:01 | |
Reporter | Echelon9 | Assigned To | chief1983 | ||
Priority | normal | Severity | major | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Product Version | 3.6.19 | ||||
Target Version | 3.7.0 | ||||
Summary | 0002820: AddressSanitizer: global-buffer-overflow in os_init_cmdline() cmdline.cpp on Mac | ||||
Description | Reported by AddressSanitizer, a memory error detector for C/C++, in FS2Open builds based on trunk r9585. ERROR: AddressSanitizer: global-buffer-overflow on address 0x0001033d189f at pc 0x10080244f bp 0x7fff5fbfd150 sp 0x7fff5fbfd148 READ of size 1 at 0x0001033d189f thread T0 #0 0x10080244e in os_init_cmdline cmdline.cpp:773 0000001 0x100808a9c in parse_cmdline cmdline.cpp:1600 0000002 0x1003058e8 in game_main freespace.cpp:6970 0000003 0x100307586 in SDL_main freespace.cpp:7179 ... void os_init_cmdline(char *cmdline) { ... #elif defined(APPLE_APP) extern char full_path[1024]; char *c = NULL, data_path[1024]; c = strstr(full_path, ".app"); if ( c != NULL ) { while (c && (*c != '/')) <=============== c--; *c = '\0'; } snprintf(data_path, 1024, "%s/data/cmdline_fso.cfg", full_path); fp = fopen(data_path, "rt"); #else | ||||
Steps To Reproduce | 1. Utilise a version of Clang that supports AddressSantizer (https://code.google.com/p/address-sanitizer/wiki/AddressSanitizer) 2. Build with -fsanitize=address C/C++ flag 3. Run the game with mediavps 3.6.12 | ||||
Additional Information | ==ERROR: AddressSanitizer: global-buffer-overflow on address 0x0001033d189f at pc 0x10080244f bp 0x7fff5fbfd150 sp 0x7fff5fbfd148 READ of size 1 at 0x0001033d189f thread T0 #0 0x10080244e in os_init_cmdline cmdline.cpp:773 0000001 0x100808a9c in parse_cmdline cmdline.cpp:1600 0000002 0x1003058e8 in game_main freespace.cpp:6970 0000003 0x100307586 in SDL_main freespace.cpp:7179 0000004 0x100002a5a in -[SDLMain applicationDidFinishLaunching:] SDLMain.m:300 0000005 0x7fff8ffe3ed9 in _CFXNotificationPost (in CoreFoundation) + 2553 0000006 0x7fff85fc6e25 in -[NSNotificationCenter postNotificationName:object:userInfo:] (in Foundation) + 63 0000007 0x7fff8b92855c in -[NSApplication _postDidFinishNotification] (in AppKit) + 291 0000008 0x7fff8b928295 in -[NSApplication _sendFinishLaunchingNotification] (in AppKit) + 215 0000009 0x7fff8b925481 in -[NSApplication(NSAppleEventHandling) _handleAEOpenEvent:] (in AppKit) + 565 0000010 0x7fff8b92507b in -[NSApplication(NSAppleEventHandling) _handleCoreEvent:withReplyEvent:] (in AppKit) + 350 #11 0x7fff85fe070a in -[NSAppleEventManager dispatchRawAppleEvent:withRawReply:handlerRefCon:] (in Foundation) + 307 0000012 0x7fff85fe056c in _NSAppleEventManagerGenericHandler (in Foundation) + 105 0000013 0x7fff8e3a9077 in aeDispatchAppleEvent(AEDesc const*, AEDesc*, unsigned int, unsigned char*) (in AE) + 306 0000014 0x7fff8e3a8ed8 in dispatchEventAndSendReply(AEDesc const*, AEDesc*) (in AE) + 36 0000015 0x7fff8e3a8d98 in aeProcessAppleEvent (in AE) + 317 0000016 0x7fff88b11708 in AEProcessAppleEvent (in HIToolbox) + 99 0000017 0x7fff8b921865 in _DPSNextEvent (in AppKit) + 1455 0000018 0x7fff8b920e21 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in AppKit) + 127 0000019 0x7fff8b9181d2 in -[NSApplication run] (in AppKit) + 516 0000020 0x100004b73 in CustomApplicationMain SDLMain.m:227 0000021 0x1000045f0 in main SDLMain.m:377 0000022 0x1000017c3 in start (in FS2_Open (debug)) + 51 0000023 0x0 in 0x0 0x0001033d189f is located 1 bytes to the left of global variable 'full_path' from 'fs2open/trunk/fs2_open/projects/Xcode4/../../code/freespace2/freespace.cpp' (0x1033d18a0) of size 1024 'full_path' is ascii string 'FS2_Open (debug).app/Contents/MacOS/FS2_Open (debug)' 0x0001033d189f is located 39 bytes to the right of global variable 'dc_gamma' from 'fs2open/trunk/fs2_open/projects/Xcode4/../../code/freespace2/freespace.cpp' (0x1033d1860) of size 24 Shadow bytes around the buggy address: 0x10002067a2c0: f9 f9 f9 f9 00 00 00 f9 f9 f9 f9 f9 00 00 00 f9 0x10002067a2d0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 f9 0x10002067a2e0: f9 f9 f9 f9 00 00 00 f9 f9 f9 f9 f9 04 f9 f9 f9 0x10002067a2f0: f9 f9 f9 f9 00 00 00 f9 f9 f9 f9 f9 00 00 00 f9 0x10002067a300: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 f9 =>0x10002067a310: f9 f9 f9[f9]00 00 00 00 00 00 00 00 00 00 00 00 0x10002067a320: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10002067a330: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10002067a340: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10002067a350: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10002067a360: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap righ redzone: fb Freed Heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 ASan internal: fe ==24931==ABORTING | ||||
Tags | No tags attached. | ||||
|
fix-for-mantis-2820-path-issues.patch (1,706 bytes)
Index: code/cmdline/cmdline.cpp =================================================================== --- code/cmdline/cmdline.cpp (revision 9588) +++ code/cmdline/cmdline.cpp (working copy) @@ -765,19 +765,12 @@ #ifdef _WIN32 fp = fopen("data\\cmdline_fso.cfg", "rt"); #elif defined(APPLE_APP) - extern char full_path[1024]; - char *c = NULL, data_path[1024]; + char resolved_path[MAX_PATH], data_path[MAX_PATH_LEN]; + + GetCurrentDirectory(MAX_PATH_LEN-1, data_path); + snprintf(resolved_path, MAX_PATH, "%s/data/cmdline_fso.cfg", data_path); - c = strstr(full_path, ".app"); - if ( c != NULL ) { - while (c && (*c != '/')) - c--; - - *c = '\0'; - } - snprintf(data_path, 1024, "%s/data/cmdline_fso.cfg", full_path); - - fp = fopen(data_path, "rt"); + fp = fopen(resolved_path, "rt"); #else fp = fopen("data/cmdline_fso.cfg", "rt"); #endif Index: code/freespace2/freespace.cpp =================================================================== --- code/freespace2/freespace.cpp (revision 9588) +++ code/freespace2/freespace.cpp (working copy) @@ -1724,20 +1724,8 @@ cmdline_debug_print_cmdline(); #endif -#ifdef APPLE_APP - // some OSX hackery to drop us out of the APP the binary is run from - char *c = NULL; - c = strstr(full_path, ".app"); - if ( c != NULL ) { - while (c && (*c != '/')) - c--; - - *c = '\0'; - } - strncpy(whee, full_path, MAX_PATH_LEN-1); -#else GetCurrentDirectory(MAX_PATH_LEN-1, whee); -#endif + strcat_s(whee, DIR_SEPARATOR_STR); strcat_s(whee, EXE_FNAME); @@ -8089,7 +8077,7 @@ _finddata_t find; int find_handle; - GetCurrentDirectory(MAX_PATH, oldpath); + GetCurrentDirectory(MAX_PATH-1, oldpath); for (i = 0; i < 26; i++) { |
|
The process of walking back along the char array reads one byte too far past the beginning in the current form. Adding a proposed patch that resolves this memory access error, plus a similar one in freespace.cpp |
|
Chief1983, would you mind reviewing this coding patch to resolve memory corruption in the manner in which config files are read on Mac? |
|
It looks sound but I'd like it to actually be tested on Windows at least first. And I'm headed to bed at the moment, got a motorcycle class to occupy myself with this weekend so I won't be getting to that any time soon. |
|
Compiled and ran fine on MSVC 2008, no issues to report. |
|
Fix committed to trunk@9626. |
fs2open: trunk r9626 2013-04-13 00:51 Ported: N/A Details Diff |
Fix for Mantis 2820: AddressSanitizer: global-buffer-overflow in os_init_cmdline() cmdline.cpp on Mac |
Affected Issues 0002820 |
|
mod - /trunk/fs2_open/code/cmdline/cmdline.cpp | Diff File | ||
mod - /trunk/fs2_open/code/freespace2/freespace.cpp | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2013-03-23 04:33 | Echelon9 | New Issue | |
2013-03-23 04:33 | Echelon9 | Status | new => assigned |
2013-03-23 04:33 | Echelon9 | Assigned To | => Echelon9 |
2013-03-23 04:36 | Echelon9 | File Added: fix-for-mantis-2820-path-issues.patch | |
2013-03-23 04:36 | Echelon9 | Note Added: 0014811 | |
2013-03-23 04:37 | Echelon9 | Note Added: 0014812 | |
2013-03-23 04:37 | Echelon9 | Assigned To | Echelon9 => chief1983 |
2013-03-23 04:37 | Echelon9 | Status | assigned => code review |
2013-03-23 05:31 | chief1983 | Note Added: 0014813 | |
2013-03-26 11:23 | Echelon9 | Summary | ERROR: AddressSanitizer: global-buffer-overflow in os_init_cmdline() cmdline.cpp on Mac => AddressSanitizer: global-buffer-overflow in os_init_cmdline() cmdline.cpp on Mac |
2013-04-13 02:08 | Zacam | Note Added: 0014916 | |
2013-04-13 04:01 | Echelon9 | Changeset attached | => fs2open trunk r9626 |
2013-04-13 04:01 | Echelon9 | Note Added: 0014917 | |
2013-04-13 04:01 | Echelon9 | Status | code review => resolved |
2013-04-13 04:01 | Echelon9 | Resolution | open => fixed |