2017-11-17 20:54 EST


View Issue Details Jump to Notes ] Related Changesets ]
IDProjectCategoryView StatusLast Update
0002880FSSCPPilot datapublic2013-05-31 18:40
Reporterniffiwan 
Assigned Toniffiwan 
PrioritynormalSeveritymajorReproducibilityalways
StatusresolvedResolutionfixed 
Product Version3.6.19 
Target Version3.7.0Fixed in Version 
Summary0002880: the ship_class of dead/departed wingmen in a red alert mission is not read correctly
DescriptionAs reported by Deepstar:
http://www.hard-light.net/forums/index.php?topic=84619

In a red alert mission, dead/departed wingman with the red-alert-carry flag have their ship_class recorded as -1. This is written to the CSG. However when reading this data back, -1 is used as an index into an array, which results in semi-random memory being used as the ship_class.

The symptoms can either be a crash, or nothing at all, it depends on the contents of the -1 indexed memory.
Steps To ReproduceUse the attached mod.
Select the Red Alert Dead Wing campaign
Press 2/3/4 to destroy relevant wingmen
Press 1 to advance to the next mission
Exit the mission
Exit FSO
Use a hex editor on the CSG to view the 4-byte int starting at the 5th byte after the destroyed wingman's ship name. It should be -1. It will be some random number.
FSO *may* now crash when selecting the pilot, depending on the exact value of the ship_class
Additional InformationAddress Sanitizer helped to find the exact cause of the bug:

$ ./fs2_open_3.6.19_DEBUG_9679_asan 2>&1 | asan_symbolize.py | c++filt
=================================================================
==20176== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6052004ec978 at pc 0xb85395 bp 0x7fffdab5e280 sp 0x7fffdab5e278
READ of size 4 at 0x6052004ec978 thread T0
    #0 0xb85394 in pilotfile::csg_read_redalert() /home/me/src/git-fs2_open/code/pilotfile/csg.cpp:803
    0000001 0xb89fa9 in pilotfile::load_savefile(char const*) /home/me/src/git-fs2_open/code/pilotfile/csg.cpp:1401
    0000002 0x7ff4f5 in mission_campaign_load(char*, player*, int) /home/me/src/git-fs2_open/code/mission/missioncampaign.cpp:665
    0000003 0x8057ab in mission_load_up_campaign(player*) /home/me/src/git-fs2_open/code/mission/missioncampaign.cpp:1671
    0000004 0x41cdb9 in game_enter_state(int, int) /home/me/src/git-fs2_open/code/freespace2/freespace.cpp:5889
    0000005 0x58f46d in gameseq_set_state(int, int) /home/me/src/git-fs2_open/code/gamesequence/gamesequence.cpp:280
    0000006 0x41bb7b in game_process_event(int, int) /home/me/src/git-fs2_open/code/freespace2/freespace.cpp:5165
    0000007 0x58ff47 in gameseq_process_events() /home/me/src/git-fs2_open/code/gamesequence/gamesequence.cpp:395
    0000008 0x41f081 in game_main(char*) /home/me/src/git-fs2_open/code/freespace2/freespace.cpp:7034
    0000009 0x41f535 in main /home/me/src/git-fs2_open/code/freespace2/freespace.cpp:7168
    0000010 0x7fdb70cec76c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226
    #11 0x40b0f8 in _start ??:0
0x6052004ec978 is located 8 bytes to the left of 2048-byte region [0x6052004ec980,0x6052004ed180)
allocated by thread T0 here:
    #0 0x7fdb7325456a in __interceptor_malloc ??:0
    0000001 0xde131a in _vm_malloc(int, char*, int, int) ??:0
    0000002 0xb70949 in SCP_vm_allocator<pilotfile::index_list_t>::allocate(unsigned long) /home/me/src/git-fs2_open/code/./globalincs/vmallocator.h:59
    0000003 0xb7077b in std::_Vector_base<pilotfile::index_list_t, SCP_vm_allocator<pilotfile::index_list_t> >::_M_allocate(unsigned long) /usr/local/gcc-4.8.0/include/c++/4.8.0/bits/stl_vector.h:168
    0000004 0xb6ff6b in std::vector<pilotfile::index_list_t, SCP_vm_allocator<pilotfile::index_list_t> >::_M_insert_aux(__gnu_cxx::__normal_iterator<pilotfile::index_list_t*, std::vector<pilotfile::index_list_t, SCP_vm_allocator<pilotfile::index_list_t> > >, pilotfile::index_list_t const&) /usr/local/gcc-4.8.0/include/c++/4.8.0/bits/vector.tcc:345
    0000005 0xb6f972 in std::vector<pilotfile::index_list_t, SCP_vm_allocator<pilotfile::index_list_t> >::push_back(pilotfile::index_list_t const&) /usr/local/gcc-4.8.0/include/c++/4.8.0/bits/stl_vector.h:913
    0000006 0xb7de65 in pilotfile::csg_read_info() /home/me/src/git-fs2_open/code/pilotfile/csg.cpp:70
    0000007 0xb89f40 in pilotfile::load_savefile(char const*) /home/me/src/git-fs2_open/code/pilotfile/csg.cpp:1386
    0000008 0x7d1e36 in player_select_close() /home/me/src/git-fs2_open/code/menuui/playermenu.cpp:480
    0000009 0x41cb96 in game_leave_state(int, int) /home/me/src/git-fs2_open/code/freespace2/freespace.cpp:5809
    0000010 0x58f31c in gameseq_set_state(int, int) /home/me/src/git-fs2_open/code/gamesequence/gamesequence.cpp:275
    #11 0x41b8b5 in game_process_event(int, int) /home/me/src/git-fs2_open/code/freespace2/freespace.cpp:5070
    0000012 0x58ff47 in gameseq_process_events() /home/me/src/git-fs2_open/code/gamesequence/gamesequence.cpp:395
    0000013 0x41f081 in game_main(char*) /home/me/src/git-fs2_open/code/freespace2/freespace.cpp:7034
    0000014 0x41f535 in main /home/me/src/git-fs2_open/code/freespace2/freespace.cpp:7168
    0000015 0x7fdb70cec76c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226
Shadow bytes around the buggy address:
  0x0c0ac00958d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0ac00958e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0ac00958f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0ac0095900: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0ac0095910: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c0ac0095920: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]
  0x0c0ac0095930:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c0ac0095940: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c0ac0095950: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c0ac0095960: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c0ac0095970: 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
==20176== ABORTING
TagsNo tags attached.
Attached Files
  • 7z file icon mantis2880.7z (10,758 bytes) 2013-05-25 21:53
  • patch file icon mantis2880-svn.patch (2,069 bytes) 2013-05-25 23:57 -
    Index: code/pilotfile/csg.cpp
    ===================================================================
    --- code/pilotfile/csg.cpp	(revision 9680)
    +++ code/pilotfile/csg.cpp	(working copy)
    @@ -800,7 +800,14 @@
     
     		// ship class, index into ship_list[]
     		i = cfread_int(cfp);
    -		ras.ship_class = ship_list[i].index;
    +		if ( (i >= (int)ship_list.size()) || (i < -1) ) {
    +			mprintf(("CSG => Parse Warning: Invalid value for red alert ship index (%d), emptying slot.\n", i));
    +			ras.ship_class = -1;
    +		} else if ( i == -1 ) {  // ship destroyed/exited
    +			ras.ship_class = i;
    +		} else {
    +			ras.ship_class = ship_list[i].index;
    +		}
     
     		// subsystem hits
     		count = cfread_int(cfp);
    @@ -850,8 +857,8 @@
     			ras.secondary_weapons.push_back( weapons );
     		}
     
    -		// this is quite likely a *bad* thing if it happens
    -		if (ras.ship_class >= 0) {
    +		// this is quite likely a *bad* thing if it doesn't happen
    +		if (ras.ship_class >= RED_ALERT_EXITED_SHIP_CLASS) {
     			Red_alert_wingman_status.push_back( ras );
     		}
     	}
    Index: code/missionui/redalert.cpp
    ===================================================================
    --- code/missionui/redalert.cpp	(revision 9680)
    +++ code/missionui/redalert.cpp	(working copy)
    @@ -44,10 +44,6 @@
     //static int Red_alert_num_slots_used = 0;
     static int Red_alert_voice_started;
     
    -#define RED_ALERT_WARN_TIME		4000				// time to warn user that new orders are coming
    -
    -#define RED_ALERT_EXITED_SHIP_CLASS		-1
    -
     SCP_vector<red_alert_ship_status> Red_alert_wingman_status;
     SCP_string Red_alert_precursor_mission;
     
    Index: code/missionui/redalert.h
    ===================================================================
    --- code/missionui/redalert.h	(revision 9680)
    +++ code/missionui/redalert.h	(working copy)
    @@ -34,6 +34,11 @@
     
     // should only ever be defined in redalert.cpp and the pilot file code!!
     #ifdef REDALERT_INTERNAL
    +
    +#define RED_ALERT_WARN_TIME		4000				// time to warn user that new orders are coming
    +
    +#define RED_ALERT_EXITED_SHIP_CLASS		-1
    +
     typedef struct red_alert_ship_status {
     	SCP_string	name;
     	float		hull;
    
    patch file icon mantis2880-svn.patch (2,069 bytes) 2013-05-25 23:57 +

-Relationships
related to 0001326resolvedniffiwan Departed wingmen are displayed as destroyed after red alert 
+Relationships

-Notes

~0015087

FUBAR-BDHR (developer)

This has been around for ages. I set it as a duplicates of the old ticket. Probably a good idea to merge them and close one.

~0015088

niffiwan (developer)

Actually, I'm pretty sure they're slightly different (although related). I'll see about fixing 0001326 after this one - I think it'll require some changes in the red-alert "store" code as well as the "read" code.

~0015108

The_E (administrator)

Patch looks good from here

~0015110

niffiwan (developer)

Fix committed to trunk@9688.
+Notes

+Related Changesets

-Issue History
Date Modified Username Field Change
2013-05-25 21:50 niffiwan New Issue
2013-05-25 21:50 niffiwan Status new => assigned
2013-05-25 21:50 niffiwan Assigned To => niffiwan
2013-05-25 21:53 niffiwan File Added: mantis2880.7z
2013-05-25 22:36 FUBAR-BDHR Relationship added duplicate of 0001326
2013-05-25 22:37 FUBAR-BDHR Note Added: 0015087
2013-05-25 22:47 niffiwan Note Added: 0015088
2013-05-25 22:47 niffiwan Relationship replaced related to 0001326
2013-05-25 23:57 niffiwan File Added: mantis2880-svn.patch
2013-05-25 23:57 niffiwan Status assigned => code review
2013-05-31 17:49 The_E Note Added: 0015108
2013-05-31 18:40 niffiwan Changeset attached => fs2open trunk r9688
2013-05-31 18:40 niffiwan Note Added: 0015110
2013-05-31 18:40 niffiwan Status code review => resolved
2013-05-31 18:40 niffiwan Resolution open => fixed
+Issue History