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,