From 3a2c803dc341a2bc266effb07df9863c14a7aeaa Mon Sep 17 00:00:00 2001 From: kwiberg Date: Fri, 3 Mar 2017 05:44:49 -0800 Subject: [PATCH] Multiply in 64 bits to avoid overflow A fuzzer run caused the operands of this multiplication to be 512 and 5000000, resulting in a product about 20% too large for int32_t. So change this from a 16x32->32 to a 16x32->64 multiplication. Since we right shift by 2 at the end, the end result will still fit in int32_t. I also had to fix a few follow-on add/sub overflows found by the same fuzzer input once the multiplication was fixed. I chose to saturate these, since it wasn't just an intermediate value that overflowed. BUG=chromium:693868 Review-Url: https://codereview.webrtc.org/2729573002 Cr-Commit-Position: refs/heads/master@{#17003} --- .../codecs/isac/fix/source/entropy_coding.c | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/webrtc/modules/audio_coding/codecs/isac/fix/source/entropy_coding.c b/webrtc/modules/audio_coding/codecs/isac/fix/source/entropy_coding.c index 8e351277c3..10e05bc31a 100644 --- a/webrtc/modules/audio_coding/codecs/isac/fix/source/entropy_coding.c +++ b/webrtc/modules/audio_coding/codecs/isac/fix/source/entropy_coding.c @@ -26,6 +26,7 @@ #include "lpc_tables.h" #include "settings.h" #include "signal_processing_library.h" +#include "webrtc/base/checks.h" /* * Eenumerations for arguments to functions WebRtcIsacfix_MatrixProduct1() @@ -230,8 +231,12 @@ static void CalcInvArSpec(const int16_t *ARCoefQ12, CurveQ16[n] = sum; for (k = 1; k < AR_ORDER; k += 2) { - for (n = 0; n < FRAMESAMPLES/8; n++) - CurveQ16[n] += (WebRtcIsacfix_kCos[k][n] * CorrQ11[k + 1] + 2) >> 2; + for (n = 0; n < FRAMESAMPLES/8; n++) { + const int64_t p = + (WebRtcIsacfix_kCos[k][n] * (int64_t)CorrQ11[k + 1] + 2) >> 2; + RTC_DCHECK_EQ(p, (int32_t)p); // p fits in 32 bits + CurveQ16[n] += (int32_t)p; + } } CS_ptrQ9 = WebRtcIsacfix_kCos[0]; @@ -256,8 +261,9 @@ static void CalcInvArSpec(const int16_t *ARCoefQ12, for (k=0; k> 2] + 2195456) >> 16)); + gainQ10 = WebRtcSpl_DivW32W16ResW16( + 30 << 10, (int16_t)((uint32_t)(WebRtcSpl_AddSatW32( + invARSpec2_Q16[k >> 2], 2195456)) >> + 16)); *frQ7++ = (int16_t)((data[k] * gainQ10 + 512) >> 10); *fiQ7++ = (int16_t)((data[k + 1] * gainQ10 + 512) >> 10); *frQ7++ = (int16_t)((data[k + 2] * gainQ10 + 512) >> 10); @@ -504,8 +512,10 @@ int WebRtcIsacfix_DecodeSpec(Bitstr_dec *streamdata, { for (k = 0; k < FRAMESAMPLES; k += 4) { - gainQ10 = WebRtcSpl_DivW32W16ResW16(36 << 10, - (int16_t)((uint32_t)(invARSpec2_Q16[k >> 2] + 2654208) >> 16)); + gainQ10 = WebRtcSpl_DivW32W16ResW16( + 36 << 10, (int16_t)((uint32_t)(WebRtcSpl_AddSatW32( + invARSpec2_Q16[k >> 2], 2654208)) >> + 16)); *frQ7++ = (int16_t)((data[k] * gainQ10 + 512) >> 10); *fiQ7++ = (int16_t)((data[k + 1] * gainQ10 + 512) >> 10); *frQ7++ = (int16_t)((data[k + 2] * gainQ10 + 512) >> 10);