2019-10-21 16:12 EDT


View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0002314FSSCPsoundpublic2010-09-26 12:02
ReporterEchelon9 
Assigned ToEchelon9 
PrioritynormalSeveritycrashReproducibilityrandom
StatusresolvedResolutionfixed 
Product Version3.6.13 
Target VersionFixed in Version3.6.13 
Summary0002314: Write outside the aav_data[3] array via an off-by-one
DescriptionPossible 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 InformationIntroduction of the SEXP here http://www.hard-light.net/forums/index.php?topic=71019
TagsNo tags attached.
Attached Files
  • patch file icon mantis-2314.patch (538 bytes) 2010-09-26 11:10 -
    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;
    
    patch file icon mantis-2314.patch (538 bytes) 2010-09-26 11:10 +

-Relationships
+Relationships

-Notes

~0012363

Echelon9 (developer)

Proposed patch attached.

~0012364

The_E (administrator)

Yeah, sorry, my bad. Forgot to remove some bits from when I was still developing this.

~0012365

The_E (administrator)

Fixed in revision 6524

~0012366

Echelon9 (developer)

Resolved, with slightly modified patch in r6524 http://svn.icculus.org/fs2open?view=rev&revision=6524
+Notes

-Issue History
Date Modified Username Field Change
2010-09-26 11:09 Echelon9 New Issue
2010-09-26 11:09 Echelon9 Status new => assigned
2010-09-26 11:09 Echelon9 Assigned To => Echelon9
2010-09-26 11:10 Echelon9 File Added: mantis-2314.patch
2010-09-26 11:10 Echelon9 Note Added: 0012363
2010-09-26 11:38 The_E Note Added: 0012364
2010-09-26 11:58 The_E Note Added: 0012365
2010-09-26 11:59 The_E Status assigned => resolved
2010-09-26 11:59 The_E Fixed in Version => 3.6.13
2010-09-26 11:59 The_E Resolution open => fixed
2010-09-26 12:02 Echelon9 Note Added: 0012366
+Issue History