View Issue Details

IDProjectCategoryView StatusLast Update
0002820FSSCPpublic2013-04-13 04:01
ReporterEchelon9 Assigned Tochief1983  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.6.19 
Target Version3.7.0 
Summary0002820: AddressSanitizer: global-buffer-overflow in os_init_cmdline() cmdline.cpp on Mac
DescriptionReported 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 Reproduce1. 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
TagsNo tags attached.

Activities

Echelon9

2013-03-23 04:36

developer  

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++) 
 	{

Echelon9

2013-03-23 04:36

developer   ~0014811

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

Echelon9

2013-03-23 04:37

developer   ~0014812

Chief1983, would you mind reviewing this coding patch to resolve memory corruption in the manner in which config files are read on Mac?

chief1983

2013-03-23 05:31

administrator   ~0014813

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.

Zacam

2013-04-13 02:08

administrator   ~0014916

Compiled and ran fine on MSVC 2008, no issues to report.

Echelon9

2013-04-13 04:01

developer   ~0014917

Fix committed to trunk@9626.

Related Changesets

fs2open: trunk r9626

2013-04-13 00:51

Echelon9


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

Issue History

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