Reduce likelihood of overflow and handle arithmetic overflow gracefully.
authorJean-Yves Avenard <jyavenard@mozilla.com>
Sat, 7 May 2016 11:49:08 +0000 (21:49 +1000)
committerTristan Matthews <tmatth@videolan.org>
Tue, 10 May 2016 18:48:31 +0000 (14:48 -0400)
From bugzilla.mozilla.org bug #1266260

Signed-off-by: Tristan Matthews <tmatth@videolan.org>
include/speex/speex_resampler.h
libspeexdsp/resample.c

index 90c989b..0841ade 100644 (file)
@@ -104,6 +104,7 @@ enum {
    RESAMPLER_ERR_BAD_STATE       = 2,
    RESAMPLER_ERR_INVALID_ARG     = 3,
    RESAMPLER_ERR_PTR_OVERLAP     = 4,
+   RESAMPLER_ERR_OVERFLOW        = 5,
 
    RESAMPLER_ERR_MAX_ERROR
 };
index 3c43b6d..106647e 100644 (file)
@@ -90,6 +90,10 @@ static void speex_free (void *ptr) {free(ptr);}
 #define NULL 0
 #endif
 
+#ifndef UINT32_MAX
+#define UINT32_MAX 4294967296U
+#endif
+
 #ifdef _USE_SSE
 #include "resample_sse.h"
 #endif
@@ -585,6 +589,19 @@ static int resampler_basic_zero(SpeexResamplerState *st, spx_uint32_t channel_in
    return out_sample;
 }
 
+static int _muldiv(spx_uint32_t *result, spx_uint32_t value, spx_uint32_t mul, spx_uint32_t div)
+{
+   spx_uint32_t major = value / div;
+   spx_uint32_t remainder = value % div;
+   /* TODO: Could use 64 bits operation to check for overflow. But only guaranteed in C99+ */
+   if (remainder > UINT32_MAX / mul || major > UINT32_MAX / mul
+       || major * mul > UINT32_MAX - remainder * mul / div)
+      return RESAMPLER_ERR_OVERFLOW;
+   if (result)
+      *result = remainder * mul / div + major * mul;
+   return RESAMPLER_ERR_SUCCESS;
+}
+
 static int update_filter(SpeexResamplerState *st)
 {
    spx_uint32_t old_length = st->filt_len;
@@ -602,8 +619,8 @@ static int update_filter(SpeexResamplerState *st)
    {
       /* down-sampling */
       st->cutoff = quality_map[st->quality].downsample_bandwidth * st->den_rate / st->num_rate;
-      /* FIXME: divide the numerator and denominator by a certain amount if they're too large */
-      st->filt_len = st->filt_len*st->num_rate / st->den_rate;
+      if (_muldiv(&st->filt_len,st->filt_len,st->num_rate,st->den_rate) != RESAMPLER_ERR_SUCCESS)
+         goto fail;
       /* Round up to make sure we have a multiple of 8 for SSE */
       st->filt_len = ((st->filt_len-1)&(~0x7))+8;
       if (2*st->den_rate < st->num_rate)
@@ -1095,36 +1112,6 @@ static inline spx_uint32_t _gcd(spx_uint32_t a, spx_uint32_t b)
    return a;
 }
 
-static spx_uint32_t _muldiv(spx_uint32_t a, spx_uint32_t b, spx_uint32_t c)
-{
-   spx_uint32_t q = 0, r = 0;
-   spx_uint32_t qn = b / c;
-   spx_uint32_t rn = b % c;
-
-   while(a)
-   {
-      if (a & 1)
-      {
-         q += qn;
-         r += rn;
-         if (r >= c)
-         {
-             q++;
-             r -= c;
-         }
-      }
-      a  >>= 1;
-      qn <<= 1;
-      rn <<= 1;
-      if (rn >= c)
-      {
-         qn++;
-         rn -= c;
-      }
-   }
-   return q;
-}
-
 EXPORT int speex_resampler_set_rate_frac(SpeexResamplerState *st, spx_uint32_t ratio_num, spx_uint32_t ratio_den, spx_uint32_t in_rate, spx_uint32_t out_rate)
 {
    spx_uint32_t fact;
@@ -1148,7 +1135,8 @@ EXPORT int speex_resampler_set_rate_frac(SpeexResamplerState *st, spx_uint32_t r
    {
       for (i=0;i<st->nb_channels;i++)
       {
-         st->samp_frac_num[i]= _muldiv(st->samp_frac_num[i],st->den_rate,old_den);
+         if (_muldiv(&st->samp_frac_num[i],st->samp_frac_num[i],st->den_rate,old_den) != RESAMPLER_ERR_SUCCESS)
+            return RESAMPLER_ERR_OVERFLOW;
          /* Safety net */
          if (st->samp_frac_num[i] >= st->den_rate)
             st->samp_frac_num[i] = st->den_rate-1;