View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002855 | FSSCP | Pilot data | public | 2013-04-28 21:10 | 2013-04-30 07:42 |
| Reporter | niffiwan | Assigned To | niffiwan | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | 3.6.19 | ||||
| Target Version | 3.7.0 | ||||
| Summary | 0002855: off-by-one error in csg_read_loadout clamp code | ||||
| Description | In 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 Information | Depends on 2753 only because 2753 changes the idx var name | ||||
| Tags | No tags attached. | ||||
|
|
mantis2855-svn.patch (1,838 bytes)
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 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. |
|
|
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. |
|
|
Fix committed to trunk@9664. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-04-28 21:10 | niffiwan | New Issue | |
| 2013-04-28 21:10 | niffiwan | Status | new => assigned |
| 2013-04-28 21:10 | niffiwan | Assigned To | => niffiwan |
| 2013-04-28 21:11 | niffiwan | Relationship added | related to 0002753 |
| 2013-04-29 08:51 | niffiwan | File Added: mantis2855-svn.patch | |
| 2013-04-29 08:59 | niffiwan | Note Added: 0014976 | |
| 2013-04-29 08:59 | niffiwan | Status | assigned => code review |
| 2013-04-30 06:54 | Zacam | Note Added: 0014989 | |
| 2013-04-30 07:42 | niffiwan | Changeset attached | => fs2open trunk r9664 |
| 2013-04-30 07:42 | niffiwan | Note Added: 0014995 | |
| 2013-04-30 07:42 | niffiwan | Status | code review => resolved |
| 2013-04-30 07:42 | niffiwan | Resolution | open => fixed |
