Fixed bug 3668 - Overflow of SDL_AudioCVT.filters with some downmixes
authorSam Lantinga <slouken@libsdl.org>
Mon, 12 Jun 2017 16:39:15 -0700
changeset 11096819384789a7a
parent 11095 5cf754a84eb7
child 11097 62a0a6e9b48b
Fixed bug 3668 - Overflow of SDL_AudioCVT.filters with some downmixes

Simon Hug

There's a chance that an audio conversion from many channels to a few can use more than 9 audio filters. SDL_AudioCVT has 10 SDL_AudioFilter pointers of which one has to be the terminating NULL pointer. The SDL code has no checks for this limit. If it overflows there can be stack or heap corruption or a call to 0xa.

Attached patch adds a function that checks for this limit and throws an error if it is reached. Also adds some documentation.

Test parameters that trigger this issue:
AUDIO_U16MSB with 224 channels at 46359 Hz
V
AUDIO_S16MSB with 6 channels at 27463 Hz

The fuzzer program I uploaded in bug 3667 has more of them.
include/SDL_audio.h
src/audio/SDL_audiocvt.c
     1.1 --- a/include/SDL_audio.h	Mon Jun 12 16:35:34 2017 -0700
     1.2 +++ b/include/SDL_audio.h	Mon Jun 12 16:39:15 2017 -0700
     1.3 @@ -184,7 +184,17 @@
     1.4                                            SDL_AudioFormat format);
     1.5  
     1.6  /**
     1.7 - *  A structure to hold a set of audio conversion filters and buffers.
     1.8 + *  \brief Upper limit of filters in SDL_AudioCVT
     1.9 + *
    1.10 + *  The maximum number of SDL_AudioFilter functions in SDL_AudioCVT is
    1.11 + *  currently limited to 9. The SDL_AudioCVT.filters array has 10 pointers,
    1.12 + *  one of which is the terminating NULL pointer.
    1.13 + */
    1.14 +#define SDL_AUDIOCVT_MAX_FILTERS 9
    1.15 +
    1.16 +/**
    1.17 + *  \struct SDL_AudioCVT
    1.18 + *  \brief A structure to hold a set of audio conversion filters and buffers.
    1.19   *
    1.20   *  Note that various parts of the conversion pipeline can take advantage
    1.21   *  of SIMD operations (like SSE2, for example). SDL_AudioCVT doesn't require
    1.22 @@ -214,7 +224,7 @@
    1.23      int len_cvt;                /**< Length of converted audio buffer */
    1.24      int len_mult;               /**< buffer must be len*len_mult big */
    1.25      double len_ratio;           /**< Given len, final size is len*len_ratio */
    1.26 -    SDL_AudioFilter filters[10];        /**< Filter list */
    1.27 +    SDL_AudioFilter filters[SDL_AUDIOCVT_MAX_FILTERS + 1]; /**< NULL-terminated list of filter functions */
    1.28      int filter_index;           /**< Current audio conversion function */
    1.29  } SDL_AUDIOCVT_PACKED SDL_AudioCVT;
    1.30  
     2.1 --- a/src/audio/SDL_audiocvt.c	Mon Jun 12 16:35:34 2017 -0700
     2.2 +++ b/src/audio/SDL_audiocvt.c	Mon Jun 12 16:39:15 2017 -0700
     2.3 @@ -514,6 +514,19 @@
     2.4      }
     2.5  }
     2.6  
     2.7 +static int
     2.8 +SDL_AddAudioCVTFilter(SDL_AudioCVT *cvt, const SDL_AudioFilter filter)
     2.9 +{
    2.10 +    if (cvt->filter_index >= SDL_AUDIOCVT_MAX_FILTERS) {
    2.11 +        return SDL_SetError("Too many filters needed for conversion, exceeded maximum of %d", SDL_AUDIOCVT_MAX_FILTERS);
    2.12 +    }
    2.13 +    if (filter == NULL) {
    2.14 +        return SDL_SetError("Audio filter pointer is NULL");
    2.15 +    }
    2.16 +    cvt->filters[cvt->filter_index++] = filter;
    2.17 +    cvt->filters[cvt->filter_index] = NULL; /* Moving terminator */
    2.18 +    return 0;
    2.19 +}
    2.20  
    2.21  static int
    2.22  SDL_BuildAudioTypeCVTToFloat(SDL_AudioCVT *cvt, const SDL_AudioFormat src_fmt)
    2.23 @@ -521,7 +534,9 @@
    2.24      int retval = 0;  /* 0 == no conversion necessary. */
    2.25  
    2.26      if ((SDL_AUDIO_ISBIGENDIAN(src_fmt) != 0) == (SDL_BYTEORDER == SDL_LIL_ENDIAN)) {
    2.27 -        cvt->filters[cvt->filter_index++] = SDL_Convert_Byteswap;
    2.28 +        if (SDL_AddAudioCVTFilter(cvt, SDL_Convert_Byteswap) < 0) {
    2.29 +            return -1;
    2.30 +        }
    2.31          retval = 1;  /* added a converter. */
    2.32      }
    2.33  
    2.34 @@ -543,7 +558,9 @@
    2.35              return SDL_SetError("No conversion available for these formats");
    2.36          }
    2.37  
    2.38 -        cvt->filters[cvt->filter_index++] = filter;
    2.39 +        if (SDL_AddAudioCVTFilter(cvt, filter) < 0) {
    2.40 +            return -1;
    2.41 +        }
    2.42          if (src_bitsize < dst_bitsize) {
    2.43              const int mult = (dst_bitsize / src_bitsize);
    2.44              cvt->len_mult *= mult;
    2.45 @@ -580,7 +597,9 @@
    2.46              return SDL_SetError("No conversion available for these formats");
    2.47          }
    2.48  
    2.49 -        cvt->filters[cvt->filter_index++] = filter;
    2.50 +        if (SDL_AddAudioCVTFilter(cvt, filter) < 0) {
    2.51 +            return -1;
    2.52 +        }
    2.53          if (src_bitsize < dst_bitsize) {
    2.54              const int mult = (dst_bitsize / src_bitsize);
    2.55              cvt->len_mult *= mult;
    2.56 @@ -592,7 +611,9 @@
    2.57      }
    2.58  
    2.59      if ((SDL_AUDIO_ISBIGENDIAN(dst_fmt) != 0) == (SDL_BYTEORDER == SDL_LIL_ENDIAN)) {
    2.60 -        cvt->filters[cvt->filter_index++] = SDL_Convert_Byteswap;
    2.61 +        if (SDL_AddAudioCVTFilter(cvt, SDL_Convert_Byteswap) < 0) {
    2.62 +            return -1;
    2.63 +        }
    2.64          retval = 1;  /* added a converter. */
    2.65      }
    2.66  
    2.67 @@ -665,7 +686,9 @@
    2.68      }
    2.69  
    2.70      /* Update (cvt) with filter details... */
    2.71 -    cvt->filters[cvt->filter_index++] = filter;
    2.72 +    if (SDL_AddAudioCVTFilter(cvt, filter) < 0) {
    2.73 +        return -1;
    2.74 +    }
    2.75      if (src_rate < dst_rate) {
    2.76          const double mult = ((double) dst_rate) / ((double) src_rate);
    2.77          cvt->len_mult *= (int) SDL_ceil(mult);
    2.78 @@ -739,7 +762,9 @@
    2.79         format as well. */
    2.80      if ((src_channels == 2) && (dst_channels == 2) && (src_fmt == AUDIO_S16SYS) && (dst_fmt == AUDIO_S16SYS) && (src_rate != dst_rate)) {
    2.81          cvt->needed = 1;
    2.82 -        cvt->filters[cvt->filter_index++] = SDL_ResampleCVT_si16_c2;
    2.83 +        if (SDL_AddAudioCVTFilter(cvt, SDL_ResampleCVT_si16_c2) < 0) {
    2.84 +            return -1;
    2.85 +        }
    2.86          if (src_rate < dst_rate) {
    2.87              const double mult = ((double) dst_rate) / ((double) src_rate);
    2.88              cvt->len_mult *= (int) SDL_ceil(mult);
    2.89 @@ -772,7 +797,9 @@
    2.90  
    2.91          /* just a byteswap needed? */
    2.92          if ((src_fmt & ~SDL_AUDIO_MASK_ENDIAN) == (dst_fmt & ~SDL_AUDIO_MASK_ENDIAN)) {
    2.93 -            cvt->filters[cvt->filter_index++] = SDL_Convert_Byteswap;
    2.94 +            if (SDL_AddAudioCVTFilter(cvt, SDL_Convert_Byteswap) < 0) {
    2.95 +                return -1;
    2.96 +            }
    2.97              cvt->needed = 1;
    2.98              return 1;
    2.99          }
   2.100 @@ -786,36 +813,48 @@
   2.101      /* Channel conversion */
   2.102      if (src_channels != dst_channels) {
   2.103          if ((src_channels == 1) && (dst_channels > 1)) {
   2.104 -            cvt->filters[cvt->filter_index++] = SDL_ConvertMonoToStereo;
   2.105 +            if (SDL_AddAudioCVTFilter(cvt, SDL_ConvertMonoToStereo) < 0) {
   2.106 +                return -1;
   2.107 +            }
   2.108              cvt->len_mult *= 2;
   2.109              src_channels = 2;
   2.110              cvt->len_ratio *= 2;
   2.111          }
   2.112          if ((src_channels == 2) && (dst_channels == 6)) {
   2.113 -            cvt->filters[cvt->filter_index++] = SDL_ConvertStereoTo51;
   2.114 +            if (SDL_AddAudioCVTFilter(cvt, SDL_ConvertStereoTo51) < 0) {
   2.115 +                return -1;
   2.116 +            }
   2.117              src_channels = 6;
   2.118              cvt->len_mult *= 3;
   2.119              cvt->len_ratio *= 3;
   2.120          }
   2.121          if ((src_channels == 2) && (dst_channels == 4)) {
   2.122 -            cvt->filters[cvt->filter_index++] = SDL_ConvertStereoToQuad;
   2.123 +            if (SDL_AddAudioCVTFilter(cvt, SDL_ConvertStereoToQuad) < 0) {
   2.124 +                return -1;
   2.125 +            }
   2.126              src_channels = 4;
   2.127              cvt->len_mult *= 2;
   2.128              cvt->len_ratio *= 2;
   2.129          }
   2.130          while ((src_channels * 2) <= dst_channels) {
   2.131 -            cvt->filters[cvt->filter_index++] = SDL_ConvertMonoToStereo;
   2.132 +            if (SDL_AddAudioCVTFilter(cvt, SDL_ConvertMonoToStereo) < 0) {
   2.133 +                return -1;
   2.134 +            }
   2.135              cvt->len_mult *= 2;
   2.136              src_channels *= 2;
   2.137              cvt->len_ratio *= 2;
   2.138          }
   2.139          if ((src_channels == 6) && (dst_channels <= 2)) {
   2.140 -            cvt->filters[cvt->filter_index++] = SDL_Convert51ToStereo;
   2.141 +            if (SDL_AddAudioCVTFilter(cvt, SDL_Convert51ToStereo) < 0) {
   2.142 +                return -1;
   2.143 +            }
   2.144              src_channels = 2;
   2.145              cvt->len_ratio /= 3;
   2.146          }
   2.147          if ((src_channels == 6) && (dst_channels == 4)) {
   2.148 -            cvt->filters[cvt->filter_index++] = SDL_Convert51ToQuad;
   2.149 +            if (SDL_AddAudioCVTFilter(cvt, SDL_Convert51ToQuad) < 0) {
   2.150 +                return -1;
   2.151 +            }
   2.152              src_channels = 4;
   2.153              cvt->len_ratio /= 2;
   2.154          }
   2.155 @@ -837,7 +876,9 @@
   2.156                  filter = SDL_ConvertStereoToMono;
   2.157              }
   2.158  
   2.159 -            cvt->filters[cvt->filter_index++] = filter;
   2.160 +            if (SDL_AddAudioCVTFilter(cvt, filter) < 0) {
   2.161 +                return -1;
   2.162 +            }
   2.163  
   2.164              src_channels /= 2;
   2.165              cvt->len_ratio /= 2;