Source Code Project Mantis - FSSCP
View Issue Details
0002966FSSCPmultiplayerpublic2013-11-28 21:452013-12-03 10:15
Reporterchief1983 
Assigned Tozookeeper 
PrioritynormalSeveritycrashReproducibilityalways
StatusresolvedResolutionfixed 
Platformx86OSGentoo LinuxOS VersionKernel 3.5.7
Product Version3.7.1 
Target Version3.7.2Fixed in Version 
Summary0002966: Standalone AddressSanitizer: global-buffer-overflow in shield_info_reset() - relates to hud_create_complete_escort_list()
DescriptionLooks like this is from the new shield code as well, which zookeeper is frantically trying to keep from having memory issues :)
Steps To ReproduceConnect a client to a *nix standalone compiled with AddressSanitizer enabled.
Load up Asteroid Dogfight (or probably any dogfight, they only require one client)
Hit launch, accept that stats won't be saved if it asks.
Standalone crashes.
Additional Information=================================================================
==6298== ERROR: AddressSanitizer: global-buffer-overflow on address 0x0940c8a8 at pc 0x82fc29a bp 0xbf9c1508 sp 0xbf9c14fc
READ of size 4 at 0x0940c8a8 thread T0
    #0 0x82fc299 in shield_info_reset(object*, shield_hit_info*) /home/chief1983/fs2_open_build/code/hud/hudshield.cpp:485
    0000001 0x82770f6 in hud_create_complete_escort_list(escort_info*, int*) /home/chief1983/fs2_open_build/code/hud/hudescort.cpp:566
    0000002 0x8277a50 in hud_setup_escort_list(int) /home/chief1983/fs2_open_build/code/hud/hudescort.cpp:648
    0000003 0x8053b3f in game_post_level_init() /home/chief1983/fs2_open_build/code/freespace2/freespace.cpp:1370
    0000004 0x8053f10 in game_start_mission() /home/chief1983/fs2_open_build/code/freespace2/freespace.cpp:1464
    0000005 0x8605654 in multi_sync_pre_do() /home/chief1983/fs2_open_build/code/network/multiui.cpp:8036
    0000006 0x8602bc6 in multi_sync_do() /home/chief1983/fs2_open_build/code/network/multiui.cpp:7468
    0000007 0x8064303 in game_do_state(int) /home/chief1983/fs2_open_build/code/freespace2/freespace.cpp:6687
    0000008 0x81c54ee in gameseq_process_events() /home/chief1983/fs2_open_build/code/gamesequence/gamesequence.cpp:409
    0000009 0x8065468 in game_main(char*) /home/chief1983/fs2_open_build/code/freespace2/freespace.cpp:7061
    0000010 0x806589d in main /home/chief1983/fs2_open_build/code/freespace2/freespace.cpp:7195
    #11 0xb5bdd595 in ?? ??:0
0x0940c8a8 is located 4 bytes to the right of global variable 'Viewer_obj (object/object.cpp)' (0x940c8a0) of size 4
  'Viewer_obj (object/object.cpp)' is ascii string ''
0x0940c8a8 is located 56 bytes to the left of global variable 'Objects (object/object.cpp)' (0x940c8e0) of size 1778000
Shadow bytes around the buggy address:
  0x212818c0: 00 00 00 04 f9 f9 f9 f9 00 00 00 00 00 00 00 00
  0x212818d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x212818e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x212818f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x21281900: 00 00 00 00 00 00 00 04 f9 f9 f9 f9 04 f9 f9 f9
=>0x21281910: f9 f9 f9 f9 04[f9]f9 f9 f9 f9 f9 f9 00 00 00 00
  0x21281920: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x21281930: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x21281940: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x21281950: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x21281960: 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
==6298== ABORTING
TagsNo tags attached.
Attached Filespatch fix-mantis-2966.patch (729) 2013-11-30 20:14
http://scp.indiegames.us/mantis/file_download.php?file_id=2301&type=bug

Notes
(0015469)
Echelon9   
2013-11-30 19:51   
Likely caused by a negative value returned by Net_players[idx].m_player->objnum being used as an index into Objects[] in hud_create_complete_escort_list(), and then passed as a pointer to shield_info_reset().

---

// create complete priority sorted escort list for all active ships
// escorts - array of escort info
// num_escorts - number of escorts requests in field of active ships
// This will be culled to MAX_ESCORTS, selecting the top set from escorts
void hud_create_complete_escort_list(escort_info *escorts, int *num_escorts)
{
    ship_obj *so;
    object *objp;

    // start with none on list
    *num_escorts = 0;

    int idx;

    // multiplayer dogfight
    if(MULTI_DOGFIGHT){
        for(idx=0; idx<MAX_PLAYERS; idx++){
            ...

            // is this a valid player
            if(MULTI_CONNECTED(Net_players[idx]) && !MULTI_OBSERVER(Net_players[idx]) && !MULTI_STANDALONE(Net_players[idx])){
                // add the ship
                escorts[*num_escorts].objnum = -1;
                escorts[*num_escorts].obj_signature = -1;
                escorts[*num_escorts].priority = -1;
                escorts[*num_escorts].np_id = Net_players[idx].player_id;
                shield_info_reset(&Objects[Net_players[idx].m_player->objnum], &escorts[*num_escorts].hit_info); <======= HERE
                (*num_escorts)++;
            }
        }
    }
(0015470)
Echelon9   
2013-11-30 20:16   
Resolution patch attached.

It is passed on utilising the broader validity check present in multiutil.cpp in a few locations for valid Net_players.

As a subsequent step, it might be helpful to turn that combination of checks into a single macro. Then replace the various uses of it, or near checks, throughout the code so we don't run into it again.
(0015490)
zookeeper   
2013-12-03 04:57   
Fix committed to trunk@10185.
(0015499)
Goober5000   
2013-12-03 10:15   
credit zookeeper

Issue History
2013-11-28 21:45chief1983New Issue
2013-11-28 21:45chief1983Statusnew => assigned
2013-11-28 21:45chief1983Assigned To => zookeeper
2013-11-30 19:51Echelon9Note Added: 0015469
2013-11-30 20:14Echelon9File Added: fix-mantis-2966.patch
2013-11-30 20:16Echelon9Note Added: 0015470
2013-11-30 20:16Echelon9Assigned Tozookeeper => Echelon9
2013-11-30 20:16Echelon9Assigned ToEchelon9 => chief1983
2013-11-30 20:16Echelon9Statusassigned => code review
2013-11-30 20:23Echelon9SummaryStandalone AddressSanitizer crash => Standalone AddressSanitizer: global-buffer-overflow in shield_info_reset() - relates to hud_create_complete_escort_list()
2013-12-03 04:57zookeeperChangeset attached => fs2open trunk r10185
2013-12-03 04:57zookeeperNote Added: 0015490
2013-12-03 04:57zookeeperStatuscode review => resolved
2013-12-03 04:57zookeeperResolutionopen => fixed
2013-12-03 10:15Goober5000Note Added: 0015499
2013-12-03 10:15Goober5000Assigned Tochief1983 => zookeeper