All Projects FSSCP  wxFRED

View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0002966FSSCPmultiplayerpublic2013-11-29 02:452013-12-03 15:15
Reporterchief1983 Assigned Tozookeeper  
PrioritynormalSeveritycrashReproducibilityalways
Status resolvedResolutionfixed 
Platformx86OSGentoo LinuxOS VersionKernel 3.5.7
Product Version3.7.1 
Target Version3.7.2 
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.

Activities

Echelon9

2013-12-01 00:51

developer   ~0015469

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)++;
            }
        }
    }

Echelon9

2013-12-01 01:14

developer  

fix-mantis-2966.patch (729 bytes)    
Index: code/hud/hudescort.cpp
===================================================================
--- code/hud/hudescort.cpp	(revision 10174)
+++ code/hud/hudescort.cpp	(working copy)
@@ -557,7 +557,7 @@
 			}		
 
 			// is this a valid player			
-			if(MULTI_CONNECTED(Net_players[idx]) && !MULTI_OBSERVER(Net_players[idx]) && !MULTI_STANDALONE(Net_players[idx])){
+			if(MULTI_CONNECTED(Net_players[idx]) && !MULTI_OBSERVER(Net_players[idx]) && !MULTI_STANDALONE(Net_players[idx]) && (Net_players[idx].m_player != NULL) && (Net_players[idx].m_player->objnum >= 0) && (Net_players[idx].m_player->objnum < MAX_OBJECTS)){
 				// add the ship
 				escorts[*num_escorts].objnum = -1;
 				escorts[*num_escorts].obj_signature = -1;
fix-mantis-2966.patch (729 bytes)   

Echelon9

2013-12-01 01:16

developer   ~0015470

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.

zookeeper

2013-12-03 09:57

developer   ~0015490

Fix committed to trunk@10185.

Goober5000

2013-12-03 15:15

administrator   ~0015499

credit zookeeper

Related Changesets

fs2open: trunk r10185

2013-12-03 05:27

zookeeper


Ported: N/A

Details Diff		 Replaced shield_hit_info in escort_info with its own simple set of timer variables. Fixes Mantis 0002966 and makes the code simpler and safer. Affected Issues
0002966
mod - /trunk/fs2_open/code/hud/hudescort.cpp Diff File

Issue History

Date Modified Username Field Change
2013-11-29 02:45 chief1983 New Issue
2013-11-29 02:45 chief1983 Status new => assigned
2013-11-29 02:45 chief1983 Assigned To => zookeeper
2013-12-01 00:51 Echelon9 Note Added: 0015469
2013-12-01 01:14 Echelon9 File Added: fix-mantis-2966.patch
2013-12-01 01:16 Echelon9 Note Added: 0015470
2013-12-01 01:16 Echelon9 Assigned To zookeeper => Echelon9
2013-12-01 01:16 Echelon9 Assigned To Echelon9 => chief1983
2013-12-01 01:16 Echelon9 Status assigned => code review
2013-12-01 01:23 Echelon9 Summary Standalone AddressSanitizer crash => Standalone AddressSanitizer: global-buffer-overflow in shield_info_reset() - relates to hud_create_complete_escort_list()
2013-12-03 09:57 zookeeper Changeset attached => fs2open trunk r10185
2013-12-03 09:57 zookeeper Note Added: 0015490
2013-12-03 09:57 zookeeper Status code review => resolved
2013-12-03 09:57 zookeeper Resolution open => fixed
2013-12-03 15:15 Goober5000 Note Added: 0015499
2013-12-03 15:15 Goober5000 Assigned To chief1983 => zookeeper