From c2665b890c6904714866b0ad21312622b67b97db Mon Sep 17 00:00:00 2001 From: John Regan Date: Thu, 7 Jul 2022 09:38:07 -0400 Subject: [PATCH] Fix undefined behavior: shift exponent is too large Previously, the bestOrder value was being decremented inside the loop. This caused the following: In the case where the loop enters with bestOrder == 2, the value is decremented to 1, and the following operations occurs: m_tnsPredGains[channelIndex] >> (8 * bestOrder - 16) bestOrder being 1 - this evaluates to (8 - 16), causing an integer underflow, and a right shift by 4294967288. m_tnsPredGains is a 32-bit unsigned int, right-shifting by a value > 32 is undefined. I think the intent of the loop conditional is to avoid running the calculation with bestOrder == 1 - as far as I can tell, this is the lowest possible value. By re-arranging the comparison and using a prefix decrement, we can exhibit the same behavior but avoid the case where bestOrder == 1 within the loop body. --- src/lib/exhaleEnc.cpp | 3 +-- src/lib/specAnalysis.cpp | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/lib/exhaleEnc.cpp b/src/lib/exhaleEnc.cpp index a5f76a4..4af64d1 100644 --- a/src/lib/exhaleEnc.cpp +++ b/src/lib/exhaleEnc.cpp @@ -731,9 +731,8 @@ unsigned ExhaleEncoder::getOptParCorCoeffs (const SfbGroupData& grpData, const u predGainCurr = (temp >> 24) & UCHAR_MAX; predGainPrev = (temp >> 16) & UCHAR_MAX; - while ((bestOrder > 1) && (predGainPrev >= predGainCurr)) // lowest-order gain max. + while ((predGainPrev >= predGainCurr) && --bestOrder > 1) // lowest-order gain max. { - bestOrder--; predGainCurr = predGainPrev; predGainPrev = (temp >> (8 * bestOrder - 16)) & UCHAR_MAX; } diff --git a/src/lib/specAnalysis.cpp b/src/lib/specAnalysis.cpp index f1e79b2..24f95cb 100644 --- a/src/lib/specAnalysis.cpp +++ b/src/lib/specAnalysis.cpp @@ -66,9 +66,8 @@ unsigned SpecAnalyzer::getLinPredCoeffs (short parCorCoeffs[MAX_PREDICTION_ORDER predGainCurr = (m_tnsPredGains[channelIndex] >> 24) & UCHAR_MAX; predGainPrev = (m_tnsPredGains[channelIndex] >> 16) & UCHAR_MAX; - while ((bestOrder > 1) && (predGainPrev >= predGainCurr)) // find lowest-order gain maximum + while ((predGainPrev >= predGainCurr) && --bestOrder > 1) // find lowest-order gain maximum { - bestOrder--; predGainCurr = predGainPrev; predGainPrev = (m_tnsPredGains[channelIndex] >> (8 * bestOrder - 16)) & UCHAR_MAX; }