From 546961a9d3333a3886cc665d04c2618dd07c588b Mon Sep 17 00:00:00 2001 From: "turaj@webrtc.org" Date: Fri, 23 May 2014 17:14:29 +0000 Subject: [PATCH] Avoid reading uninitialized values (outside baundary) in DFT arithmatic decoder of iSAC-fix. Arithmetic encoder does not right the last 2 or 3 bytes of |streamval| when terminating the bit-stream. Perhaps the last bytes makes no difference in decoding the stream. However, the decoder reads full |streamval| (int16_t) going out of boundary and reading uninitialized values. This avoids this problem. by inserting zero-bytes whenever decoder intends to read outside boundary. BUG=1353,chrome373312,b/13468260 R=tina.legrand@webrtc.org Review URL: https://webrtc-codereview.appspot.com/16499005 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6234 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../isac/fix/source/arith_routines_logist.c | 31 +++++++++++++------ .../codecs/isac/fix/source/isacfix.c | 1 + .../codecs/isac/fix/source/structs.h | 1 + 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/webrtc/modules/audio_coding/codecs/isac/fix/source/arith_routines_logist.c b/webrtc/modules/audio_coding/codecs/isac/fix/source/arith_routines_logist.c index b540ed5ee..62a20a2ef 100644 --- a/webrtc/modules/audio_coding/codecs/isac/fix/source/arith_routines_logist.c +++ b/webrtc/modules/audio_coding/codecs/isac/fix/source/arith_routines_logist.c @@ -248,7 +248,7 @@ int16_t WebRtcIsacfix_DecLogisticMulti2(int16_t *dataQ7, int16_t envCount; uint16_t tmpARSpecQ8 = 0; int k, i; - + int offset = 0; /* point to beginning of stream buffer */ streamPtr = streamData->stream + streamData->stream_index; @@ -377,14 +377,27 @@ int16_t WebRtcIsacfix_DecLogisticMulti2(int16_t *dataQ7, * W_upper < 2^24 */ while ( !(W_upper & 0xFF000000) ) { - /* read next byte from stream */ - if (streamData->full == 0) { - streamVal = WEBRTC_SPL_LSHIFT_W32(streamVal, 8) | (*streamPtr++ & 0x00FF); - streamData->full = 1; + if (streamPtr < streamData->stream + streamData->stream_size) { + /* read next byte from stream */ + if (streamData->full == 0) { + streamVal = WEBRTC_SPL_LSHIFT_W32(streamVal, 8) | (*streamPtr++ & 0x00FF); + streamData->full = 1; + } else { + streamVal = WEBRTC_SPL_LSHIFT_W32(streamVal, 8) | + WEBRTC_SPL_RSHIFT_U16(*streamPtr, 8); + streamData->full = 0; + } } else { - streamVal = WEBRTC_SPL_LSHIFT_W32(streamVal, 8) | - WEBRTC_SPL_RSHIFT_U16(*streamPtr, 8); - streamData->full = 0; + /* Intending to read outside the stream. This can happen for the last + * two or three bytes. It is how the algorithm is implemented. Do + * not read from the bit stream and insert zeros instead. */ + streamVal = WEBRTC_SPL_LSHIFT_W32(streamVal, 8); + if (streamData->full == 0) { + offset++; // We would have incremented the pointer in this case. + streamData->full = 1; + } else { + streamData->full = 0; + } } W_upper = WEBRTC_SPL_LSHIFT_W32(W_upper, 8); } @@ -392,7 +405,7 @@ int16_t WebRtcIsacfix_DecLogisticMulti2(int16_t *dataQ7, envCount++; } - streamData->stream_index = streamPtr - streamData->stream; + streamData->stream_index = streamPtr + offset - streamData->stream; streamData->W_upper = W_upper; streamData->streamval = streamVal; diff --git a/webrtc/modules/audio_coding/codecs/isac/fix/source/isacfix.c b/webrtc/modules/audio_coding/codecs/isac/fix/source/isacfix.c index 8baa30738..30e6f67e7 100644 --- a/webrtc/modules/audio_coding/codecs/isac/fix/source/isacfix.c +++ b/webrtc/modules/audio_coding/codecs/isac/fix/source/isacfix.c @@ -791,6 +791,7 @@ int16_t WebRtcIsacfix_Decode(ISACFIX_MainStruct *ISAC_main_inst, } (ISAC_inst->ISACdec_obj.bitstr_obj).stream = (uint16_t *)encoded; + ISAC_inst->ISACdec_obj.bitstr_obj.stream_size = (len + 1) >> 1; /* convert bitstream from int16_t to bytes */ #ifndef WEBRTC_ARCH_BIG_ENDIAN diff --git a/webrtc/modules/audio_coding/codecs/isac/fix/source/structs.h b/webrtc/modules/audio_coding/codecs/isac/fix/source/structs.h index 4d0435663..b4d2bd899 100644 --- a/webrtc/modules/audio_coding/codecs/isac/fix/source/structs.h +++ b/webrtc/modules/audio_coding/codecs/isac/fix/source/structs.h @@ -33,6 +33,7 @@ typedef struct Bitstreamstruct_dec { int16_t full; /* 0 - first byte in memory filled, second empty*/ /* 1 - both bytes are empty (we just filled the previous memory */ + int stream_size; /* The size of stream. */ } Bitstr_dec; /* Bitstream struct for encoder */