View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002880 | FSSCP | Pilot data | public | 2013-05-26 01:50 | 2013-05-31 22:40 |
Reporter | niffiwan | Assigned To | niffiwan | ||
Priority | normal | Severity | major | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Product Version | 3.6.19 | ||||
Target Version | 3.7.0 | ||||
Summary | 0002880: the ship_class of dead/departed wingmen in a red alert mission is not read correctly | ||||
Description | As 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 Reproduce | Use 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 Information | Address 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 | ||||
Tags | No tags attached. | ||||
|
|
|
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. |
|
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. |
|
mantis2880-svn.patch (2,069 bytes)
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 looks good from here |
|
Fix committed to trunk@9688. |
fs2open: trunk r9688 2013-05-31 19:38 Ported: N/A Details Diff |
Fix for mantis 2880: handle -1 red alert departed ship indexes |
Affected Issues 0002880 |
|
mod - /trunk/fs2_open/code/missionui/redalert.cpp | Diff File | ||
mod - /trunk/fs2_open/code/missionui/redalert.h | Diff File | ||
mod - /trunk/fs2_open/code/pilotfile/csg.cpp | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2013-05-26 01:50 | niffiwan | New Issue | |
2013-05-26 01:50 | niffiwan | Status | new => assigned |
2013-05-26 01:50 | niffiwan | Assigned To | => niffiwan |
2013-05-26 01:53 | niffiwan | File Added: mantis2880.7z | |
2013-05-26 02:36 | FUBAR-BDHR | Relationship added | duplicate of 0001326 |
2013-05-26 02:37 | FUBAR-BDHR | Note Added: 0015087 | |
2013-05-26 02:47 | niffiwan | Note Added: 0015088 | |
2013-05-26 02:47 | niffiwan | Relationship replaced | related to 0001326 |
2013-05-26 03:57 | niffiwan | File Added: mantis2880-svn.patch | |
2013-05-26 03:57 | niffiwan | Status | assigned => code review |
2013-05-31 21:49 | The_E | Note Added: 0015108 | |
2013-05-31 22:40 | niffiwan | Changeset attached | => fs2open trunk r9688 |
2013-05-31 22:40 | niffiwan | Note Added: 0015110 | |
2013-05-31 22:40 | niffiwan | Status | code review => resolved |
2013-05-31 22:40 | niffiwan | Resolution | open => fixed |