2020-07-11 19:38 EDT
All Projects | FSSCP: wxFRED

  

View Issue Details [ Jump to Notes ] [ Issue History ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0002314FSSCPsoundpublic2010-09-26 11:092010-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)

2010-09-26 11:10

 Proposed patch attached.

~0012364

The_E (administrator)

2010-09-26 11:38

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

~0012365

The_E (administrator)

2010-09-26 11:58

 Fixed in revision 6524

~0012366

Echelon9 (developer)

2010-09-26 12:02

 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