All Projects FSSCP  wxFRED

View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0002844FSSCPPilot datapublic2013-04-14 01:552013-04-15 13:58
ReporterEchelon9 Assigned ToEchelon9  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.6.19 
Target Version3.7.0 
Summary0002844: AddressSanitizer: heap-buffer-overflow in pilotfile_convert::csg_import_red_alert csg_convert.cpp:438
DescriptionReported by AddressSanitizer, a memory error detector for C/C++, in FS2Open builds based on trunk r9629.

ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61d000111278 at pc 0x1000503e9 bp 0x7fff5fbfbff0 sp 0x7fff5fbfbfe8
READ of size 4 at 0x61d000111278 thread T0
    #0 0x1000503e8 in pilotfile_convert::csg_import_red_alert csg_convert.cpp:438
    0000001 0x100056095 in pilotfile_convert::csg_import csg_convert.cpp:648
    0000002 0x10006629a in pilotfile_convert::csg_convert csg_convert.cpp:1159
    0000003 0x1000d97ee in convert_pilot_files pilotfile_convert.cpp:143
    0000004 0x1002c88ee in game_init freespace.cpp:2008
    0000005 0x10030705b in game_main freespace.cpp:6976
    0000006 0x100308c06 in SDL_main freespace.cpp:7167
    ...
    0x61d000111278 is located 8 bytes to the left of 2048-byte region [0x61d000111280,0x61d000111a80)
allocated by thread T0 here:
    #0 0x1054986d3 in wrap_malloc (in libclang_rt.asan_osx_dynamic.dylib) + 51
    0000001 0x1025b773f in _vm_malloc stubs.cpp:571
    0000002 0x1000a5a0f in SCP_vm_allocator<index_list_t>::allocate vmallocator.h:59
    0000003 0x1000a5784 in std::_Vector_base<index_list_t, SCP_vm_allocator<index_list_t> >::_M_allocate stl_vector.h:131
    0000004 0x1000b3693 in std::vector<index_list_t, SCP_vm_allocator<index_list_t> >::_M_insert_aux vector.tcc:271
    0000005 0x10006a009 in std::vector<index_list_t, SCP_vm_allocator<index_list_t> >::push_back stl_vector.h:608
    0000006 0x100048536 in pilotfile_convert::csg_import_ships_weapons csg_convert.cpp:181
    0000007 0x100055f5e in pilotfile_convert::csg_import csg_convert.cpp:641
    0000008 0x10006629a in pilotfile_convert::csg_convert csg_convert.cpp:1159
    0000009 0x1000d97ee in convert_pilot_files pilotfile_convert.cpp:143
    0000010 0x1002c88ee in game_init freespace.cpp:2008
    #11 0x10030705b in game_main freespace.cpp:6976
    0000012 0x100308c06 in SDL_main freespace.cpp:7167


void pilotfile_convert::csg_import_red_alert()
{
    ...
            for (j = 0; j < 3; j++) {
            i = cfread_int(cfp);

            if (i >= weapon_list_size) {
                throw "Data check failure (RedAlert-weapon)!";
            }

            weapons.index = csg->weapon_list[i].index; <========
            weapons.count = cfread_int(cfp);

            if (weapons.index >= 0) {
                ras.primary_weapons.push_back( weapons );
            }
        }
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. Open FS2Open such that it will need to convert a non-Inferno pilot, affected by the Red Alert bug, into an Inferno pilot format
Additional InformationERROR: AddressSanitizer: heap-buffer-overflow on address 0x61d000111278 at pc 0x1000503e9 bp 0x7fff5fbfbff0 sp 0x7fff5fbfbfe8
READ of size 4 at 0x61d000111278 thread T0
    #0 0x1000503e8 in pilotfile_convert::csg_import_red_alert csg_convert.cpp:438
    0000001 0x100056095 in pilotfile_convert::csg_import csg_convert.cpp:648
    0000002 0x10006629a in pilotfile_convert::csg_convert csg_convert.cpp:1159
    0000003 0x1000d97ee in convert_pilot_files pilotfile_convert.cpp:143
    0000004 0x1002c88ee in game_init freespace.cpp:2008
    0000005 0x10030705b in game_main freespace.cpp:6976
    0000006 0x100308c06 in SDL_main freespace.cpp:7167
    0000007 0x100002b7a in -[SDLMain applicationDidFinishLaunching:] SDLMain.m:300
    0000008 0x7fff9502aed9 in _CFXNotificationPost (in CoreFoundation) + 2553
    0000009 0x7fff8b00de25 in -[NSNotificationCenter postNotificationName:object:userInfo:] (in Foundation) + 63
    0000010 0x7fff9096f55c in -[NSApplication _postDidFinishNotification] (in AppKit) + 291
    #11 0x7fff9096f295 in -[NSApplication _sendFinishLaunchingNotification] (in AppKit) + 215
    0000012 0x7fff9096c481 in -[NSApplication(NSAppleEventHandling) _handleAEOpenEvent:] (in AppKit) + 565
    0000013 0x7fff9096c07b in -[NSApplication(NSAppleEventHandling) _handleCoreEvent:withReplyEvent:] (in AppKit) + 350
    0000014 0x7fff8b02770a in -[NSAppleEventManager dispatchRawAppleEvent:withRawReply:handlerRefCon:] (in Foundation) + 307
    0000015 0x7fff8b02756c in _NSAppleEventManagerGenericHandler (in Foundation) + 105
    0000016 0x7fff933f0077 in aeDispatchAppleEvent(AEDesc const*, AEDesc*, unsigned int, unsigned char*) (in AE) + 306
    0000017 0x7fff933efed8 in dispatchEventAndSendReply(AEDesc const*, AEDesc*) (in AE) + 36
    0000018 0x7fff933efd98 in aeProcessAppleEvent (in AE) + 317
    0000019 0x7fff8db58708 in AEProcessAppleEvent (in HIToolbox) + 99
    0000020 0x7fff90968865 in _DPSNextEvent (in AppKit) + 1455
    0000021 0x7fff90967e21 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in AppKit) + 127
    0000022 0x7fff9095f1d2 in -[NSApplication run] (in AppKit) + 516
    0000023 0x100004c93 in CustomApplicationMain SDLMain.m:227
    0000024 0x100004710 in main SDLMain.m:377
    0000025 0x1000018e3 in start (in FS2_Open (debug)) + 51
    0000026 0x0 in 0x0
0x61d000111278 is located 8 bytes to the left of 2048-byte region [0x61d000111280,0x61d000111a80)
allocated by thread T0 here:
    #0 0x1054986d3 in wrap_malloc (in libclang_rt.asan_osx_dynamic.dylib) + 51
    0000001 0x1025b773f in _vm_malloc stubs.cpp:571
    0000002 0x1000a5a0f in SCP_vm_allocator<index_list_t>::allocate vmallocator.h:59
    0000003 0x1000a5784 in std::_Vector_base<index_list_t, SCP_vm_allocator<index_list_t> >::_M_allocate stl_vector.h:131
    0000004 0x1000b3693 in std::vector<index_list_t, SCP_vm_allocator<index_list_t> >::_M_insert_aux vector.tcc:271
    0000005 0x10006a009 in std::vector<index_list_t, SCP_vm_allocator<index_list_t> >::push_back stl_vector.h:608
    0000006 0x100048536 in pilotfile_convert::csg_import_ships_weapons csg_convert.cpp:181
    0000007 0x100055f5e in pilotfile_convert::csg_import csg_convert.cpp:641
    0000008 0x10006629a in pilotfile_convert::csg_convert csg_convert.cpp:1159
    0000009 0x1000d97ee in convert_pilot_files pilotfile_convert.cpp:143
    0000010 0x1002c88ee in game_init freespace.cpp:2008
    #11 0x10030705b in game_main freespace.cpp:6976
    0000012 0x100308c06 in SDL_main freespace.cpp:7167
    0000013 0x100002b7a in -[SDLMain applicationDidFinishLaunching:] SDLMain.m:300
    0000014 0x7fff9502aed9 in _CFXNotificationPost (in CoreFoundation) + 2553
    0000015 0x7fff8b00de25 in -[NSNotificationCenter postNotificationName:object:userInfo:] (in Foundation) + 63
    0000016 0x7fff9096f55c in -[NSApplication _postDidFinishNotification] (in AppKit) + 291
    0000017 0x7fff9096f295 in -[NSApplication _sendFinishLaunchingNotification] (in AppKit) + 215
    0000018 0x7fff9096c481 in -[NSApplication(NSAppleEventHandling) _handleAEOpenEvent:] (in AppKit) + 565
    0000019 0x7fff9096c07b in -[NSApplication(NSAppleEventHandling) _handleCoreEvent:withReplyEvent:] (in AppKit) + 350
    0000020 0x7fff8b02770a in -[NSAppleEventManager dispatchRawAppleEvent:withRawReply:handlerRefCon:] (in Foundation) + 307
    0000021 0x7fff8b02756c in _NSAppleEventManagerGenericHandler (in Foundation) + 105
    0000022 0x7fff933f0077 in aeDispatchAppleEvent(AEDesc const*, AEDesc*, unsigned int, unsigned char*) (in AE) + 306
    0000023 0x7fff933efed8 in dispatchEventAndSendReply(AEDesc const*, AEDesc*) (in AE) + 36
    0000024 0x7fff933efd98 in aeProcessAppleEvent (in AE) + 317
    0000025 0x7fff8db58708 in AEProcessAppleEvent (in HIToolbox) + 99
    0000026 0x7fff90968865 in _DPSNextEvent (in AppKit) + 1455
    0000027 0x7fff90967e21 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in AppKit) + 127
    0000028 0x7fff9095f1d2 in -[NSApplication run] (in AppKit) + 516
    0000029 0x100004c93 in CustomApplicationMain SDLMain.m:227
Shadow bytes around the buggy address:
  0x1c3a000221f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c3a00022200: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c3a00022210: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c3a00022220: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c3a00022230: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x1c3a00022240: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]
  0x1c3a00022250:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1c3a00022260: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1c3a00022270: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1c3a00022280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1c3a00022290: 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
==9333==ABORTING
TagsNo tags attached.

Activities

Echelon9

2013-04-14 02:05

developer  

Corrupt_player_campaign.zip (6,513 bytes)

Echelon9

2013-04-14 02:07

developer   ~0014923

Have uploaded a player and campaign that triggers this heap overflow.

It is caused by cfread_int(cfp) returning -1, which then passes through the current validity check. At present we only validate that the integer is less than weapon_list_size (being the size of the STL vector).

Echelon9

2013-04-14 02:07

developer  

fix-mantis-2844-csg_convert-heap-buffer-overflow.patch (418 bytes)    
Index: code/pilotfile/csg_convert.cpp
===================================================================
--- code/pilotfile/csg_convert.cpp	(revision 9629)
+++ code/pilotfile/csg_convert.cpp	(working copy)
@@ -431,7 +431,7 @@
 		for (j = 0; j < 3; j++) {
 			i = cfread_int(cfp);
 
-			if (i >= weapon_list_size) {
+			if (i >= weapon_list_size || i < 0) {
 				throw "Data check failure (RedAlert-weapon)!";
 			}
fix-mantis-2844-csg_convert-heap-buffer-overflow.patch (418 bytes)   

niffiwan

2013-04-14 08:48

developer   ~0014924

looks good to me

Echelon9

2013-04-15 13:58

developer   ~0014925

Fix committed to trunk@9630.

Related Changesets

fs2open: trunk r9630

2013-04-15 10:48

Echelon9


Ported: N/A

Details Diff		 Fix for Mantis 2844: AddressSanitizer: heap-buffer-overflow in pilotfile_convert::csg_import_red_alert csg_convert.cpp:438 Affected Issues
0002844
mod - /trunk/fs2_open/code/pilotfile/csg_convert.cpp Diff File

Issue History

Date Modified Username Field Change
2013-04-14 01:55 Echelon9 New Issue
2013-04-14 02:05 Echelon9 File Added: Corrupt_player_campaign.zip
2013-04-14 02:07 Echelon9 Note Added: 0014923
2013-04-14 02:07 Echelon9 File Added: fix-mantis-2844-csg_convert-heap-buffer-overflow.patch
2013-04-14 02:08 Echelon9 Assigned To => chief1983
2013-04-14 02:08 Echelon9 Status new => code review
2013-04-14 08:48 niffiwan Note Added: 0014924
2013-04-15 13:58 Echelon9 Assigned To chief1983 => Echelon9
2013-04-15 13:58 Echelon9 Status code review => assigned
2013-04-15 13:58 Echelon9 Changeset attached => fs2open trunk r9630
2013-04-15 13:58 Echelon9 Note Added: 0014925
2013-04-15 13:58 Echelon9 Status assigned => resolved
2013-04-15 13:58 Echelon9 Resolution open => fixed