From 15292f7e9620caf9e8df26a62efc2a2891ea822e Mon Sep 17 00:00:00 2001 From: Fraunhofer IIS FDK Date: Fri, 8 Jun 2018 18:03:16 +0200 Subject: [PATCH 1/9] Prevent bit buffer counter overflow. While long-term test we discovered a bit counter overflow in the bit buffer. The bit buffer state was only used by HCR and RVLC tool and can easily be substituted with FDKgetValidBits() call. The following patch completely removes the bit counter and all its obsolete functions. Bug: 112662184 Test: atest DecoderTestXheAac ; atest DecoderTestAacDrc Change-Id: Icee0519d26a2aa62367d2dece59cd3d60ffcade7 --- libAACdec/src/aacdec_hcr.cpp | 82 ++++++++++++++++---------------- libAACdec/src/aacdec_hcr_bit.cpp | 7 +-- libAACdec/src/aacdec_hcr_bit.h | 3 +- libAACdec/src/aacdec_hcr_types.h | 2 +- libAACdec/src/aacdec_hcrs.cpp | 42 ++++++++-------- libAACdec/src/rvlc.cpp | 29 ++++++----- libAACdec/src/rvlc_info.h | 1 + libAACdec/src/rvlcbit.cpp | 6 +-- libAACdec/src/rvlcbit.h | 4 +- libFDK/include/FDK_bitbuffer.h | 6 --- libFDK/include/FDK_bitstream.h | 46 ------------------ libFDK/src/FDK_bitbuffer.cpp | 29 ----------- 12 files changed, 91 insertions(+), 166 deletions(-) diff --git a/libAACdec/src/aacdec_hcr.cpp b/libAACdec/src/aacdec_hcr.cpp index 84e05b0..6114756 100644 --- a/libAACdec/src/aacdec_hcr.cpp +++ b/libAACdec/src/aacdec_hcr.cpp @@ -134,17 +134,18 @@ static void DeriveNumberOfExtendedSortedSectionsInSets( USHORT *pNumExtendedSortedSectionsInSets, int numExtendedSortedSectionsInSetsIdx); -static INT DecodeEscapeSequence(HANDLE_FDK_BITSTREAM bs, INT quantSpecCoef, - INT *pLeftStartOfSegment, +static INT DecodeEscapeSequence(HANDLE_FDK_BITSTREAM bs, const INT bsAnchor, + INT quantSpecCoef, INT *pLeftStartOfSegment, SCHAR *pRemainingBitsInSegment, int *pNumDecodedBits); -static int DecodePCW_Sign(HANDLE_FDK_BITSTREAM bs, UINT codebookDim, - const SCHAR *pQuantVal, FIXP_DBL *pQuantSpecCoef, - int *quantSpecCoefIdx, INT *pLeftStartOfSegment, +static int DecodePCW_Sign(HANDLE_FDK_BITSTREAM bs, const INT bsAnchor, + UINT codebookDim, const SCHAR *pQuantVal, + FIXP_DBL *pQuantSpecCoef, int *quantSpecCoefIdx, + INT *pLeftStartOfSegment, SCHAR *pRemainingBitsInSegment, int *pNumDecodedBits); -static const SCHAR *DecodePCW_Body(HANDLE_FDK_BITSTREAM bs, +static const SCHAR *DecodePCW_Body(HANDLE_FDK_BITSTREAM bs, const INT bsAnchor, const UINT *pCurrentTree, const SCHAR *pQuantValBase, INT *pLeftStartOfSegment, @@ -291,7 +292,7 @@ UINT HcrInit(H_HCR_INFO pHcr, CAacDecoderChannelInfo *pAacDecoderChannelInfo, SPEC_LONG(pAacDecoderChannelInfo->pSpectralCoefficient); FDKsyncCache(bs); - pHcr->decInOut.bitstreamIndex = FDKgetBitCnt(bs); + pHcr->decInOut.bitstreamAnchor = (INT)FDKgetValidBits(bs); if (!IsLongBlock(&pAacDecoderChannelInfo->icsInfo)) /* short block */ { @@ -436,7 +437,7 @@ UINT HcrDecoder(H_HCR_INFO pHcr, CAacDecoderChannelInfo *pAacDecoderChannelInfo, int pTmp5; INT bitCntOffst; - INT saveBitCnt = FDKgetBitCnt(bs); /* save bitstream position */ + INT saveBitCnt = (INT)FDKgetValidBits(bs); /* save bitstream position */ HcrCalcNumCodeword(pHcr); @@ -487,7 +488,7 @@ UINT HcrDecoder(H_HCR_INFO pHcr, CAacDecoderChannelInfo *pAacDecoderChannelInfo, pSamplingRateInfo); /* restore bitstream position */ - bitCntOffst = saveBitCnt - FDKgetBitCnt(bs); + bitCntOffst = (INT)FDKgetValidBits(bs) - saveBitCnt; if (bitCntOffst) { FDKpushBiDirectional(bs, bitCntOffst); } @@ -815,7 +816,6 @@ static void HcrPrepareSegmentationGrid(H_HCR_INFO pHcr) { INT *pLeftStartOfSegment = pHcr->segmentInfo.pLeftStartOfSegment; INT *pRightStartOfSegment = pHcr->segmentInfo.pRightStartOfSegment; SCHAR *pRemainingBitsInSegment = pHcr->segmentInfo.pRemainingBitsInSegment; - INT bitstreamIndex = pHcr->decInOut.bitstreamIndex; const UCHAR *pMaxCwLength = aMaxCwLen; for (i = numSortedSection; i != 0; i--) { @@ -825,7 +825,7 @@ static void HcrPrepareSegmentationGrid(H_HCR_INFO pHcr) { for (j = *pNumSortedCodewordInSection; j != 0; j--) { /* width allows a new segment */ - intermediateResult = bitstreamIndex + segmentStart; + intermediateResult = segmentStart; if ((segmentStart + segmentWidth) <= lengthOfReorderedSpectralData) { /* store segment start, segment length and increment the number of * segments */ @@ -841,12 +841,11 @@ static void HcrPrepareSegmentationGrid(H_HCR_INFO pHcr) { pLeftStartOfSegment--; pRightStartOfSegment--; pRemainingBitsInSegment--; - segmentStart = *pLeftStartOfSegment - bitstreamIndex; + segmentStart = *pLeftStartOfSegment; lastSegmentWidth = lengthOfReorderedSpectralData - segmentStart; *pRemainingBitsInSegment = lastSegmentWidth; - *pRightStartOfSegment = - bitstreamIndex + segmentStart + lastSegmentWidth - 1; + *pRightStartOfSegment = segmentStart + lastSegmentWidth - 1; endFlag = 1; break; } @@ -1071,9 +1070,9 @@ static void DecodePCWs(HANDLE_FDK_BITSTREAM bs, H_HCR_INFO pHcr) { numDecodedBits = 0; /* decode PCW_BODY */ - pQuantVal = - DecodePCW_Body(bs, pCurrentTree, pQuantValBase, pLeftStartOfSegment, - pRemainingBitsInSegment, &numDecodedBits); + pQuantVal = DecodePCW_Body( + bs, pHcr->decInOut.bitstreamAnchor, pCurrentTree, pQuantValBase, + pLeftStartOfSegment, pRemainingBitsInSegment, &numDecodedBits); /* result is written out here because NO sign bits follow the body */ for (i = dimension; i != 0; i--) { @@ -1115,14 +1114,14 @@ static void DecodePCWs(HANDLE_FDK_BITSTREAM bs, H_HCR_INFO pHcr) { int err; numDecodedBits = 0; - pQuantVal = - DecodePCW_Body(bs, pCurrentTree, pQuantValBase, pLeftStartOfSegment, - pRemainingBitsInSegment, &numDecodedBits); + pQuantVal = DecodePCW_Body( + bs, pHcr->decInOut.bitstreamAnchor, pCurrentTree, pQuantValBase, + pLeftStartOfSegment, pRemainingBitsInSegment, &numDecodedBits); err = DecodePCW_Sign( - bs, dimension, pQuantVal, pQuantizedSpectralCoefficients, - &quantizedSpectralCoefficientsIdx, pLeftStartOfSegment, - pRemainingBitsInSegment, &numDecodedBits); + bs, pHcr->decInOut.bitstreamAnchor, dimension, pQuantVal, + pQuantizedSpectralCoefficients, &quantizedSpectralCoefficientsIdx, + pLeftStartOfSegment, pRemainingBitsInSegment, &numDecodedBits); if (err != 0) { return; } @@ -1157,14 +1156,14 @@ static void DecodePCWs(HANDLE_FDK_BITSTREAM bs, H_HCR_INFO pHcr) { numDecodedBits = 0; /* decode PCW_BODY */ - pQuantVal = - DecodePCW_Body(bs, pCurrentTree, pQuantValBase, pLeftStartOfSegment, - pRemainingBitsInSegment, &numDecodedBits); + pQuantVal = DecodePCW_Body( + bs, pHcr->decInOut.bitstreamAnchor, pCurrentTree, pQuantValBase, + pLeftStartOfSegment, pRemainingBitsInSegment, &numDecodedBits); err = DecodePCW_Sign( - bs, dimension, pQuantVal, pQuantizedSpectralCoefficients, - &quantizedSpectralCoefficientsIdx, pLeftStartOfSegment, - pRemainingBitsInSegment, &numDecodedBits); + bs, pHcr->decInOut.bitstreamAnchor, dimension, pQuantVal, + pQuantizedSpectralCoefficients, &quantizedSpectralCoefficientsIdx, + pLeftStartOfSegment, pRemainingBitsInSegment, &numDecodedBits); if (err != 0) { return; } @@ -1177,7 +1176,7 @@ static void DecodePCWs(HANDLE_FDK_BITSTREAM bs, H_HCR_INFO pHcr) { (FIXP_DBL)ESCAPE_VALUE) { pQuantizedSpectralCoefficients[quantizedSpectralCoefficientsIdx] = (FIXP_DBL)DecodeEscapeSequence( - bs, + bs, pHcr->decInOut.bitstreamAnchor, pQuantizedSpectralCoefficients [quantizedSpectralCoefficientsIdx], pLeftStartOfSegment, pRemainingBitsInSegment, @@ -1193,7 +1192,7 @@ static void DecodePCWs(HANDLE_FDK_BITSTREAM bs, H_HCR_INFO pHcr) { (FIXP_DBL)ESCAPE_VALUE) { pQuantizedSpectralCoefficients[quantizedSpectralCoefficientsIdx] = (FIXP_DBL)DecodeEscapeSequence( - bs, + bs, pHcr->decInOut.bitstreamAnchor, pQuantizedSpectralCoefficients [quantizedSpectralCoefficientsIdx], pLeftStartOfSegment, pRemainingBitsInSegment, @@ -1331,7 +1330,7 @@ void CarryBitToBranchValue(UCHAR carryBit, UINT treeNode, UINT *branchValue, spectral coefficients -------------------------------------------------------------------------------------------- */ -static const SCHAR *DecodePCW_Body(HANDLE_FDK_BITSTREAM bs, +static const SCHAR *DecodePCW_Body(HANDLE_FDK_BITSTREAM bs, const INT bsAnchor, const UINT *pCurrentTree, const SCHAR *pQuantValBase, INT *pLeftStartOfSegment, @@ -1349,7 +1348,7 @@ static const SCHAR *DecodePCW_Body(HANDLE_FDK_BITSTREAM bs, /* decode whole PCW-codeword-body */ while (1) { - carryBit = HcrGetABitFromBitstream(bs, pLeftStartOfSegment, + carryBit = HcrGetABitFromBitstream(bs, bsAnchor, pLeftStartOfSegment, pLeftStartOfSegment, /* dummy */ FROM_LEFT_TO_RIGHT); *pRemainingBitsInSegment -= 1; @@ -1384,8 +1383,8 @@ value == 16, a escapeSequence is decoded in two steps: -------------------------------------------------------------------------------------------- */ -static INT DecodeEscapeSequence(HANDLE_FDK_BITSTREAM bs, INT quantSpecCoef, - INT *pLeftStartOfSegment, +static INT DecodeEscapeSequence(HANDLE_FDK_BITSTREAM bs, const INT bsAnchor, + INT quantSpecCoef, INT *pLeftStartOfSegment, SCHAR *pRemainingBitsInSegment, int *pNumDecodedBits) { UINT i; @@ -1396,7 +1395,7 @@ static INT DecodeEscapeSequence(HANDLE_FDK_BITSTREAM bs, INT quantSpecCoef, /* decode escape prefix */ while (1) { - carryBit = HcrGetABitFromBitstream(bs, pLeftStartOfSegment, + carryBit = HcrGetABitFromBitstream(bs, bsAnchor, pLeftStartOfSegment, pLeftStartOfSegment, /* dummy */ FROM_LEFT_TO_RIGHT); *pRemainingBitsInSegment -= 1; @@ -1412,7 +1411,7 @@ static INT DecodeEscapeSequence(HANDLE_FDK_BITSTREAM bs, INT quantSpecCoef, /* decode escape word */ for (i = escapeOnesCounter; i != 0; i--) { - carryBit = HcrGetABitFromBitstream(bs, pLeftStartOfSegment, + carryBit = HcrGetABitFromBitstream(bs, bsAnchor, pLeftStartOfSegment, pLeftStartOfSegment, /* dummy */ FROM_LEFT_TO_RIGHT); *pRemainingBitsInSegment -= 1; @@ -1441,9 +1440,10 @@ the last of eight function of HCR) line) -------------------------------------------------------------------------------------------- */ -static int DecodePCW_Sign(HANDLE_FDK_BITSTREAM bs, UINT codebookDim, - const SCHAR *pQuantVal, FIXP_DBL *pQuantSpecCoef, - int *quantSpecCoefIdx, INT *pLeftStartOfSegment, +static int DecodePCW_Sign(HANDLE_FDK_BITSTREAM bs, const INT bsAnchor, + UINT codebookDim, const SCHAR *pQuantVal, + FIXP_DBL *pQuantSpecCoef, int *quantSpecCoefIdx, + INT *pLeftStartOfSegment, SCHAR *pRemainingBitsInSegment, int *pNumDecodedBits) { UINT i; @@ -1453,7 +1453,7 @@ static int DecodePCW_Sign(HANDLE_FDK_BITSTREAM bs, UINT codebookDim, for (i = codebookDim; i != 0; i--) { quantSpecCoef = *pQuantVal++; if (quantSpecCoef != 0) { - carryBit = HcrGetABitFromBitstream(bs, pLeftStartOfSegment, + carryBit = HcrGetABitFromBitstream(bs, bsAnchor, pLeftStartOfSegment, pLeftStartOfSegment, /* dummy */ FROM_LEFT_TO_RIGHT); *pRemainingBitsInSegment -= 1; diff --git a/libAACdec/src/aacdec_hcr_bit.cpp b/libAACdec/src/aacdec_hcr_bit.cpp index a53ef16..0198659 100644 --- a/libAACdec/src/aacdec_hcr_bit.cpp +++ b/libAACdec/src/aacdec_hcr_bit.cpp @@ -132,13 +132,14 @@ read direction. It is called very often, therefore it makes sense to inline it return: - bit from bitstream -------------------------------------------------------------------------------------------- */ -UINT HcrGetABitFromBitstream(HANDLE_FDK_BITSTREAM bs, INT *pLeftStartOfSegment, +UINT HcrGetABitFromBitstream(HANDLE_FDK_BITSTREAM bs, const INT bsAnchor, + INT *pLeftStartOfSegment, INT *pRightStartOfSegment, UCHAR readDirection) { UINT bit; INT readBitOffset; if (readDirection == FROM_LEFT_TO_RIGHT) { - readBitOffset = *pLeftStartOfSegment - FDKgetBitCnt(bs); + readBitOffset = (INT)FDKgetValidBits(bs) - bsAnchor + *pLeftStartOfSegment; if (readBitOffset) { FDKpushBiDirectional(bs, readBitOffset); } @@ -147,7 +148,7 @@ UINT HcrGetABitFromBitstream(HANDLE_FDK_BITSTREAM bs, INT *pLeftStartOfSegment, *pLeftStartOfSegment += 1; } else { - readBitOffset = *pRightStartOfSegment - FDKgetBitCnt(bs); + readBitOffset = (INT)FDKgetValidBits(bs) - bsAnchor + *pRightStartOfSegment; if (readBitOffset) { FDKpushBiDirectional(bs, readBitOffset); } diff --git a/libAACdec/src/aacdec_hcr_bit.h b/libAACdec/src/aacdec_hcr_bit.h index 7a57c8c..77242ac 100644 --- a/libAACdec/src/aacdec_hcr_bit.h +++ b/libAACdec/src/aacdec_hcr_bit.h @@ -107,7 +107,8 @@ amm-info@iis.fraunhofer.de UCHAR ToggleReadDirection(UCHAR readDirection); -UINT HcrGetABitFromBitstream(HANDLE_FDK_BITSTREAM bs, INT *pLeftStartOfSegment, +UINT HcrGetABitFromBitstream(HANDLE_FDK_BITSTREAM bs, const INT bsAnchor, + INT *pLeftStartOfSegment, INT *pRightStartOfSegment, UCHAR readDirection); #endif /* AACDEC_HCR_BIT_H */ diff --git a/libAACdec/src/aacdec_hcr_types.h b/libAACdec/src/aacdec_hcr_types.h index d550bc2..1cc3cb0 100644 --- a/libAACdec/src/aacdec_hcr_types.h +++ b/libAACdec/src/aacdec_hcr_types.h @@ -350,7 +350,7 @@ typedef struct { SHORT lengthOfReorderedSpectralData; SHORT numSection; SHORT *pNumLineInSect; - INT bitstreamIndex; + INT bitstreamAnchor; SCHAR lengthOfLongestCodeword; UCHAR *pCodebook; } HCR_INPUT_OUTPUT; diff --git a/libAACdec/src/aacdec_hcrs.cpp b/libAACdec/src/aacdec_hcrs.cpp index e2b7cd8..1d5aa27 100644 --- a/libAACdec/src/aacdec_hcrs.cpp +++ b/libAACdec/src/aacdec_hcrs.cpp @@ -615,9 +615,9 @@ UINT Hcr_State_BODY_ONLY(HANDLE_FDK_BITSTREAM bs, void *ptr) { for (; pRemainingBitsInSegment[segmentOffset] > 0; pRemainingBitsInSegment[segmentOffset] -= 1) { - carryBit = HcrGetABitFromBitstream(bs, &pLeftStartOfSegment[segmentOffset], - &pRightStartOfSegment[segmentOffset], - readDirection); + carryBit = HcrGetABitFromBitstream( + bs, pHcr->decInOut.bitstreamAnchor, &pLeftStartOfSegment[segmentOffset], + &pRightStartOfSegment[segmentOffset], readDirection); CarryBitToBranchValue(carryBit, /* make a step in decoding tree */ treeNode, &branchValue, &branchNode); @@ -749,9 +749,9 @@ UINT Hcr_State_BODY_SIGN__BODY(HANDLE_FDK_BITSTREAM bs, void *ptr) { for (; pRemainingBitsInSegment[segmentOffset] > 0; pRemainingBitsInSegment[segmentOffset] -= 1) { - carryBit = HcrGetABitFromBitstream(bs, &pLeftStartOfSegment[segmentOffset], - &pRightStartOfSegment[segmentOffset], - readDirection); + carryBit = HcrGetABitFromBitstream( + bs, pHcr->decInOut.bitstreamAnchor, &pLeftStartOfSegment[segmentOffset], + &pRightStartOfSegment[segmentOffset], readDirection); CarryBitToBranchValue(carryBit, /* make a step in decoding tree */ treeNode, &branchValue, &branchNode); @@ -884,9 +884,9 @@ UINT Hcr_State_BODY_SIGN__SIGN(HANDLE_FDK_BITSTREAM bs, void *ptr) { /* loop for sign bit decoding */ for (; pRemainingBitsInSegment[segmentOffset] > 0; pRemainingBitsInSegment[segmentOffset] -= 1) { - carryBit = HcrGetABitFromBitstream(bs, &pLeftStartOfSegment[segmentOffset], - &pRightStartOfSegment[segmentOffset], - readDirection); + carryBit = HcrGetABitFromBitstream( + bs, pHcr->decInOut.bitstreamAnchor, &pLeftStartOfSegment[segmentOffset], + &pRightStartOfSegment[segmentOffset], readDirection); cntSign -= 1; /* decrement sign counter because one sign bit has been read */ @@ -997,9 +997,9 @@ UINT Hcr_State_BODY_SIGN_ESC__BODY(HANDLE_FDK_BITSTREAM bs, void *ptr) { for (; pRemainingBitsInSegment[segmentOffset] > 0; pRemainingBitsInSegment[segmentOffset] -= 1) { - carryBit = HcrGetABitFromBitstream(bs, &pLeftStartOfSegment[segmentOffset], - &pRightStartOfSegment[segmentOffset], - readDirection); + carryBit = HcrGetABitFromBitstream( + bs, pHcr->decInOut.bitstreamAnchor, &pLeftStartOfSegment[segmentOffset], + &pRightStartOfSegment[segmentOffset], readDirection); /* make a step in tree */ CarryBitToBranchValue(carryBit, treeNode, &branchValue, &branchNode); @@ -1159,9 +1159,9 @@ UINT Hcr_State_BODY_SIGN_ESC__SIGN(HANDLE_FDK_BITSTREAM bs, void *ptr) { /* loop for sign bit decoding */ for (; pRemainingBitsInSegment[segmentOffset] > 0; pRemainingBitsInSegment[segmentOffset] -= 1) { - carryBit = HcrGetABitFromBitstream(bs, &pLeftStartOfSegment[segmentOffset], - &pRightStartOfSegment[segmentOffset], - readDirection); + carryBit = HcrGetABitFromBitstream( + bs, pHcr->decInOut.bitstreamAnchor, &pLeftStartOfSegment[segmentOffset], + &pRightStartOfSegment[segmentOffset], readDirection); /* decrement sign counter because one sign bit has been read */ cntSign -= 1; @@ -1314,9 +1314,9 @@ UINT Hcr_State_BODY_SIGN_ESC__ESC_PREFIX(HANDLE_FDK_BITSTREAM bs, void *ptr) { /* decode escape prefix */ for (; pRemainingBitsInSegment[segmentOffset] > 0; pRemainingBitsInSegment[segmentOffset] -= 1) { - carryBit = HcrGetABitFromBitstream(bs, &pLeftStartOfSegment[segmentOffset], - &pRightStartOfSegment[segmentOffset], - readDirection); + carryBit = HcrGetABitFromBitstream( + bs, pHcr->decInOut.bitstreamAnchor, &pLeftStartOfSegment[segmentOffset], + &pRightStartOfSegment[segmentOffset], readDirection); /* count ones and store sum in escapePrefixUp */ if (carryBit == 1) { @@ -1435,9 +1435,9 @@ UINT Hcr_State_BODY_SIGN_ESC__ESC_WORD(HANDLE_FDK_BITSTREAM bs, void *ptr) { /* decode escape word */ for (; pRemainingBitsInSegment[segmentOffset] > 0; pRemainingBitsInSegment[segmentOffset] -= 1) { - carryBit = HcrGetABitFromBitstream(bs, &pLeftStartOfSegment[segmentOffset], - &pRightStartOfSegment[segmentOffset], - readDirection); + carryBit = HcrGetABitFromBitstream( + bs, pHcr->decInOut.bitstreamAnchor, &pLeftStartOfSegment[segmentOffset], + &pRightStartOfSegment[segmentOffset], readDirection); /* build escape word */ escapeWord <<= diff --git a/libAACdec/src/rvlc.cpp b/libAACdec/src/rvlc.cpp index 92f9f02..b7a9be1 100644 --- a/libAACdec/src/rvlc.cpp +++ b/libAACdec/src/rvlc.cpp @@ -168,13 +168,14 @@ static void rvlcInit(CErRvlcInfo *pRvlc, /* set base bitstream ptr to the RVL-coded part (start of RVLC data (ESC 2)) */ FDKsyncCache(bs); + pRvlc->bsAnchor = (INT)FDKgetValidBits(bs); - pRvlc->bitstreamIndexRvlFwd = FDKgetBitCnt( - bs); /* first bit within RVL coded block as start address for forward - decoding */ - pRvlc->bitstreamIndexRvlBwd = FDKgetBitCnt(bs) + pRvlc->length_of_rvlc_sf - - 1; /* last bit within RVL coded block as start - address for backward decoding */ + pRvlc->bitstreamIndexRvlFwd = + 0; /* first bit within RVL coded block as start address for forward + decoding */ + pRvlc->bitstreamIndexRvlBwd = + pRvlc->length_of_rvlc_sf - 1; /* last bit within RVL coded block as start + address for backward decoding */ /* skip RVLC-bitstream-part -- pointing now to escapes (if present) or to TNS * data (if present) */ @@ -183,7 +184,7 @@ static void rvlcInit(CErRvlcInfo *pRvlc, if (pRvlc->sf_escapes_present != 0) { /* locate internal bitstream ptr at escapes (which is the second part) */ FDKsyncCache(bs); - pRvlc->bitstreamIndexEsc = FDKgetBitCnt(bs); + pRvlc->bitstreamIndexEsc = pRvlc->bsAnchor - (INT)FDKgetValidBits(bs); /* skip escapeRVLC-bitstream-part -- pointing to TNS data (if present) to * make decoder continue */ @@ -259,8 +260,9 @@ static SCHAR rvlcDecodeEscapeWord(CErRvlcInfo *pRvlc, HANDLE_FDK_BITSTREAM bs) { treeNode = *pEscTree; /* init at starting node */ for (i = MAX_LEN_RVLC_ESCAPE_WORD - 1; i >= 0; i--) { - carryBit = rvlcReadBitFromBitstream(bs, /* get next bit */ - pBitstreamIndexEsc, FWD); + carryBit = + rvlcReadBitFromBitstream(bs, /* get next bit */ + pRvlc->bsAnchor, pBitstreamIndexEsc, FWD); CarryBitToBranchValue(carryBit, /* huffman decoding, do a single step in huffman decoding tree */ @@ -370,8 +372,9 @@ SCHAR decodeRVLCodeword(HANDLE_FDK_BITSTREAM bs, CErRvlcInfo *pRvlc) { UINT treeNode = *pRvlCodeTree; for (i = MAX_LEN_RVLC_CODE_WORD - 1; i >= 0; i--) { - carryBit = rvlcReadBitFromBitstream(bs, /* get next bit */ - pBitstrIndxRvl, direction); + carryBit = + rvlcReadBitFromBitstream(bs, /* get next bit */ + pRvlc->bsAnchor, pBitstrIndxRvl, direction); CarryBitToBranchValue(carryBit, /* huffman decoding, do a single step in huffman decoding tree */ @@ -1140,7 +1143,7 @@ void CRvlc_Decode(CAacDecoderChannelInfo *pAacDecoderChannelInfo, rvlcInit(pRvlc, pAacDecoderChannelInfo, bs); /* save bitstream position */ - saveBitCnt = FDKgetBitCnt(bs); + saveBitCnt = (INT)FDKgetValidBits(bs); if (pRvlc->sf_escapes_present) rvlcDecodeEscapes( @@ -1155,7 +1158,7 @@ void CRvlc_Decode(CAacDecoderChannelInfo *pAacDecoderChannelInfo, pAacDecoderChannelInfo->data.aac.PnsData.PnsActive = pRvlc->noise_used; /* restore bitstream position */ - bitCntOffst = saveBitCnt - FDKgetBitCnt(bs); + bitCntOffst = (INT)FDKgetValidBits(bs) - saveBitCnt; if (bitCntOffst) { FDKpushBiDirectional(bs, bitCntOffst); } diff --git a/libAACdec/src/rvlc_info.h b/libAACdec/src/rvlc_info.h index fc9c19d..e7b3b99 100644 --- a/libAACdec/src/rvlc_info.h +++ b/libAACdec/src/rvlc_info.h @@ -164,6 +164,7 @@ typedef struct { UCHAR direction; /* bitstream indices */ + INT bsAnchor; /* hcr bit buffer reference index */ INT bitstreamIndexRvlFwd; /* base address of RVL-coded-scalefactor data (ESC 2) for forward decoding */ INT bitstreamIndexRvlBwd; /* base address of RVL-coded-scalefactor data (ESC diff --git a/libAACdec/src/rvlcbit.cpp b/libAACdec/src/rvlcbit.cpp index c06cf96..b0c4596 100644 --- a/libAACdec/src/rvlcbit.cpp +++ b/libAACdec/src/rvlcbit.cpp @@ -123,10 +123,10 @@ read direction. It is called very often, therefore it makes sense to inline it -------------------------------------------------------------------------------------------- */ -UCHAR rvlcReadBitFromBitstream(HANDLE_FDK_BITSTREAM bs, INT *pPosition, - UCHAR readDirection) { +UCHAR rvlcReadBitFromBitstream(HANDLE_FDK_BITSTREAM bs, const INT bsAnchor, + INT *pPosition, UCHAR readDirection) { UINT bit; - INT readBitOffset = *pPosition - FDKgetBitCnt(bs); + INT readBitOffset = (INT)FDKgetValidBits(bs) - bsAnchor + *pPosition; if (readBitOffset) { FDKpushBiDirectional(bs, readBitOffset); diff --git a/libAACdec/src/rvlcbit.h b/libAACdec/src/rvlcbit.h index 5c6a3f1..2578453 100644 --- a/libAACdec/src/rvlcbit.h +++ b/libAACdec/src/rvlcbit.h @@ -105,7 +105,7 @@ amm-info@iis.fraunhofer.de #include "rvlc.h" -UCHAR rvlcReadBitFromBitstream(HANDLE_FDK_BITSTREAM bs, INT *pPosition, - UCHAR readDirection); +UCHAR rvlcReadBitFromBitstream(HANDLE_FDK_BITSTREAM bs, const INT bsAnchor, + INT *pPosition, UCHAR readDirection); #endif /* RVLCBIT_H */ diff --git a/libFDK/include/FDK_bitbuffer.h b/libFDK/include/FDK_bitbuffer.h index ed0b2f6..19a24b3 100644 --- a/libFDK/include/FDK_bitbuffer.h +++ b/libFDK/include/FDK_bitbuffer.h @@ -113,7 +113,6 @@ typedef struct { UINT ValidBits; UINT ReadOffset; UINT WriteOffset; - UINT BitCnt; UINT BitNdx; UCHAR *Buffer; @@ -159,15 +158,10 @@ void FDK_pushBack(HANDLE_FDK_BITBUF hBitBuffer, const UINT numberOfBits, void FDK_pushForward(HANDLE_FDK_BITBUF hBitBuffer, const UINT numberOfBits, UCHAR config); -void FDK_byteAlign(HANDLE_FDK_BITBUF hBitBuffer, UCHAR config); - UINT FDK_getValidBits(HANDLE_FDK_BITBUF hBitBuffer); INT FDK_getFreeBits(HANDLE_FDK_BITBUF hBitBuffer); -void FDK_setBitCnt(HANDLE_FDK_BITBUF hBitBuffer, const UINT value); -INT FDK_getBitCnt(HANDLE_FDK_BITBUF hBitBuffer); - void FDK_Feed(HANDLE_FDK_BITBUF hBitBuffer, const UCHAR inputBuffer[], const UINT bufferSize, UINT *bytesValid); diff --git a/libFDK/include/FDK_bitstream.h b/libFDK/include/FDK_bitstream.h index 49eeeaf..f799026 100644 --- a/libFDK/include/FDK_bitstream.h +++ b/libFDK/include/FDK_bitstream.h @@ -480,21 +480,6 @@ FDK_INLINE void FDKsyncCacheBwd(HANDLE_FDK_BITSTREAM hBitStream) { hBitStream->CacheWord = 0; } -/** - * \brief Byte Alignment Function. - * This function performs the byte_alignment() syntactic function on the - * input stream, i.e. some bits will be discarded/padded so that the next bits - * to be read/written will be aligned on a byte boundary with respect to - * the bit position 0. - * - * \param hBitStream HANDLE_FDK_BITSTREAM handle - * \return void - */ -FDK_INLINE void FDKbyteAlign(HANDLE_FDK_BITSTREAM hBitStream) { - FDKsyncCache(hBitStream); - FDK_byteAlign(&hBitStream->hBitBuf, (UCHAR)hBitStream->ConfigCache); -} - /** * \brief Byte Alignment Function with anchor * This function performs the byte_alignment() syntactic function on the @@ -603,37 +588,6 @@ FDK_INLINE INT FDKgetFreeBits(HANDLE_FDK_BITSTREAM hBitStream) { return FDK_getFreeBits(&hBitStream->hBitBuf); } -/** - * \brief reset bitcounter in bitBuffer to zero. - * \param hBitStream HANDLE_FDK_BITSTREAM handle - * \return void - */ -FDK_INLINE void FDKresetBitCnt(HANDLE_FDK_BITSTREAM hBitStream) { - FDKsyncCache(hBitStream); - FDK_setBitCnt(&hBitStream->hBitBuf, 0); -} - -/** - * \brief set bitcoutner in bitBuffer to given value. - * \param hBitStream HANDLE_FDK_BITSTREAM handle - * \param value new value to be assigned to the bit counter - * \return void - */ -FDK_INLINE void FDKsetBitCnt(HANDLE_FDK_BITSTREAM hBitStream, UINT value) { - FDKsyncCache(hBitStream); - FDK_setBitCnt(&hBitStream->hBitBuf, value); -} - -/** - * \brief get bitcounter state from bitBuffer. - * \param hBitStream HANDLE_FDK_BITSTREAM handle - * \return current bit counter value - */ -FDK_INLINE INT FDKgetBitCnt(HANDLE_FDK_BITSTREAM hBitStream) { - FDKsyncCache(hBitStream); - return FDK_getBitCnt(&hBitStream->hBitBuf); -} - /** * \brief Fill the BitBuffer with a number of input bytes from external source. * The bytesValid variable returns the number of ramaining valid bytes in diff --git a/libFDK/src/FDK_bitbuffer.cpp b/libFDK/src/FDK_bitbuffer.cpp index a990c58..98905ea 100644 --- a/libFDK/src/FDK_bitbuffer.cpp +++ b/libFDK/src/FDK_bitbuffer.cpp @@ -128,7 +128,6 @@ void FDK_InitBitBuffer(HANDLE_FDK_BITBUF hBitBuf, UCHAR *pBuffer, UINT bufSize, hBitBuf->ValidBits = validBits; hBitBuf->ReadOffset = 0; hBitBuf->WriteOffset = 0; - hBitBuf->BitCnt = 0; hBitBuf->BitNdx = 0; hBitBuf->Buffer = pBuffer; @@ -151,7 +150,6 @@ void FDK_ResetBitBuffer(HANDLE_FDK_BITBUF hBitBuf) { hBitBuf->ValidBits = 0; hBitBuf->ReadOffset = 0; hBitBuf->WriteOffset = 0; - hBitBuf->BitCnt = 0; hBitBuf->BitNdx = 0; } @@ -161,7 +159,6 @@ INT FDK_get(HANDLE_FDK_BITBUF hBitBuf, const UINT numberOfBits) { UINT bitOffset = hBitBuf->BitNdx & 0x07; hBitBuf->BitNdx = (hBitBuf->BitNdx + numberOfBits) & (hBitBuf->bufBits - 1); - hBitBuf->BitCnt += numberOfBits; hBitBuf->ValidBits -= numberOfBits; UINT byteMask = hBitBuf->bufSize - 1; @@ -184,7 +181,6 @@ INT FDK_get(HANDLE_FDK_BITBUF hBitBuf, const UINT numberOfBits) { INT FDK_get32(HANDLE_FDK_BITBUF hBitBuf) { UINT BitNdx = hBitBuf->BitNdx + 32; hBitBuf->BitNdx = BitNdx & (hBitBuf->bufBits - 1); - hBitBuf->BitCnt += 32; hBitBuf->ValidBits = (UINT)((INT)hBitBuf->ValidBits - (INT)32); UINT byteOffset = (BitNdx - 1) >> 3; @@ -223,7 +219,6 @@ INT FDK_getBwd(HANDLE_FDK_BITBUF hBitBuf, const UINT numberOfBits) { int i; hBitBuf->BitNdx = (hBitBuf->BitNdx - numberOfBits) & (hBitBuf->bufBits - 1); - hBitBuf->BitCnt -= numberOfBits; hBitBuf->ValidBits += numberOfBits; UINT tx = hBitBuf->Buffer[(byteOffset - 3) & byteMask] << 24 | @@ -256,7 +251,6 @@ void FDK_put(HANDLE_FDK_BITBUF hBitBuf, UINT value, const UINT numberOfBits) { UINT bitOffset = hBitBuf->BitNdx & 0x7; hBitBuf->BitNdx = (hBitBuf->BitNdx + numberOfBits) & (hBitBuf->bufBits - 1); - hBitBuf->BitCnt += numberOfBits; hBitBuf->ValidBits += numberOfBits; UINT byteMask = hBitBuf->bufSize - 1; @@ -307,7 +301,6 @@ void FDK_putBwd(HANDLE_FDK_BITBUF hBitBuf, UINT value, int i; hBitBuf->BitNdx = (hBitBuf->BitNdx - numberOfBits) & (hBitBuf->bufBits - 1); - hBitBuf->BitCnt -= numberOfBits; hBitBuf->ValidBits -= numberOfBits; /* in place turn around */ @@ -344,7 +337,6 @@ void FDK_putBwd(HANDLE_FDK_BITBUF hBitBuf, UINT value, #ifndef FUNCTION_FDK_pushBack void FDK_pushBack(HANDLE_FDK_BITBUF hBitBuf, const UINT numberOfBits, UCHAR config) { - hBitBuf->BitCnt = (UINT)((INT)hBitBuf->BitCnt - (INT)numberOfBits); hBitBuf->ValidBits = (config == 0) ? (UINT)((INT)hBitBuf->ValidBits + (INT)numberOfBits) : ((UINT)((INT)hBitBuf->ValidBits - (INT)numberOfBits)); @@ -355,7 +347,6 @@ void FDK_pushBack(HANDLE_FDK_BITBUF hBitBuf, const UINT numberOfBits, void FDK_pushForward(HANDLE_FDK_BITBUF hBitBuf, const UINT numberOfBits, UCHAR config) { - hBitBuf->BitCnt = (UINT)((INT)hBitBuf->BitCnt + (INT)numberOfBits); hBitBuf->ValidBits = (config == 0) ? ((UINT)((INT)hBitBuf->ValidBits - (INT)numberOfBits)) : (UINT)((INT)hBitBuf->ValidBits + (INT)numberOfBits); @@ -363,19 +354,6 @@ void FDK_pushForward(HANDLE_FDK_BITBUF hBitBuf, const UINT numberOfBits, (UINT)((INT)hBitBuf->BitNdx + (INT)numberOfBits) & (hBitBuf->bufBits - 1); } -void FDK_byteAlign(HANDLE_FDK_BITBUF hBitBuf, UCHAR config) { - INT alignment = hBitBuf->BitCnt & 0x07; - - if (alignment) { - if (config == 0) - FDK_pushForward(hBitBuf, 8 - alignment, config); /* BS_READER */ - else - FDK_put(hBitBuf, 0, 8 - alignment); /* BS_WRITER */ - } - - hBitBuf->BitCnt = 0; -} - #ifndef FUNCTION_FDK_getValidBits UINT FDK_getValidBits(HANDLE_FDK_BITBUF hBitBuf) { return hBitBuf->ValidBits; } #endif /* #ifndef FUNCTION_FDK_getValidBits */ @@ -384,12 +362,6 @@ INT FDK_getFreeBits(HANDLE_FDK_BITBUF hBitBuf) { return (hBitBuf->bufBits - hBitBuf->ValidBits); } -void FDK_setBitCnt(HANDLE_FDK_BITBUF hBitBuf, const UINT value) { - hBitBuf->BitCnt = value; -} - -INT FDK_getBitCnt(HANDLE_FDK_BITBUF hBitBuf) { return hBitBuf->BitCnt; } - void FDK_Feed(HANDLE_FDK_BITBUF hBitBuf, const UCHAR *RESTRICT inputBuffer, const UINT bufferSize, UINT *bytesValid) { inputBuffer = &inputBuffer[bufferSize - *bytesValid]; @@ -438,7 +410,6 @@ void CopyAlignedBlock(HANDLE_FDK_BITBUF h_BitBufSrc, UCHAR *RESTRICT dstBuffer, h_BitBufSrc->BitNdx = (h_BitBufSrc->BitNdx + bToRead) & (h_BitBufSrc->bufBits - 1); - h_BitBufSrc->BitCnt += bToRead; h_BitBufSrc->ValidBits -= bToRead; } From ba003785774efe355bcac950158fc78a0cff0c2b Mon Sep 17 00:00:00 2001 From: Fraunhofer IIS FDK Date: Fri, 29 Jun 2018 16:35:24 +0200 Subject: [PATCH 2/9] Add sampling rate sanity check Bug: 112661641 Test: atest DecoderTestXheAac ; atest DecoderTestAacDrc Change-Id: I8e416fb1501dabda20babd4a28a99ab06950b221 --- libMpegTPDec/src/tpdec_asc.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libMpegTPDec/src/tpdec_asc.cpp b/libMpegTPDec/src/tpdec_asc.cpp index b0f1c6a..8d411e8 100644 --- a/libMpegTPDec/src/tpdec_asc.cpp +++ b/libMpegTPDec/src/tpdec_asc.cpp @@ -2102,7 +2102,9 @@ TRANSPORTDEC_ERROR AudioSpecificConfig_Parse( self->m_aot = getAOT(bs); self->m_samplingFrequency = getSampleRate(bs, &self->m_samplingFrequencyIndex, 4); - if (self->m_samplingFrequency <= 0) { + if (self->m_samplingFrequency <= 0 || + (self->m_samplingFrequency > 96000 && self->m_aot != 39) || + self->m_samplingFrequency > 4 * 96000) { return TRANSPORTDEC_PARSE_ERROR; } From 3347cfb91a7ecabf5800d72e936f04ce44752bf3 Mon Sep 17 00:00:00 2001 From: Fraunhofer IIS FDK Date: Wed, 15 Aug 2018 14:35:00 +0200 Subject: [PATCH 3/9] Break audio element loop in case element_count becomes too large. Bug: 112891564 Test: atest DecoderTestXheAac ; atest DecoderTestAacDrc Change-Id: I35f02d23c0cfd620088291a52d9996a0d5a17199 --- libAACdec/src/aacdecoder.cpp | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/libAACdec/src/aacdecoder.cpp b/libAACdec/src/aacdecoder.cpp index b8b1327..362e0b6 100644 --- a/libAACdec/src/aacdecoder.cpp +++ b/libAACdec/src/aacdecoder.cpp @@ -2519,8 +2519,14 @@ LINKSPEC_CPP AAC_DECODER_ERROR CAacDecoder_DecodeFrame( if (!(self->flags[0] & (AC_USAC | AC_RSVD50 | AC_RSV603DA | AC_ELD | AC_SCALABLE | AC_ER))) type = (MP4_ELEMENT_ID)FDKreadBits(bs, 3); - else + else { + if (element_count >= (3 * ((8) * 2) + (((8) * 2)) / 2 + 4 * (1) + 1)) { + self->frameOK = 0; + ErrorStatus = AAC_DEC_PARSE_ERROR; + break; + } type = self->elements[element_count]; + } if ((self->flags[streamIndex] & (AC_USAC | AC_RSVD50) && element_count == 0) || @@ -2564,6 +2570,11 @@ LINKSPEC_CPP AAC_DECODER_ERROR CAacDecoder_DecodeFrame( case ID_USAC_SCE: case ID_USAC_CPE: case ID_USAC_LFE: + if (element_count >= (3 * ((8) * 2) + (((8) * 2)) / 2 + 4 * (1) + 1)) { + self->frameOK = 0; + ErrorStatus = AAC_DEC_PARSE_ERROR; + break; + } el_channels = CAacDecoder_GetELChannels( type, self->usacStereoConfigIndex[element_count]); @@ -2795,12 +2806,24 @@ LINKSPEC_CPP AAC_DECODER_ERROR CAacDecoder_DecodeFrame( } break; case ID_EXT: + if (element_count >= (3 * ((8) * 2) + (((8) * 2)) / 2 + 4 * (1) + 1)) { + self->frameOK = 0; + ErrorStatus = AAC_DEC_PARSE_ERROR; + break; + } + ErrorStatus = aacDecoder_ParseExplicitMpsAndSbr( self, bs, previous_element, previous_element_index, element_count, el_cnt); break; case ID_USAC_EXT: { + if ((element_count - element_count_prev_streams) >= + TP_USAC_MAX_ELEMENTS) { + self->frameOK = 0; + ErrorStatus = AAC_DEC_PARSE_ERROR; + break; + } /* parse extension element payload q.v. rsv603daExtElement() ISO/IEC DIS 23008-3 Table 30 or UsacExElement() ISO/IEC FDIS 23003-3:2011(E) Table 21 From f44b50b83529604f1b22e77084f50d575262c4fc Mon Sep 17 00:00:00 2001 From: Fraunhofer IIS FDK Date: Wed, 15 Aug 2018 14:42:23 +0200 Subject: [PATCH 4/9] Prevent overflow in concealment clipping check Bug: 112890225 Test: atest DecoderTestXheAac ; atest DecoderTestAacDrc Change-Id: Ie386e4b6fe5cdb38180f673edde8f84c36c7b522 --- libAACdec/src/conceal.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libAACdec/src/conceal.cpp b/libAACdec/src/conceal.cpp index a6064b6..569d672 100644 --- a/libAACdec/src/conceal.cpp +++ b/libAACdec/src/conceal.cpp @@ -2080,11 +2080,11 @@ static void CConcealment_TDNoise_Apply(CConcealmentInfo *const pConcealmentInfo, noiseVal = FX_DBL2FX_PCM(fMult(noiseValLong, TDNoiseAtt)); /* add filtered noise - check for clipping, before */ - if (pcmdata[ii] > (FIXP_PCM)MAXVAL_FIXP_PCM - noiseVal && - noiseVal > (FIXP_PCM)0) { + if (noiseVal > (FIXP_PCM)0 && + pcmdata[ii] > (FIXP_PCM)MAXVAL_FIXP_PCM - noiseVal) { noiseVal = noiseVal * (FIXP_PCM)-1; - } else if (pcmdata[ii] < (FIXP_PCM)MINVAL_FIXP_PCM - noiseVal && - noiseVal < (FIXP_PCM)0) { + } else if (noiseVal < (FIXP_PCM)0 && + pcmdata[ii] < (FIXP_PCM)MINVAL_FIXP_PCM - noiseVal) { noiseVal = noiseVal * (FIXP_PCM)-1; } From f2bc07da2ed70eb069f3faab1179c4c89792bf3d Mon Sep 17 00:00:00 2001 From: Fraunhofer IIS FDK Date: Wed, 15 Aug 2018 14:33:56 +0200 Subject: [PATCH 5/9] Unify audio element loop abort criterion in ER syntax Bug: 112891548 Test: atest DecoderTestXheAac ; atest DecoderTestAacDrc Change-Id: Iea56cf804cfb9d396810124c718fc91bdff68392 --- libAACdec/src/aacdecoder.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/libAACdec/src/aacdecoder.cpp b/libAACdec/src/aacdecoder.cpp index b8b1327..272de9f 100644 --- a/libAACdec/src/aacdecoder.cpp +++ b/libAACdec/src/aacdecoder.cpp @@ -2055,17 +2055,12 @@ CAacDecoder_Init(HANDLE_AACDECODER self, const CSAudioSpecificConfig *asc, if (self->flags[streamIndex] & (AC_RSV603DA | AC_USAC)) { _numElements = (int)asc->m_sc.m_usacConfig.m_usacNumElements; } - if (self->flags[streamIndex] & (AC_ER | AC_LD | AC_ELD)) { - _numElements = (asc->m_channelConfiguration == 7) - ? 8 - : asc->m_channelConfiguration; - } for (int _el = 0; _el < _numElements; _el++) { int el_channels = 0; int el = elementOffset + _el; if (self->flags[streamIndex] & - (AC_ELD | AC_RSV603DA | AC_USAC | AC_RSVD50)) { + (AC_ER | AC_LD | AC_ELD | AC_RSV603DA | AC_USAC | AC_RSVD50)) { if (ch >= ascChannels) { break; } @@ -2115,7 +2110,9 @@ CAacDecoder_Init(HANDLE_AACDECODER self, const CSAudioSpecificConfig *asc, (SPECTRAL_PTR)&self->workBufferCore2[ch * 1024]; if (el_channels == 2) { - FDK_ASSERT(ch < (8) - 1); + if (ch >= (8) - 1) { + return AAC_DEC_UNSUPPORTED_CHANNELCONFIG; + } self->pAacDecoderChannelInfo[ch + 1]->pComData = self->pAacDecoderChannelInfo[ch]->pComData; self->pAacDecoderChannelInfo[ch + 1]->pComStaticData = From 25b209f229879a155759d791fe463b8abd283677 Mon Sep 17 00:00:00 2001 From: Fraunhofer IIS FDK Date: Fri, 8 Jun 2018 18:07:14 +0200 Subject: [PATCH 6/9] Always check whether given channel config is supported. Bug: 112660981 Test: atest DecoderTestXheAac ; atest DecoderTestAacDrc Change-Id: I169161dd31bc624f2cab6be2b4c6518946ed32ba Merged-In: I169161dd31bc624f2cab6be2b4c6518946ed32ba --- libAACdec/src/aacdecoder.cpp | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/libAACdec/src/aacdecoder.cpp b/libAACdec/src/aacdecoder.cpp index 362e0b6..fab30de 100644 --- a/libAACdec/src/aacdecoder.cpp +++ b/libAACdec/src/aacdecoder.cpp @@ -1630,17 +1630,9 @@ CAacDecoder_Init(HANDLE_AACDECODER self, const CSAudioSpecificConfig *asc, aacChannelsOffset = 0; aacChannelsOffsetIdx = 0; elementOffset = 0; - if (configMode & AC_CM_ALLOC_MEM) { - if ((ascChannels <= 0) || - (asc->m_channelConfiguration > AACDEC_MAX_CH_CONF)) { - return AAC_DEC_UNSUPPORTED_CHANNELCONFIG; - } - if ((ascChannels + aacChannelsOffsetIdx) > ((8) * 2)) { - return AAC_DEC_UNSUPPORTED_CHANNELCONFIG; - } - if ((ascChannels + aacChannelsOffset) > (8)) { - return AAC_DEC_UNSUPPORTED_CHANNELCONFIG; - } + if ((ascChannels <= 0) || (ascChannels > (8)) || + (asc->m_channelConfiguration > AACDEC_MAX_CH_CONF)) { + return AAC_DEC_UNSUPPORTED_CHANNELCONFIG; } /* Set syntax flags */ From 4dad829df00932b89858b9833cf5dcded8d97c37 Mon Sep 17 00:00:00 2001 From: Fraunhofer IIS FDK Date: Mon, 10 Sep 2018 16:39:30 +0200 Subject: [PATCH 7/9] Prevent out of bounds accesses in lppTransposer() and lppTransposerHBE() Bug: 112160868 Test: see poc in bug Change-Id: I6a2161865d9cb9b51dc37c09d6e3a4a8e5d11f86 --- Android.bp | 3 ++ libSBRdec/src/lpp_tran.cpp | 74 ++++++++++++++++++++++++-------------- 2 files changed, 50 insertions(+), 27 deletions(-) diff --git a/Android.bp b/Android.bp index 50cc092..c89a95c 100644 --- a/Android.bp +++ b/Android.bp @@ -27,6 +27,9 @@ cc_library_static { misc_undefined:["unsigned-integer-overflow", "signed-integer-overflow"], cfi: true, }, + shared_libs: [ + "liblog", + ], export_include_dirs: [ "libAACdec/include", "libAACenc/include", diff --git a/libSBRdec/src/lpp_tran.cpp b/libSBRdec/src/lpp_tran.cpp index aa1fd5d..2ef07eb 100644 --- a/libSBRdec/src/lpp_tran.cpp +++ b/libSBRdec/src/lpp_tran.cpp @@ -118,6 +118,10 @@ amm-info@iis.fraunhofer.de \sa lppTransposer(), main_audio.cpp, sbr_scale.h, \ref documentationOverview */ +#ifdef __ANDROID__ +#include "log/log.h" +#endif + #include "lpp_tran.h" #include "sbr_ram.h" @@ -295,7 +299,6 @@ void lppTransposer( int ovLowBandShift; int lowBandShift; /* int ovHighBandShift;*/ - int targetStopBand; alphai[0] = FL2FXCONST_SGL(0.0f); alphai[1] = FL2FXCONST_SGL(0.0f); @@ -311,25 +314,34 @@ void lppTransposer( autoCorrLength = pSettings->nCols + pSettings->overlap; - /* Set upper subbands to zero: - This is required in case that the patches do not cover the complete - highband (because the last patch would be too short). Possible - optimization: Clearing bands up to usb would be sufficient here. */ - targetStopBand = patchParam[pSettings->noOfPatches - 1].targetStartBand + - patchParam[pSettings->noOfPatches - 1].numBandsInPatch; + if (pSettings->noOfPatches > 0) { + /* Set upper subbands to zero: + This is required in case that the patches do not cover the complete + highband (because the last patch would be too short). Possible + optimization: Clearing bands up to usb would be sufficient here. */ + int targetStopBand = + patchParam[pSettings->noOfPatches - 1].targetStartBand + + patchParam[pSettings->noOfPatches - 1].numBandsInPatch; - int memSize = ((64) - targetStopBand) * sizeof(FIXP_DBL); + int memSize = ((64) - targetStopBand) * sizeof(FIXP_DBL); - if (!useLP) { - for (i = startSample; i < stopSampleClear; i++) { - FDKmemclear(&qmfBufferReal[i][targetStopBand], memSize); - FDKmemclear(&qmfBufferImag[i][targetStopBand], memSize); - } - } else { - for (i = startSample; i < stopSampleClear; i++) { - FDKmemclear(&qmfBufferReal[i][targetStopBand], memSize); + if (!useLP) { + for (i = startSample; i < stopSampleClear; i++) { + FDKmemclear(&qmfBufferReal[i][targetStopBand], memSize); + FDKmemclear(&qmfBufferImag[i][targetStopBand], memSize); + } + } else { + for (i = startSample; i < stopSampleClear; i++) { + FDKmemclear(&qmfBufferReal[i][targetStopBand], memSize); + } } } +#ifdef __ANDROID__ + else { + // Safetynet logging + android_errorWriteLog(0x534e4554, "112160868"); + } +#endif /* init bwIndex for each patch */ FDKmemclear(bwIndex, sizeof(bwIndex)); @@ -874,7 +886,6 @@ void lppTransposerHBE( int ovLowBandShift; int lowBandShift; /* int ovHighBandShift;*/ - int targetStopBand; alphai[0] = FL2FXCONST_SGL(0.0f); alphai[1] = FL2FXCONST_SGL(0.0f); @@ -889,19 +900,28 @@ void lppTransposerHBE( autoCorrLength = pSettings->nCols + pSettings->overlap; - /* Set upper subbands to zero: - This is required in case that the patches do not cover the complete - highband (because the last patch would be too short). Possible - optimization: Clearing bands up to usb would be sufficient here. */ - targetStopBand = patchParam[pSettings->noOfPatches - 1].targetStartBand + - patchParam[pSettings->noOfPatches - 1].numBandsInPatch; + if (pSettings->noOfPatches > 0) { + /* Set upper subbands to zero: + This is required in case that the patches do not cover the complete + highband (because the last patch would be too short). Possible + optimization: Clearing bands up to usb would be sufficient here. */ + int targetStopBand = + patchParam[pSettings->noOfPatches - 1].targetStartBand + + patchParam[pSettings->noOfPatches - 1].numBandsInPatch; - int memSize = ((64) - targetStopBand) * sizeof(FIXP_DBL); + int memSize = ((64) - targetStopBand) * sizeof(FIXP_DBL); - for (i = startSample; i < stopSampleClear; i++) { - FDKmemclear(&qmfBufferReal[i][targetStopBand], memSize); - FDKmemclear(&qmfBufferImag[i][targetStopBand], memSize); + for (i = startSample; i < stopSampleClear; i++) { + FDKmemclear(&qmfBufferReal[i][targetStopBand], memSize); + FDKmemclear(&qmfBufferImag[i][targetStopBand], memSize); + } } +#ifdef __ANDROID__ + else { + // Safetynet logging + android_errorWriteLog(0x534e4554, "112160868"); + } +#endif /* Calc common low band scale factor From 56ef80d7fec1fd9e201262348a96b8660558105a Mon Sep 17 00:00:00 2001 From: Jean-Michel Trivi Date: Mon, 10 Sep 2018 15:50:19 -0700 Subject: [PATCH 8/9] Prevent out of bounds accesses in lppTransposer() Check validity of pSettings->noOfPatches to prevent out of bounds access in lppTransposer(), which can also cause memSize to be negative. Bug: 112160868 Test: see poc in bug Change-Id: I789030b116da7f8ea261001b43ef6c677dd58a3d Merged-In: I6a2161865d9cb9b51dc37c09d6e3a4a8e5d11f86 --- Android.bp | 3 +++ libSBRdec/src/lpp_tran.cpp | 37 ++++++++++++++++++++++++------------- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/Android.bp b/Android.bp index 75fe8af..6932b27 100644 --- a/Android.bp +++ b/Android.bp @@ -18,6 +18,9 @@ cc_library_static { "-Wno-constant-logical-operand", "-Wno-self-assign", ], + shared_libs: [ + "liblog", + ], export_include_dirs: [ "libAACdec/include", "libAACenc/include", diff --git a/libSBRdec/src/lpp_tran.cpp b/libSBRdec/src/lpp_tran.cpp index 343aec3..ae282d5 100644 --- a/libSBRdec/src/lpp_tran.cpp +++ b/libSBRdec/src/lpp_tran.cpp @@ -96,6 +96,10 @@ amm-info@iis.fraunhofer.de \sa lppTransposer(), main_audio.cpp, sbr_scale.h, \ref documentationOverview */ +#ifdef __ANDROID__ +#include "log/log.h" +#endif + #include "lpp_tran.h" #include "sbr_ram.h" @@ -256,7 +260,6 @@ void lppTransposer (HANDLE_SBR_LPP_TRANS hLppTrans, /*!< Handle of lpp transp int ovLowBandShift; int lowBandShift; /* int ovHighBandShift;*/ - int targetStopBand; alphai[0] = FL2FXCONST_SGL(0.0f); @@ -273,24 +276,32 @@ void lppTransposer (HANDLE_SBR_LPP_TRANS hLppTrans, /*!< Handle of lpp transp autoCorrLength = pSettings->nCols + pSettings->overlap; - /* Set upper subbands to zero: - This is required in case that the patches do not cover the complete highband - (because the last patch would be too short). - Possible optimization: Clearing bands up to usb would be sufficient here. */ - targetStopBand = patchParam[pSettings->noOfPatches-1].targetStartBand - + patchParam[pSettings->noOfPatches-1].numBandsInPatch; + if (pSettings->noOfPatches > 0) { + /* Set upper subbands to zero: + This is required in case that the patches do not cover the complete highband + (because the last patch would be too short). + Possible optimization: Clearing bands up to usb would be sufficient here. */ + int targetStopBand = patchParam[pSettings->noOfPatches-1].targetStartBand + + patchParam[pSettings->noOfPatches-1].numBandsInPatch; - int memSize = ((64) - targetStopBand) * sizeof(FIXP_DBL); + int memSize = ((64) - targetStopBand) * sizeof(FIXP_DBL); - if (!useLP) { + if (!useLP) { + for (i = startSample; i < stopSampleClear; i++) { + FDKmemclear(&qmfBufferReal[i][targetStopBand], memSize); + FDKmemclear(&qmfBufferImag[i][targetStopBand], memSize); + } + } else for (i = startSample; i < stopSampleClear; i++) { FDKmemclear(&qmfBufferReal[i][targetStopBand], memSize); - FDKmemclear(&qmfBufferImag[i][targetStopBand], memSize); } - } else - for (i = startSample; i < stopSampleClear; i++) { - FDKmemclear(&qmfBufferReal[i][targetStopBand], memSize); } +#ifdef __ANDROID__ + else { + // Safetynet logging + android_errorWriteLog(0x534e4554, "112160868"); + } +#endif /* init bwIndex for each patch */ FDKmemclear(bwIndex, MAX_NUM_PATCHES*sizeof(INT)); From 2eaadebcb6036b3e1850590378c0f56ca47e4735 Mon Sep 17 00:00:00 2001 From: Jean-Michel Trivi Date: Mon, 10 Sep 2018 15:50:19 -0700 Subject: [PATCH 9/9] DO NOT MERGE Prevent out of bounds accesses in lppTransposer() Check validity of pSettings->noOfPatches to prevent out of bounds access in lppTransposer(), which can also cause memSize to be negative. Bug: 112160868 Test: see poc in bug Change-Id: I77bd1e1dfab3bac92b4522170bdc3c9eb56fdf82 --- libSBRdec/src/lpp_tran.cpp | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/libSBRdec/src/lpp_tran.cpp b/libSBRdec/src/lpp_tran.cpp index 343aec3..c2ca732 100644 --- a/libSBRdec/src/lpp_tran.cpp +++ b/libSBRdec/src/lpp_tran.cpp @@ -96,6 +96,10 @@ amm-info@iis.fraunhofer.de \sa lppTransposer(), main_audio.cpp, sbr_scale.h, \ref documentationOverview */ +#ifdef __ANDROID__ +#include +#endif + #include "lpp_tran.h" #include "sbr_ram.h" @@ -256,7 +260,6 @@ void lppTransposer (HANDLE_SBR_LPP_TRANS hLppTrans, /*!< Handle of lpp transp int ovLowBandShift; int lowBandShift; /* int ovHighBandShift;*/ - int targetStopBand; alphai[0] = FL2FXCONST_SGL(0.0f); @@ -273,24 +276,32 @@ void lppTransposer (HANDLE_SBR_LPP_TRANS hLppTrans, /*!< Handle of lpp transp autoCorrLength = pSettings->nCols + pSettings->overlap; - /* Set upper subbands to zero: - This is required in case that the patches do not cover the complete highband - (because the last patch would be too short). - Possible optimization: Clearing bands up to usb would be sufficient here. */ - targetStopBand = patchParam[pSettings->noOfPatches-1].targetStartBand - + patchParam[pSettings->noOfPatches-1].numBandsInPatch; + if (pSettings->noOfPatches > 0) { + /* Set upper subbands to zero: + This is required in case that the patches do not cover the complete highband + (because the last patch would be too short). + Possible optimization: Clearing bands up to usb would be sufficient here. */ + int targetStopBand = patchParam[pSettings->noOfPatches-1].targetStartBand + + patchParam[pSettings->noOfPatches-1].numBandsInPatch; - int memSize = ((64) - targetStopBand) * sizeof(FIXP_DBL); + int memSize = ((64) - targetStopBand) * sizeof(FIXP_DBL); - if (!useLP) { + if (!useLP) { + for (i = startSample; i < stopSampleClear; i++) { + FDKmemclear(&qmfBufferReal[i][targetStopBand], memSize); + FDKmemclear(&qmfBufferImag[i][targetStopBand], memSize); + } + } else for (i = startSample; i < stopSampleClear; i++) { FDKmemclear(&qmfBufferReal[i][targetStopBand], memSize); - FDKmemclear(&qmfBufferImag[i][targetStopBand], memSize); } - } else - for (i = startSample; i < stopSampleClear; i++) { - FDKmemclear(&qmfBufferReal[i][targetStopBand], memSize); } +#ifdef __ANDROID__ + else { + // Safetynet logging + android_errorWriteLog(0x534e4554, "112160868"); + } +#endif /* init bwIndex for each patch */ FDKmemclear(bwIndex, MAX_NUM_PATCHES*sizeof(INT));