Cleanup on bug 3894 - Fuzzing crashes for SDL_LoadWAV Simon Hug Attached is a minor cleanup patch. It changes the option name of one hint to something better, puts one or two more checks in, and adds explicit casting where warnings could appear otherwise. I hope the naming of the hints and their options is acceptable. It would be kind of awkward to change them after they get released with an official SDL version.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356 357 358 359 360 361 362 363 364 365 366 367 368 369 370 371 372
diff --git a/include/SDL_hints.h b/include/SDL_hints.h
index 4c83dd3..b5d9d8d 100644
--- a/include/SDL_hints.h
+++ b/include/SDL_hints.h
@@ -1135,8 +1135,8 @@ extern "C" {
*
* This variable can be set to the following values:
*
- * "chunksearch" - Use the RIFF chunk size as a boundary for the chunk search
- * "ignorezero" - Like "chunksearch", but a zero size searches up to 4 GiB (default)
+ * "force" - Always use the RIFF chunk size as a boundary for the chunk search
+ * "ignorezero" - Like "force", but a zero size searches up to 4 GiB (default)
* "ignore" - Ignore the RIFF chunk size and always search up to 4 GiB
* "maximum" - Search for chunks until the end of file (not recommended)
*/
diff --git a/src/audio/SDL_wave.c b/src/audio/SDL_wave.c
index ac2dfc1..d3a4d34 100644
--- a/src/audio/SDL_wave.c
+++ b/src/audio/SDL_wave.c
@@ -28,7 +28,7 @@
#endif
#ifndef INT_MAX
/* Make a lucky guess. */
-#define INT_MAX (SDL_MAX_SINT32)
+#define INT_MAX SDL_MAX_SINT32
#endif
#endif
@@ -40,17 +40,18 @@
#include "SDL_wave.h"
/* Reads the value stored at the location of the f1 pointer, multiplies it
- * with the second argument, and then stores it back to f1 again.
- * Returns SDL_TRUE if the multiplication overflows, f1 does not get modified.
+ * with the second argument and then stores the result to f1.
+ * Returns 0 on success, or -1 if the multiplication overflows, in which case f1
+ * does not get modified.
*/
-static SDL_bool
-MultiplySize(size_t *f1, size_t f2)
+static int
+SafeMult(size_t *f1, size_t f2)
{
if (*f1 > 0 && SIZE_MAX / *f1 <= f2) {
- return SDL_TRUE;
+ return -1;
}
*f1 *= f2;
- return SDL_FALSE;
+ return 0;
}
typedef struct ADPCM_DecoderState
@@ -333,9 +334,9 @@ static int
MS_ADPCM_CalculateSampleFrames(WaveFile *file, size_t datalength)
{
WaveFormat *format = &file->format;
- const size_t blockheadersize = file->format.channels * 7;
+ const size_t blockheadersize = (size_t)file->format.channels * 7;
const size_t availableblocks = datalength / file->format.blockalign;
- const size_t blockframebitsize = file->format.bitspersample * file->format.channels;
+ const size_t blockframebitsize = (size_t)file->format.bitspersample * file->format.channels;
const size_t trailingdata = datalength % file->format.blockalign;
if (file->trunchint == TruncVeryStrict || file->trunchint == TruncStrict) {
@@ -374,9 +375,9 @@ MS_ADPCM_Init(WaveFile *file, size_t datalength)
{
WaveFormat *format = &file->format;
WaveChunk *chunk = &file->chunk;
- const size_t blockheadersize = format->channels * 7;
+ const size_t blockheadersize = (size_t)format->channels * 7;
const size_t blockdatasize = (size_t)format->blockalign - blockheadersize;
- const size_t blockframebitsize = format->bitspersample * format->channels;
+ const size_t blockframebitsize = (size_t)format->bitspersample * format->channels;
const size_t blockdatasamples = (blockdatasize * 8) / blockframebitsize;
const Sint16 presetcoeffs[14] = {256, 0, 512, -256, 0, 0, 192, 64, 240, 0, 460, -208, 392, -232};
size_t i, coeffcount;
@@ -393,7 +394,7 @@ MS_ADPCM_Init(WaveFile *file, size_t datalength)
}
if (format->bitspersample != 4) {
- return SDL_SetError("Invalid MS ADPCM bits per sample of %d", (int)format->bitspersample);
+ return SDL_SetError("Invalid MS ADPCM bits per sample of %u", (unsigned int)format->bitspersample);
}
/* The block size must be big enough to contain the block header. */
@@ -607,10 +608,10 @@ MS_ADPCM_DecodeBlockData(ADPCM_DecoderState *state)
while (blockframesleft > 0) {
for (c = 0; c < channels; c++) {
- if (nybble & 0x8000) {
+ if (nybble & 0x4000) {
nybble <<= 4;
} else if (blockpos < blocksize) {
- nybble = state->block.data[blockpos++] | 0x8000;
+ nybble = state->block.data[blockpos++] | 0x4000;
} else {
/* Out of input data. Drop the incomplete frame and return. */
state->output.pos = outpos - c;
@@ -661,7 +662,7 @@ MS_ADPCM_Decode(WaveFile *file, Uint8 **audio_buf, Uint32 *audio_len)
state.blocksize = file->format.blockalign;
state.channels = file->format.channels;
- state.blockheadersize = state.channels * 7;
+ state.blockheadersize = (size_t)state.channels * 7;
state.samplesperblock = file->format.samplesperblock;
state.framesize = state.channels * sizeof(Sint16);
state.ddata = file->decoderdata;
@@ -674,7 +675,7 @@ MS_ADPCM_Decode(WaveFile *file, Uint8 **audio_buf, Uint32 *audio_len)
/* The output size in bytes. May get modified if data is truncated. */
outputsize = (size_t)state.framestotal;
- if (MultiplySize(&outputsize, state.framesize)) {
+ if (SafeMult(&outputsize, state.framesize)) {
return SDL_OutOfMemory();
} else if (outputsize > SDL_MAX_UINT32 || state.framestotal > SIZE_MAX) {
return SDL_SetError("WAVE file too big");
@@ -737,8 +738,8 @@ static int
IMA_ADPCM_CalculateSampleFrames(WaveFile *file, size_t datalength)
{
WaveFormat *format = &file->format;
- const size_t blockheadersize = format->channels * 4;
- const size_t subblockframesize = format->channels * 4;
+ const size_t blockheadersize = (size_t)format->channels * 4;
+ const size_t subblockframesize = (size_t)format->channels * 4;
const size_t availableblocks = datalength / format->blockalign;
const size_t trailingdata = datalength % format->blockalign;
@@ -792,18 +793,18 @@ IMA_ADPCM_Init(WaveFile *file, size_t datalength)
{
WaveFormat *format = &file->format;
WaveChunk *chunk = &file->chunk;
- const size_t blockheadersize = format->channels * 4;
+ const size_t blockheadersize = (size_t)format->channels * 4;
const size_t blockdatasize = (size_t)format->blockalign - blockheadersize;
- const size_t blockframebitsize = format->bitspersample * format->channels;
+ const size_t blockframebitsize = (size_t)format->bitspersample * format->channels;
const size_t blockdatasamples = (blockdatasize * 8) / blockframebitsize;
/* Sanity checks. */
- /* IMA ADPCAM can also have 3-bit samples, but it's not supported by SDL at this time. */
+ /* IMA ADPCM can also have 3-bit samples, but it's not supported by SDL at this time. */
if (format->bitspersample == 3) {
return SDL_SetError("3-bit IMA ADPCM currently not supported");
} else if (format->bitspersample != 4) {
- return SDL_SetError("Invalid IMA ADPCM bits per sample of %d", (int)format->bitspersample);
+ return SDL_SetError("Invalid IMA ADPCM bits per sample of %u", (unsigned int)format->bitspersample);
}
/* The block size is required to be a multiple of 4 and it must be able to
@@ -1054,7 +1055,7 @@ IMA_ADPCM_Decode(WaveFile *file, Uint8 **audio_buf, Uint32 *audio_len)
state.channels = file->format.channels;
state.blocksize = file->format.blockalign;
- state.blockheadersize = state.channels * 4;
+ state.blockheadersize = (size_t)state.channels * 4;
state.samplesperblock = file->format.samplesperblock;
state.framesize = state.channels * sizeof(Sint16);
state.framestotal = file->sampleframes;
@@ -1066,7 +1067,7 @@ IMA_ADPCM_Decode(WaveFile *file, Uint8 **audio_buf, Uint32 *audio_len)
/* The output size in bytes. May get modified if data is truncated. */
outputsize = (size_t)state.framestotal;
- if (MultiplySize(&outputsize, state.framesize)) {
+ if (SafeMult(&outputsize, state.framesize)) {
return SDL_OutOfMemory();
} else if (outputsize > SDL_MAX_UINT32 || state.framestotal > SIZE_MAX) {
return SDL_SetError("WAVE file too big");
@@ -1137,7 +1138,7 @@ LAW_Init(WaveFile *file, size_t datalength)
/* Standards Update requires this to be 8. */
if (format->bitspersample != 8) {
- return SDL_SetError("Invalid companded bits per sample of %d", (int)format->bitspersample);
+ return SDL_SetError("Invalid companded bits per sample of %u", (unsigned int)format->bitspersample);
}
/* Not going to bother with weird padding. */
@@ -1222,12 +1223,12 @@ LAW_Decode(WaveFile *file, Uint8 **audio_buf, Uint32 *audio_len)
}
sample_count = (size_t)file->sampleframes;
- if (MultiplySize(&sample_count, format->channels)) {
+ if (SafeMult(&sample_count, format->channels)) {
return SDL_OutOfMemory();
}
expanded_len = sample_count;
- if (MultiplySize(&expanded_len, sizeof(Sint16))) {
+ if (SafeMult(&expanded_len, sizeof(Sint16))) {
return SDL_OutOfMemory();
} else if (expanded_len > SDL_MAX_UINT32 || file->sampleframes > SIZE_MAX) {
return SDL_SetError("WAVE file too big");
@@ -1269,7 +1270,7 @@ LAW_Decode(WaveFile *file, Uint8 **audio_buf, Uint32 *audio_len)
if (exponent > 0) {
mantissa |= 0x10;
}
- mantissa = mantissa << 4 | 0x8;
+ mantissa = (mantissa << 4) | 0x8;
if (exponent > 1) {
mantissa <<= exponent - 1;
}
@@ -1281,7 +1282,7 @@ LAW_Decode(WaveFile *file, Uint8 **audio_buf, Uint32 *audio_len)
while (i--) {
Uint8 nibble = ~src[i];
Sint16 mantissa = nibble & 0xf;
- Uint8 exponent = nibble >> 4 & 0x7;
+ Uint8 exponent = (nibble >> 4) & 0x7;
Sint16 step = 4 << (exponent + 1);
mantissa = (0x80 << exponent) + step * mantissa + step / 2 - 132;
@@ -1315,11 +1316,11 @@ PCM_Init(WaveFile *file, size_t datalength)
/* These are supported. */
break;
default:
- return SDL_SetError("%d-bit PCM format not supported", (int)format->bitspersample);
+ return SDL_SetError("%u-bit PCM format not supported", (unsigned int)format->bitspersample);
}
} else if (format->encoding == IEEE_FLOAT_CODE) {
if (format->bitspersample != 32) {
- return SDL_SetError("%d-bit IEEE floating-point format not supported", (int)format->bitspersample);
+ return SDL_SetError("%u-bit IEEE floating-point format not supported", (unsigned int)format->bitspersample);
}
}
@@ -1353,12 +1354,12 @@ PCM_ConvertSint24ToSint32(WaveFile *file, Uint8 **audio_buf, Uint32 *audio_len)
Uint8 *ptr;
sample_count = (size_t)file->sampleframes;
- if (MultiplySize(&sample_count, format->channels)) {
+ if (SafeMult(&sample_count, format->channels)) {
return SDL_OutOfMemory();
}
expanded_len = sample_count;
- if (MultiplySize(&expanded_len, sizeof(Sint32))) {
+ if (SafeMult(&expanded_len, sizeof(Sint32))) {
return SDL_OutOfMemory();
} else if (expanded_len > SDL_MAX_UINT32 || file->sampleframes > SIZE_MAX) {
return SDL_SetError("WAVE file too big");
@@ -1422,7 +1423,7 @@ PCM_Decode(WaveFile *file, Uint8 **audio_buf, Uint32 *audio_len)
}
outputsize = (size_t)file->sampleframes;
- if (MultiplySize(&outputsize, format->blockalign)) {
+ if (SafeMult(&outputsize, format->blockalign)) {
return SDL_OutOfMemory();
} else if (outputsize > SDL_MAX_UINT32 || file->sampleframes > SIZE_MAX) {
return SDL_SetError("WAVE file too big");
@@ -1444,8 +1445,8 @@ WaveGetRiffSizeHint()
const char *hint = SDL_GetHint(SDL_HINT_WAVE_RIFF_CHUNK_SIZE);
if (hint != NULL) {
- if (SDL_strcmp(hint, "chunksearch") == 0) {
- return RiffSizeChunkSearch;
+ if (SDL_strcmp(hint, "force") == 0) {
+ return RiffSizeForce;
} else if (SDL_strcmp(hint, "ignore") == 0) {
return RiffSizeIgnore;
} else if (SDL_strcmp(hint, "ignorezero") == 0) {
@@ -1517,6 +1518,11 @@ WaveNextChunk(SDL_RWops *src, WaveChunk *chunk)
/* Data is no longer valid after this function returns. */
WaveFreeChunkData(chunk);
+ /* Error on overflows. */
+ if (SDL_MAX_SINT64 - chunk->length < chunk->position || SDL_MAX_SINT64 - 8 < nextposition) {
+ return -1;
+ }
+
/* RIFF chunks have a 2-byte alignment. Skip padding byte. */
if (chunk->length & 1) {
nextposition++;
@@ -1687,7 +1693,7 @@ WaveCheckFormat(WaveFile *file, size_t datalength)
return SDL_SetError("Invalid fact chunk in WAVE file");
}
- /* Check the issues common to all encodings. Some unsupported formats set
+ /* Check for issues common to all encodings. Some unsupported formats set
* the bits per sample to zero. These fall through to the 'unsupported
* format' error.
*/
@@ -1763,7 +1769,7 @@ WaveCheckFormat(WaveFile *file, size_t datalength)
const Uint32 g3 = g[6] | ((Uint32)g[7] << 8);
return SDL_SetError(errstr, g1, g2, g3, g[8], g[9], g[10], g[11], g[12], g[13], g[14], g[15]);
}
- return SDL_SetError("Unknown WAVE format tag: 0x%04x", (int)format->encoding);
+ return SDL_SetError("Unknown WAVE format tag: 0x%04x", (unsigned int)format->encoding);
}
return 0;
@@ -1837,7 +1843,7 @@ WaveLoad(SDL_RWops *src, WaveFile *file, SDL_AudioSpec *spec, Uint8 **audio_buf,
break;
}
/* fallthrough */
- case RiffSizeChunkSearch:
+ case RiffSizeForce:
RIFFend = RIFFchunk.position + RIFFchunk.length;
RIFFlengthknown = SDL_TRUE;
break;
@@ -1850,7 +1856,7 @@ WaveLoad(SDL_RWops *src, WaveFile *file, SDL_AudioSpec *spec, Uint8 **audio_buf,
* chunks. Ignore the chunks we don't know as per specification. This
* currently also ignores cue, list, and slnt chunks.
*/
- while (RIFFend > chunk->position + chunk->length + (chunk->length & 1)) {
+ while ((Uint64)RIFFend > (Uint64)chunk->position + chunk->length + (chunk->length & 1)) {
/* Abort after too many chunks or else corrupt files may waste time. */
if (chunkcount++ >= chunkcountlimit) {
return SDL_SetError("Chunk count in WAVE file exceeds limit of %u", chunkcountlimit);
@@ -1910,7 +1916,7 @@ WaveLoad(SDL_RWops *src, WaveFile *file, SDL_AudioSpec *spec, Uint8 **audio_buf,
* all required chunks were found.
*/
if (file->trunchint == TruncVeryStrict) {
- if (RIFFend < chunk->position + chunk->length) {
+ if ((Uint64)RIFFend < (Uint64)chunk->position + chunk->length) {
return SDL_SetError("RIFF size truncates chunk");
}
} else if (fmtchunk.fourcc == FMT && datachunk.fourcc == DATA) {
@@ -1938,8 +1944,8 @@ WaveLoad(SDL_RWops *src, WaveFile *file, SDL_AudioSpec *spec, Uint8 **audio_buf,
/* data chunk is handled later. */
if (chunk->fourcc != DATA && chunk->length > 0) {
Uint8 tmp;
- Sint64 position = chunk->position + chunk->length - 1;
- if (SDL_RWseek(src, position, RW_SEEK_SET) != position) {
+ Uint64 position = (Uint64)chunk->position + chunk->length - 1;
+ if (position > SDL_MAX_SINT64 || SDL_RWseek(src, (Sint64)position, RW_SEEK_SET) != (Sint64)position) {
return SDL_SetError("Could not seek to WAVE chunk data");
} else if (SDL_RWread(src, &tmp, 1, 1) != 1) {
return SDL_SetError("RIFF size truncates chunk");
@@ -2058,7 +2064,7 @@ WaveLoad(SDL_RWops *src, WaveFile *file, SDL_AudioSpec *spec, Uint8 **audio_buf,
break;
default:
/* Just in case something unexpected happened in the checks. */
- return SDL_SetError("Unexpected %d-bit PCM data format", format->bitspersample);
+ return SDL_SetError("Unexpected %u-bit PCM data format", (unsigned int)format->bitspersample);
}
break;
}
diff --git a/src/audio/SDL_wave.h b/src/audio/SDL_wave.h
index 3d0ae55..083526d 100644
--- a/src/audio/SDL_wave.h
+++ b/src/audio/SDL_wave.h
@@ -103,10 +103,10 @@ typedef struct WaveChunk
/* Controls how the size of the RIFF chunk affects the loading of a WAVE file. */
typedef enum WaveRiffSizeHint {
RiffSizeNoHint,
- RiffSizeChunkSearch,
+ RiffSizeForce,
RiffSizeIgnoreZero,
RiffSizeIgnore,
- RiffSizeMaximum,
+ RiffSizeMaximum
} WaveRiffSizeHint;
/* Controls how a truncated WAVE file is handled. */
@@ -115,7 +115,7 @@ typedef enum WaveTruncationHint {
TruncVeryStrict,
TruncStrict,
TruncDropFrame,
- TruncDropBlock,
+ TruncDropBlock
} WaveTruncationHint;
/* Controls how the fact chunk affects the loading of a WAVE file. */
@@ -124,7 +124,7 @@ typedef enum WaveFactChunkHint {
FactTruncate,
FactStrict,
FactIgnoreZero,
- FactIgnore,
+ FactIgnore
} WaveFactChunkHint;
typedef struct WaveFile