[FFmpeg-devel,2/4] opus: Fix arithmetic overflows (per RFC8251)

Submitted by Andrew D'Addesio on Dec. 2, 2017, 5:36 p.m.

Details

Message ID 20171202173627.5292-2-modchipv12@gmail.com
State New
Headers show

Commit Message

Andrew D'Addesio Dec. 2, 2017, 5:36 p.m.
The relevant sections from the RFC are:

Sec.6. Integer Wrap-Around in Inverse Gain Computation
    32-bit integer overflow in Levinson recursion. Affects
    silk_is_lpc_stable().

Sec.8. Cap on Band Energy
    NaN due to large log-energy value. Affects celt_denormalize().

Signed-off-by: Andrew D'Addesio <modchipv12@gmail.com>
---
 libavcodec/opus_celt.c |  3 ++-
 libavcodec/opus_silk.c | 11 +++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

Comments

Rostislav Pehlivanov Dec. 4, 2017, 7:37 a.m.
On 2 December 2017 at 17:36, Andrew D'Addesio <modchipv12@gmail.com> wrote:

> The relevant sections from the RFC are:
>
> Sec.6. Integer Wrap-Around in Inverse Gain Computation
>     32-bit integer overflow in Levinson recursion. Affects
>     silk_is_lpc_stable().
>
> Sec.8. Cap on Band Energy
>     NaN due to large log-energy value. Affects celt_denormalize().
>
> Signed-off-by: Andrew D'Addesio <modchipv12@gmail.com>
> ---
>  libavcodec/opus_celt.c |  3 ++-
>  libavcodec/opus_silk.c | 11 +++++++++--
>  2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/opus_celt.c b/libavcodec/opus_celt.c
> index 84d4847..ff56041 100644
> --- a/libavcodec/opus_celt.c
> +++ b/libavcodec/opus_celt.c
> @@ -481,7 +481,8 @@ static void celt_denormalize(CeltFrame *f, CeltBlock
> *block, float *data)
>
>      for (i = f->start_band; i < f->end_band; i++) {
>          float *dst = data + (ff_celt_freq_bands[i] << f->size);
> -        float norm = exp2f(block->energy[i] + ff_celt_mean_energy[i]);
> +        float log_norm = block->energy[i] + ff_celt_mean_energy[i];
> +        float norm = exp2f(FFMIN(log_norm, 32.0f));
>
>          for (j = 0; j < ff_celt_freq_range[i] << f->size; j++)
>              dst[j] *= norm;
> diff --git a/libavcodec/opus_silk.c b/libavcodec/opus_silk.c
> index 3c9c849..344333c 100644
> --- a/libavcodec/opus_silk.c
> +++ b/libavcodec/opus_silk.c
> @@ -185,8 +185,15 @@ static inline int silk_is_lpc_stable(const int16_t
> lpc[16], int order)
>          row = lpc32[k & 1];
>
>          for (j = 0; j < k; j++) {
> -            int x = prevrow[j] - ROUND_MULL(prevrow[k - j - 1], rc, 31);
> -            row[j] = ROUND_MULL(x, gain, fbits);
> +            int x = av_sat_sub32(prevrow[j], ROUND_MULL(prevrow[k - j -
> 1], rc, 31));
> +            int64_t tmp = ROUND_MULL(x, gain, fbits);
> +
> +            /* per RFC 8251 section 6, if this calculation overflows, the
> filter
> +               is considered unstable. */
> +            if (tmp < INT32_MIN || tmp > INT32_MAX)
> +                return 0;
> +
> +            row[j] = (int32_t)tmp;
>          }
>      }
>  }
> --
> 2.15.1.windows.2
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Pushed (split in 2 commits, not that it matters).

Patch hide | download patch | download mbox

diff --git a/libavcodec/opus_celt.c b/libavcodec/opus_celt.c
index 84d4847..ff56041 100644
--- a/libavcodec/opus_celt.c
+++ b/libavcodec/opus_celt.c
@@ -481,7 +481,8 @@  static void celt_denormalize(CeltFrame *f, CeltBlock *block, float *data)
 
     for (i = f->start_band; i < f->end_band; i++) {
         float *dst = data + (ff_celt_freq_bands[i] << f->size);
-        float norm = exp2f(block->energy[i] + ff_celt_mean_energy[i]);
+        float log_norm = block->energy[i] + ff_celt_mean_energy[i];
+        float norm = exp2f(FFMIN(log_norm, 32.0f));
 
         for (j = 0; j < ff_celt_freq_range[i] << f->size; j++)
             dst[j] *= norm;
diff --git a/libavcodec/opus_silk.c b/libavcodec/opus_silk.c
index 3c9c849..344333c 100644
--- a/libavcodec/opus_silk.c
+++ b/libavcodec/opus_silk.c
@@ -185,8 +185,15 @@  static inline int silk_is_lpc_stable(const int16_t lpc[16], int order)
         row = lpc32[k & 1];
 
         for (j = 0; j < k; j++) {
-            int x = prevrow[j] - ROUND_MULL(prevrow[k - j - 1], rc, 31);
-            row[j] = ROUND_MULL(x, gain, fbits);
+            int x = av_sat_sub32(prevrow[j], ROUND_MULL(prevrow[k - j - 1], rc, 31));
+            int64_t tmp = ROUND_MULL(x, gain, fbits);
+
+            /* per RFC 8251 section 6, if this calculation overflows, the filter
+               is considered unstable. */
+            if (tmp < INT32_MIN || tmp > INT32_MAX)
+                return 0;
+
+            row[j] = (int32_t)tmp;
         }
     }
 }