2018-06-21 20:23 EDT


View Issue Details Jump to Notes ] Related Changesets ]
IDProjectCategoryView StatusLast Update
0002855FSSCPPilot datapublic2013-04-30 03:42
Reporterniffiwan 
Assigned Toniffiwan 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
Product Version3.6.19 
Target Version3.7.0Fixed in Version 
Summary0002855: off-by-one error in csg_read_loadout clamp code
DescriptionIn the following code, the index starts at zero, the vector.size() starts at 1. Therefore the > should be >= and the assignment should be vector.size() - 1 (or perhaps 0 in this case since ship index 0 is more likely to be a player ship/fighter than the highest index).

        if (idx > ship_list.size()) {
            //mprintf(("CSG => Parse Warning: Invalid value for ship index (%d), Clamping to valid range.\n", idx));
             idx = ship_list.size();
         }


Repeat these changes for primary & secondary weapon index clamp code.
Additional InformationDepends on 2753 only because 2753 changes the idx var name
TagsNo tags attached.
Attached Files
  • patch file icon mantis2855-svn.patch (1,838 bytes) 2013-04-29 04:51 -
    Index: code/pilotfile/csg.cpp
    ===================================================================
    --- code/pilotfile/csg.cpp	(revision 9657)
    +++ code/pilotfile/csg.cpp	(working copy)
    @@ -556,9 +556,9 @@
     		// ship
     		ship_idx = cfread_int(cfp);
     
    -		if (ship_idx > (int)ship_list.size()) { // on the casts, assume that ship & weapon lists will never exceed ~2 billion
    -			//mprintf(("CSG => Parse Warning: Invalid value for ship index (%d), Clamping to valid range.\n", ship_idx));
    -			 ship_idx = ship_list.size();
    +		if ( (ship_idx >= (int)ship_list.size()) || (ship_idx < -1) ) { // on the casts, assume that ship & weapon lists will never exceed ~2 billion
    +			mprintf(("CSG => Parse Warning: Invalid value for ship index (%d), emptying slot.\n", ship_idx));
    +			ship_idx = -1;
     		}
     
     		if (slot) {
    @@ -575,9 +575,9 @@
     		for (j = 0; j < count; j++) {
     			wep_idx = cfread_int(cfp);
     
    -			if (wep_idx > (int)weapon_list.size()) {
    -				//mprintf(("CSG => Parse Warning: Invalid value for weapon index (%d), Clamping to valid range.\n", wep_idx));
    -				wep_idx = weapon_list.size();
    +			if ( (wep_idx >= (int)weapon_list.size()) || (wep_idx < -1) ) {
    +				mprintf(("CSG => Parse Warning: Invalid value for primary weapon index (%d), emptying slot.\n", wep_idx));
    +				wep_idx = -1;
     			}
     
     
    @@ -602,9 +602,9 @@
     		for (j = 0; j < count; j++) {
     			wep_idx = cfread_int(cfp);
     
    -			if (wep_idx > (int)weapon_list.size()) {
    -				//mprintf(("CSG => Parse Warning: Invalid value for weapon index (%d), Clamping to valid range.\n", wep_idx));
    -				wep_idx = weapon_list.size();
    +			if ( (wep_idx >= (int)weapon_list.size()) || (wep_idx < -1) ) {
    +				mprintf(("CSG => Parse Warning: Invalid value for secondary weapon index (%d), emptying slot.\n", wep_idx));
    +				wep_idx = -1;
     			}
     
     			if ( slot && (j < MAX_SHIP_SECONDARY_BANKS) ) {
    
    patch file icon mantis2855-svn.patch (1,838 bytes) 2013-04-29 04:51 +

-Relationships
related to 0002753resolvedZacam Custom loadouts not loaded 
+Relationships

-Notes

~0014976

niffiwan (developer)

Patch is ready, please review.

In addition to fixing the off-by-one error, I
1) added bounds checking that should have been added in r9657
2) emptied ship/weapon saved loadout slots rather than assign values that may not make sense (i.e. max ship idx = VolitionBravos? max wep idx = S_Cluster Bomb Baby?)
3) enabled mprintfs

On 2) I had a corrupted pilotfile, when testing, the empty wingmen loadout slots were ignored & the wingmen got the default mission loadout.

~0014989

Zacam (administrator)

I had a chance to look this over as part of the patch for 2753, but fo due diligence, tested the included patch here. Looks fine.

~0014995

niffiwan (developer)

Fix committed to trunk@9664.
+Notes

+Related Changesets

-Issue History
Date Modified Username Field Change
2013-04-28 17:10 niffiwan New Issue
2013-04-28 17:10 niffiwan Status new => assigned
2013-04-28 17:10 niffiwan Assigned To => niffiwan
2013-04-28 17:11 niffiwan Relationship added related to 0002753
2013-04-29 04:51 niffiwan File Added: mantis2855-svn.patch
2013-04-29 04:59 niffiwan Note Added: 0014976
2013-04-29 04:59 niffiwan Status assigned => code review
2013-04-30 02:54 Zacam Note Added: 0014989
2013-04-30 03:42 niffiwan Changeset attached => fs2open trunk r9664
2013-04-30 03:42 niffiwan Note Added: 0014995
2013-04-30 03:42 niffiwan Status code review => resolved
2013-04-30 03:42 niffiwan Resolution open => fixed
+Issue History