View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002314 | FSSCP | sound | public | 2010-09-26 15:09 | 2010-09-26 16:02 |
| Reporter | Echelon9 | Assigned To | Echelon9 | ||
| Priority | normal | Severity | crash | Reproducibility | random |
| Status | resolved | Resolution | fixed | ||
| Product Version | 3.6.13 | ||||
| Fixed in Version | 3.6.13 | ||||
| Summary | 0002314: Write outside the aav_data[3] array via an off-by-one | ||||
| Description | Possible problem with the adjust-audio-volume SEXP code is that the snd_adjust_audio_volume(int type, float percent, int time) function can currently attempt to write outside the aav_data[3] array via an off-by-one. The debug Assert() check is for an index into that array of less than 4, although as an array starting at zero the three elements have indices 0, 1 and 2. The one call to snd_adjust_audio_volume() in sexp_adjust_audio_volume() uses the audio_volume_option_lookup() function to pick the 'type'. This has a comment "\t1:\tSound Type to adjust, either Master, Music, Voice or Effects\r\n" i.e. 4 elements even though the SEXP doesn't appear to offer the ability to adjust the Master volume, which may be the cause of the confusion. audio_volume_option_lookup() can also return -1, which is an error code not checked for (on non-Debug), and would have likewise lead to an out of bounds write to the aav_data array. | ||||
| Additional Information | Introduction of the SEXP here http://www.hard-light.net/forums/index.php?topic=71019 | ||||
| Tags | No tags attached. | ||||
|
2010-09-26 15:10
|
mantis-2314.patch (538 bytes)
Index: code/sound/sound.cpp
===================================================================
--- code/sound/sound.cpp (revision 6523)
+++ code/sound/sound.cpp (working copy)
@@ -1396,7 +1396,7 @@
void snd_adjust_audio_volume(int type, float percent, int time)
{
- Assert( type >= 0 && type < 4 );
+ Assert( type >= 0 && type < 3 );
switch (type) {
case AAV_MUSIC:
@@ -1420,6 +1420,8 @@
else
aav_data[type].delta = percent - aav_effect_volume;
break;
+ default:
+ Int3();
}
aav_data[type].delta_time = time;
|
|
|
Proposed patch attached. |
|
|
Yeah, sorry, my bad. Forgot to remove some bits from when I was still developing this. |
|
|
Fixed in revision 6524 |
|
|
Resolved, with slightly modified patch in r6524 http://svn.icculus.org/fs2open?view=rev&revision=6524 |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2010-09-26 15:09 | Echelon9 | New Issue | |
| 2010-09-26 15:09 | Echelon9 | Status | new => assigned |
| 2010-09-26 15:09 | Echelon9 | Assigned To | => Echelon9 |
| 2010-09-26 15:10 | Echelon9 | File Added: mantis-2314.patch | |
| 2010-09-26 15:10 | Echelon9 | Note Added: 0012363 | |
| 2010-09-26 15:38 | The_E | Note Added: 0012364 | |
| 2010-09-26 15:58 | The_E | Note Added: 0012365 | |
| 2010-09-26 15:59 | The_E | Status | assigned => resolved |
| 2010-09-26 15:59 | The_E | Fixed in Version | => 3.6.13 |
| 2010-09-26 15:59 | The_E | Resolution | open => fixed |
| 2010-09-26 16:02 | Echelon9 | Note Added: 0012366 |
