View Issue Details

IDProjectCategoryView StatusLast Update
0002853FSSCPpublic2013-04-26 10:47
ReporterEchelon9 Assigned ToEchelon9  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.6.19 
Target Version3.7.0 
Summary0002853: AddressSanitizer: heap-buffer-overflow in change_ship_type() ship.cpp:9213
Descriptioneported by AddressSanitizer, a memory error detector for C/C++, in FS2Open builds based on trunk r9646.

ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60600010d2ec at pc 0x10216d7f5 bp 0x7fff5fbfa3d0 sp 0x7fff5fbfa3c8
READ of size 4 at 0x60600010d2ec thread T0
    #0 0x10216d7f4 in change_ship_type ship.cpp:9213
    0000001 0x1013cc363 in update_player_ship missionshipchoice.cpp:2493
    0000002 0x1013c8929 in create_wings missionshipchoice.cpp:2380
    0000003 0x1013c2622 in commit_pressed missionshipchoice.cpp:1872
    0000004 0x1013bc97d in ship_select_do missionshipchoice.cpp:1514
    0000005 0x100305748 in game_do_state freespace.cpp:6515
    0000006 0x100900c18 in gameseq_process_events gamesequence.cpp:405
    0000007 0x10030bed6 in game_main freespace.cpp:7034
    0000008 0x10030d574 in SDL_main freespace.cpp:7168
    ...
0x60600010d2ec is located 20 bytes to the left of 64-byte region [0x60600010d300,0x60600010d340)
allocated by thread T0 here:
    #0 0x10553ae05 in wrap_malloc (in libclang_rt.asan_osx_dynamic.dylib) + 53
    0000001 0x102650ab8 in _vm_malloc stubs.cpp:571
    0000002 0x101499667 in read_model_file modelread.cpp:1888
    0000003 0x1014aa0ef in model_load modelread.cpp:2517
    0000004 0x1013d5089 in ss_load_icons missionshipchoice.cpp:2845
    0000005 0x1013d5980 in ss_load_all_icons missionshipchoice.cpp:2866
    0000006 0x1013da359 in ship_select_common_init missionshipchoice.cpp:3198
    0000007 0x1013eb5f4 in common_select_init missionscreencommon.cpp:493
    0000008 0x1014463db in brief_init missionbrief.cpp:889
    0000009 0x100301ffc in game_enter_state freespace.cpp:5965
    0000010 0x1008ff1fd in gameseq_set_state gamesequence.cpp:280
    #11 0x1002fcbc2 in game_process_event freespace.cpp:5110
    0000012 0x100900a7e in gameseq_process_events gamesequence.cpp:395
    0000013 0x10030bed6 in game_main freespace.cpp:7034
    0000014 0x10030d574 in SDL_main freespace.cpp:7168

        for (int h = 0; h < pm->n_thrusters; h++)
        {
            for (int j = 0; j < pm->thrusters->num_points; j++) <==
            {
               ...
                // only make ab trails for thrusters that are pointing backwards
                if (pm->thrusters[h].points[j].norm.xyz.z > -0.5)
                    continue;
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, change ship type pre-mission start.
Additional InformationERROR: AddressSanitizer: heap-buffer-overflow on address 0x60600010d2ec at pc 0x10216d7f5 bp 0x7fff5fbfa3d0 sp 0x7fff5fbfa3c8
READ of size 4 at 0x60600010d2ec thread T0
    #0 0x10216d7f4 in change_ship_type ship.cpp:9213
    0000001 0x1013cc363 in update_player_ship missionshipchoice.cpp:2493
    0000002 0x1013c8929 in create_wings missionshipchoice.cpp:2380
    0000003 0x1013c2622 in commit_pressed missionshipchoice.cpp:1872
    0000004 0x1013bc97d in ship_select_do missionshipchoice.cpp:1514
    0000005 0x100305748 in game_do_state freespace.cpp:6515
    0000006 0x100900c18 in gameseq_process_events gamesequence.cpp:405
    0000007 0x10030bed6 in game_main freespace.cpp:7034
    0000008 0x10030d574 in SDL_main freespace.cpp:7168
    0000009 0x1000030d3 in -[SDLMain applicationDidFinishLaunching:] SDLMain.m:300
    0000010 0x7fff8e0f0ed9 in _CFXNotificationPost (in CoreFoundation) + 2553
    #11 0x7fff9019ee25 in -[NSNotificationCenter postNotificationName:object:userInfo:] (in Foundation) + 63
    0000012 0x7fff8ab3355c in -[NSApplication _postDidFinishNotification] (in AppKit) + 291
    0000013 0x7fff8ab33295 in -[NSApplication _sendFinishLaunchingNotification] (in AppKit) + 215
    0000014 0x7fff8ab30481 in -[NSApplication(NSAppleEventHandling) _handleAEOpenEvent:] (in AppKit) + 565
    0000015 0x7fff8ab3007b in -[NSApplication(NSAppleEventHandling) _handleCoreEvent:withReplyEvent:] (in AppKit) + 350
    0000016 0x7fff901b870a in -[NSAppleEventManager dispatchRawAppleEvent:withRawReply:handlerRefCon:] (in Foundation) + 307
    0000017 0x7fff901b856c in _NSAppleEventManagerGenericHandler (in Foundation) + 105
    0000018 0x7fff8f100077 in aeDispatchAppleEvent(AEDesc const*, AEDesc*, unsigned int, unsigned char*) (in AE) + 306
    0000019 0x7fff8f0ffed8 in dispatchEventAndSendReply(AEDesc const*, AEDesc*) (in AE) + 36
    0000020 0x7fff8f0ffd98 in aeProcessAppleEvent (in AE) + 317
    0000021 0x7fff8d126708 in AEProcessAppleEvent (in HIToolbox) + 99
    0000022 0x7fff8ab2c865 in _DPSNextEvent (in AppKit) + 1455
    0000023 0x7fff8ab2be21 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in AppKit) + 127
    0000024 0x7fff8ab231d2 in -[NSApplication run] (in AppKit) + 516
    0000025 0x100005232 in CustomApplicationMain SDLMain.m:227
    0000026 0x100004c9f in main SDLMain.m:377
    0000027 0x100001df3 in start (in FS2_Open (debug)) + 51
    0000028 0x0 in 0x0
0x60600010d2ec is located 20 bytes to the left of 64-byte region [0x60600010d300,0x60600010d340)
allocated by thread T0 here:
    #0 0x10553ae05 in wrap_malloc (in libclang_rt.asan_osx_dynamic.dylib) + 53
    0000001 0x102650ab8 in _vm_malloc stubs.cpp:571
    0000002 0x101499667 in read_model_file modelread.cpp:1888
    0000003 0x1014aa0ef in model_load modelread.cpp:2517
    0000004 0x1013d5089 in ss_load_icons missionshipchoice.cpp:2845
    0000005 0x1013d5980 in ss_load_all_icons missionshipchoice.cpp:2866
    0000006 0x1013da359 in ship_select_common_init missionshipchoice.cpp:3198
    0000007 0x1013eb5f4 in common_select_init missionscreencommon.cpp:493
    0000008 0x1014463db in brief_init missionbrief.cpp:889
    0000009 0x100301ffc in game_enter_state freespace.cpp:5965
    0000010 0x1008ff1fd in gameseq_set_state gamesequence.cpp:280
    #11 0x1002fcbc2 in game_process_event freespace.cpp:5110
    0000012 0x100900a7e in gameseq_process_events gamesequence.cpp:395
    0000013 0x10030bed6 in game_main freespace.cpp:7034
    0000014 0x10030d574 in SDL_main freespace.cpp:7168
    0000015 0x1000030d3 in -[SDLMain applicationDidFinishLaunching:] SDLMain.m:300
    0000016 0x7fff8e0f0ed9 in _CFXNotificationPost (in CoreFoundation) + 2553
    0000017 0x7fff9019ee25 in -[NSNotificationCenter postNotificationName:object:userInfo:] (in Foundation) + 63
    0000018 0x7fff8ab3355c in -[NSApplication _postDidFinishNotification] (in AppKit) + 291
    0000019 0x7fff8ab33295 in -[NSApplication _sendFinishLaunchingNotification] (in AppKit) + 215
    0000020 0x7fff8ab30481 in -[NSApplication(NSAppleEventHandling) _handleAEOpenEvent:] (in AppKit) + 565
    0000021 0x7fff8ab3007b in -[NSApplication(NSAppleEventHandling) _handleCoreEvent:withReplyEvent:] (in AppKit) + 350
    0000022 0x7fff901b870a in -[NSAppleEventManager dispatchRawAppleEvent:withRawReply:handlerRefCon:] (in Foundation) + 307
    0000023 0x7fff901b856c in _NSAppleEventManagerGenericHandler (in Foundation) + 105
    0000024 0x7fff8f100077 in aeDispatchAppleEvent(AEDesc const*, AEDesc*, unsigned int, unsigned char*) (in AE) + 306
    0000025 0x7fff8f0ffed8 in dispatchEventAndSendReply(AEDesc const*, AEDesc*) (in AE) + 36
    0000026 0x7fff8f0ffd98 in aeProcessAppleEvent (in AE) + 317
    0000027 0x7fff8d126708 in AEProcessAppleEvent (in HIToolbox) + 99
    0000028 0x7fff8ab2c865 in _DPSNextEvent (in AppKit) + 1455
    0000029 0x7fff8ab2be21 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in AppKit) + 127
Shadow bytes around the buggy address:
  0x1c0c00021a00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c0c00021a10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c0c00021a20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c0c00021a30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c0c00021a40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x1c0c00021a50: fa fa fa fa 00 00 00 00 00 00 00 fa fa[fa]fa fa
  0x1c0c00021a60: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
  0x1c0c00021a70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c0c00021a80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c0c00021a90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c0c00021aa0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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 right 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
==23754==ABORTING
TagsNo tags attached.

Activities

Echelon9

2013-04-26 10:46

developer   ~0014959

This is a bad bug as it will lead to the overwrite of adjacent heap blocks because we assume all pm->thrusters have the same number of glowpoints as the first on in the array.

May lead to a range of subtle and hard to track down bugs any time a user changes ship pre-mission.

Echelon9

2013-04-26 10:47

developer   ~0014960

Fix committed to trunk@9648.

Related Changesets

fs2open: trunk r9648

2013-04-26 07:39

Echelon9


Ported: N/A

Details Diff
Fix Mantis 2853: AddressSanitizer: heap-buffer-overflow in change_ship_type() Affected Issues
0002853
mod - /trunk/fs2_open/code/ship/ship.cpp Diff File

Issue History

Date Modified Username Field Change
2013-04-26 10:44 Echelon9 New Issue
2013-04-26 10:44 Echelon9 Status new => assigned
2013-04-26 10:44 Echelon9 Assigned To => Echelon9
2013-04-26 10:46 Echelon9 Note Added: 0014959
2013-04-26 10:47 Echelon9 Changeset attached => fs2open trunk r9648
2013-04-26 10:47 Echelon9 Note Added: 0014960
2013-04-26 10:47 Echelon9 Status assigned => resolved
2013-04-26 10:47 Echelon9 Resolution open => fixed