Fixed bug 3894 - Fuzzing crashes for SDL_LoadWAV

Simon Hug

I had a look at this and made some additions to SDL_wave.c.

The attached patch adds many checks and error messages. For some reason I also added A-law and ?-law decoders. Forgot exactly why... but hey, they're small.

The WAVE format is seriously underspecified (at least by the documents that are publicly available on the internet) and it's a shame Microsoft never put something better out there. The language used in them is so loose at times, it's not surprising the encoders and decoders behave very differently. The Windows Media Player doesn't even support MS ADPCM correctly.

The patch also adds some hints to make the decoder more strict at the cost of compatibility with weird WAVE files.

I still think it needs a bit of cleaning up (Not happy with the MultiplySize function. Don't like the name and other SDL code may want to use something like this too.) and some duplicated code may be folded together. It does work in this state and I have thrown all kinds of WAVE files at it. The AFL files also pass with it and some even play (obviously just noise). Crafty little fuzzer.

Any critique would be welcome. I have a fork of SDL with a audio-loadwav branch over here if someone wants to use the commenting feature of Bitbucket:

https://bitbucket.org/ChliHug/SDL

I also cobbled some Lua scripts together to create WAVE test files:

https://bitbucket.org/ChliHug/gendat

include/SDL_audio.h

@@ -420,23 +420,56 @@ extern DECLSPEC void SDLCALL SDL_PauseAudioDevice(SDL_AudioDeviceID dev,
 /* @} *//* Pause audio functions */
 
 /**
- *  This function loads a WAVE from the data source, automatically freeing
- *  that source if \c freesrc is non-zero.  For example, to load a WAVE file,
- *  you could do:
+ *  \brief Load the audio data of a WAVE file into memory
+ *
+ *  Loading a WAVE file requires \c src, \c spec, \c audio_buf and \c audio_len
+ *  to be valid pointers. The entire data portion of the file is then loaded
+ *  into memory and decoded if necessary.
+ *
+ *  If \c freesrc is non-zero, the data source gets automatically closed and
+ *  freed before the function returns.
+ *
+ *  Supported are RIFF WAVE files with the formats PCM (8, 16, 24, and 32 bits),
+ *  IEEE Float (32 bits), Microsoft ADPCM and IMA ADPCM (4 bits), and A-law and
+ *  µ-law (8 bits). Other formats are currently unsupported and cause an error.
+ *
+ *  If this function succeeds, the pointer returned by it is equal to \c spec
+ *  and the pointer to the audio data allocated by the function is written to
+ *  \c audio_buf and its length in bytes to \c audio_len. The \ref SDL_AudioSpec
+ *  members \c freq, \c channels, and \c format are set to the values of the
+ *  audio data in the buffer. The \c samples member is set to a sane default and
+ *  all others are set to zero.
+ *
+ *  It's necessary to use SDL_FreeWAV() to free the audio data returned in
+ *  \c audio_buf when it is no longer used.
+ *
+ *  Because of the underspecification of the Waveform format, there are many
+ *  problematic files in the wild that cause issues with strict decoders. To
+ *  provide compatibility with these files, this decoder is lenient in regards
+ *  to the truncation of the file, the fact chunk, and the size of the RIFF
+ *  chunk. The hints SDL_HINT_WAVE_RIFF_CHUNK_SIZE, SDL_HINT_WAVE_TRUNCATION,
+ *  and SDL_HINT_WAVE_FACT_CHUNK can be used to tune the behavior of the
+ *  loading process.
+ *
+ *  Any file that is invalid (due to truncation, corruption, or wrong values in
+ *  the headers), too big, or unsupported causes an error. Additionally, any
+ *  critical I/O error from the data source will terminate the loading process
+ *  with an error. The function returns NULL on error and in all cases (with the
+ *  exception of \c src being NULL), an appropriate error message will be set.
+ *
+ *  It is required that the data source supports seeking.
+ *
+ *  Example:
  *  \code
  *      SDL_LoadWAV_RW(SDL_RWFromFile("sample.wav", "rb"), 1, ...);
  *  \endcode
  *
- *  If this function succeeds, it returns the given SDL_AudioSpec,
- *  filled with the audio data format of the wave data, and sets
- *  \c *audio_buf to a malloc()'d buffer containing the audio data,
- *  and sets \c *audio_len to the length of that audio buffer, in bytes.
- *  You need to free the audio buffer with SDL_FreeWAV() when you are
- *  done with it.
- *
- *  This function returns NULL and sets the SDL error message if the
- *  wave file cannot be opened, uses an unknown data format, or is
- *  corrupt.  Currently raw and MS-ADPCM WAVE files are supported.
+ *  \param src The data source with the WAVE data
+ *  \param freesrc A integer value that makes the function close the data source if non-zero
+ *  \param spec A pointer filled with the audio format of the audio data
+ *  \param audio_buf A pointer filled with the audio data allocated by the function
+ *  \param audio_len A pointer filled with the length of the audio data buffer in bytes
+ *  \return NULL on error, or non-NULL on success.
  */
 extern DECLSPEC SDL_AudioSpec *SDLCALL SDL_LoadWAV_RW(SDL_RWops * src,
                                                       int freesrc,

include/SDL_hints.h

@@ -1121,6 +1121,70 @@ extern "C" {
 
 
 
+/**
+ *  \brief  Controls how the size of the RIFF chunk affects the loading of a WAVE file.
+ *
+ *  The size of the RIFF chunk (which includes all the sub-chunks of the WAVE
+ *  file) is not always reliable. In case the size is wrong, it's possible to
+ *  just ignore it and step through the chunks until a fixed limit is reached.
+ *
+ *  Note that files that have trailing data unrelated to the WAVE file or
+ *  corrupt files may slow down the loading process without a reliable boundary.
+ *  By default, SDL stops after 10000 chunks to prevent wasting time. Use the
+ *  environment variable SDL_WAVE_CHUNK_LIMIT to adjust this value.
+ *
+ *  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)
+ *    "ignore"       - Ignore the RIFF chunk size and always search up to 4 GiB
+ *    "maximum"      - Search for chunks until the end of file (not recommended)
+ */
+#define SDL_HINT_WAVE_RIFF_CHUNK_SIZE   "SDL_WAVE_RIFF_CHUNK_SIZE"
+
+/**
+ *  \brief  Controls how a truncated WAVE file is handled.
+ *
+ *  A WAVE file is considered truncated if any of the chunks are incomplete or
+ *  the data chunk size is not a multiple of the block size. By default, SDL
+ *  decodes until the first incomplete block, as most applications seem to do.
+ *
+ *  This variable can be set to the following values:
+ *
+ *    "verystrict" - Raise an error if the file is truncated
+ *    "strict"     - Like "verystrict", but the size of the RIFF chunk is ignored
+ *    "dropframe"  - Decode until the first incomplete sample frame
+ *    "dropblock"  - Decode until the first incomplete block (default)
+ */
+#define SDL_HINT_WAVE_TRUNCATION   "SDL_WAVE_TRUNCATION"
+
+/**
+ *  \brief  Controls how the fact chunk affects the loading of a WAVE file.
+ *
+ *  The fact chunk stores information about the number of samples of a WAVE
+ *  file. The Standards Update from Microsoft notes that this value can be used
+ *  to 'determine the length of the data in seconds'. This is especially useful
+ *  for compressed formats (for which this is a mandatory chunk) if they produce
+ *  multiple sample frames per block and truncating the block is not allowed.
+ *  The fact chunk can exactly specify how many sample frames there should be
+ *  in this case.
+ *
+ *  Unfortunately, most application seem to ignore the fact chunk and so SDL
+ *  ignores it by default as well.
+ *
+ *  This variable can be set to the following values:
+ *
+ *    "truncate"    - Use the number of samples to truncate the wave data if
+ *                    the fact chunk is present and valid
+ *    "strict"      - Like "truncate", but raise an error if the fact chunk
+ *                    is invalid, not present for non-PCM formats, or if the
+ *                    data chunk doesn't have that many samples
+ *    "ignorezero"  - Like "truncate", but ignore fact chunk if the number of
+ *                    samples is zero
+ *    "ignore"      - Ignore fact chunk entirely (default)
+ */
+#define SDL_HINT_WAVE_FACT_CHUNK   "SDL_WAVE_FACT_CHUNK"
+
 /**
  *  \brief  An enumeration of hint priorities
  */