From 50aa5be38870319395ce2ef6f91543e6475e4b97 Mon Sep 17 00:00:00 2001 From: Fraunhofer IIS FDK Date: Mon, 6 Jul 2020 12:28:12 -0700 Subject: [PATCH 1/3] [DO NOT MERGE] Fix heap buffer overflow in sbrDecoder_AssignQmfChannels2SbrChannels(). In the bug the SBR decoder has already set up 9 channels and tries to allocate one more channel. The assignment of the QMF channels to SBR channels fails since the QMF domain manages only 8+1 channels instead of 10 channels as reqeusted by SBR. Here we have added a check in sbrDecoder_InitElement() which will return with a parse error in case additional SBR channels would exceed the maximum number of SBR channels. This solves the potential heap buffer overflow. Bug: 158762825 Test: atest DecoderTestAacDrc DecoderTestXheAac Change-Id: I741f49ab3b675fa3d3217ee72e1db66b0114f7ee --- libSBRdec/src/sbrdecoder.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libSBRdec/src/sbrdecoder.cpp b/libSBRdec/src/sbrdecoder.cpp index f9ded54..2452f8e 100644 --- a/libSBRdec/src/sbrdecoder.cpp +++ b/libSBRdec/src/sbrdecoder.cpp @@ -510,9 +510,6 @@ SBR_ERROR sbrDecoder_InitElement ( self->numSbrChannels -= self->pSbrElement[elementIndex]->nChannels; } - /* Save element ID for sanity checks and to have a fallback for concealment. */ - self->pSbrElement[elementIndex]->elementID = elementID; - /* Determine amount of channels for this element */ switch (elementID) { case ID_NONE: @@ -540,6 +537,16 @@ SBR_ERROR sbrDecoder_InitElement ( } } + /* Sanity check to avoid memory leaks */ + if (elChannels < self->pSbrElement[elementIndex]->nChannels || + (self->numSbrChannels + elChannels) > (8) + (1)) { + self->numSbrChannels += self->pSbrElement[elementIndex]->nChannels; + sbrError = SBRDEC_PARSE_ERROR; + goto bail; + } + + /* Save element ID for sanity checks and to have a fallback for concealment. */ + self->pSbrElement[elementIndex]->elementID = elementID; self->pSbrElement[elementIndex]->nChannels = elChannels; for (ch=0; ch Date: Mon, 6 Jul 2020 15:12:34 -0700 Subject: [PATCH 2/3] Fix heap buffer overflow in sbrDecoder_AssignQmfChannels2SbrChannels(). In the bug the SBR decoder has already set up 9 channels and tries to allocate one more channel. The assignment of the QMF channels to SBR channels fails since the QMF domain manages only 8+1 channels instead of 10 channels as reqeusted by SBR. Here we have added a check in sbrDecoder_InitElement() which will return with a parse error in case additional SBR channels would exceed the maximum number of SBR channels. This solves the potential heap buffer overflow. Bug: 158762825 Test: atest DecoderTestAacDrc DecoderTestAacFormat DecoderTestXheAac Merged-In: I0150ac6d5a47ffce883010f531928656eebc619e Change-Id: I8569a15214707ab622e986b34b4b917251495662 --- libSBRdec/src/sbrdecoder.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libSBRdec/src/sbrdecoder.cpp b/libSBRdec/src/sbrdecoder.cpp index e2455da..45712b5 100644 --- a/libSBRdec/src/sbrdecoder.cpp +++ b/libSBRdec/src/sbrdecoder.cpp @@ -619,10 +619,6 @@ SBR_ERROR sbrDecoder_InitElement( self->numSbrChannels -= self->pSbrElement[elementIndex]->nChannels; } - /* Save element ID for sanity checks and to have a fallback for concealment. - */ - self->pSbrElement[elementIndex]->elementID = elementID; - /* Determine amount of channels for this element */ switch (elementID) { case ID_NONE: @@ -655,12 +651,16 @@ SBR_ERROR sbrDecoder_InitElement( } /* Sanity check to avoid memory leaks */ - if (elChannels < self->pSbrElement[elementIndex]->nChannels) { + if (elChannels < self->pSbrElement[elementIndex]->nChannels || + (self->numSbrChannels + elChannels) > (8) + (1)) { self->numSbrChannels += self->pSbrElement[elementIndex]->nChannels; sbrError = SBRDEC_PARSE_ERROR; goto bail; } + /* Save element ID for sanity checks and to have a fallback for concealment. + */ + self->pSbrElement[elementIndex]->elementID = elementID; self->pSbrElement[elementIndex]->nChannels = elChannels; for (ch = 0; ch < elChannels; ch++) { From bb8f983bf36ee2ad8af6acebf4823a58060004ab Mon Sep 17 00:00:00 2001 From: Fraunhofer IIS FDK Date: Mon, 6 Jul 2020 16:37:38 +0200 Subject: [PATCH 3/3] Fix heap buffer overflow in sbrDecoder_AssignQmfChannels2SbrChannels(). In the bug the SBR decoder has already set up 9 channels and tries to allocate one more channel. The assignment of the QMF channels to SBR channels fails since the QMF domain manages only 8+1 channels instead of 10 channels as reqeusted by SBR. Here we have added a check in sbrDecoder_InitElement() which will return with a parse error in case additional SBR channels would exceed the maximum number of SBR channels. This solves the potential heap buffer overflow. Bug: 158762825 Test: atest DecoderTestAacDrc DecoderTestAacFormat DecoderTestXheAac Change-Id: I0150ac6d5a47ffce883010f531928656eebc619e Merged-In: I0150ac6d5a47ffce883010f531928656eebc619e --- libSBRdec/src/sbrdecoder.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libSBRdec/src/sbrdecoder.cpp b/libSBRdec/src/sbrdecoder.cpp index 6e4aad4..55f929f 100644 --- a/libSBRdec/src/sbrdecoder.cpp +++ b/libSBRdec/src/sbrdecoder.cpp @@ -1,7 +1,7 @@ /* ----------------------------------------------------------------------------- Software License for The Fraunhofer FDK AAC Codec Library for Android -© Copyright 1995 - 2019 Fraunhofer-Gesellschaft zur Förderung der angewandten +© Copyright 1995 - 2020 Fraunhofer-Gesellschaft zur Förderung der angewandten Forschung e.V. All rights reserved. 1. INTRODUCTION @@ -617,10 +617,6 @@ SBR_ERROR sbrDecoder_InitElement( self->numSbrChannels -= self->pSbrElement[elementIndex]->nChannels; } - /* Save element ID for sanity checks and to have a fallback for concealment. - */ - self->pSbrElement[elementIndex]->elementID = elementID; - /* Determine amount of channels for this element */ switch (elementID) { case ID_NONE: @@ -653,12 +649,16 @@ SBR_ERROR sbrDecoder_InitElement( } /* Sanity check to avoid memory leaks */ - if (elChannels < self->pSbrElement[elementIndex]->nChannels) { + if (elChannels < self->pSbrElement[elementIndex]->nChannels || + (self->numSbrChannels + elChannels) > (8) + (1)) { self->numSbrChannels += self->pSbrElement[elementIndex]->nChannels; sbrError = SBRDEC_PARSE_ERROR; goto bail; } + /* Save element ID for sanity checks and to have a fallback for concealment. + */ + self->pSbrElement[elementIndex]->elementID = elementID; self->pSbrElement[elementIndex]->nChannels = elChannels; for (ch = 0; ch < elChannels; ch++) {