From ed03681d52f2759b64dc26f749a2aaa496fcbf99 Mon Sep 17 00:00:00 2001 From: Sushanth Patil Date: Wed, 6 Mar 2019 12:48:40 +0530 Subject: [PATCH] Fix for heap buffer overflow in read_bits_buf In ixheaacd_arith_decode(), ixheaacd_read_bidireciton() was being called even when bitbuffer was exhausted which in turn made cnt_bits and bit_pos more and more negative which overflowed eventually and resulted in heap buffer overlow. We also observed the variable "cumulative" value to be zero even after reaching the end of bitbuffer and this in turn lead to an infinte for loop "for (lev = esc_nb = 0;;)" in ixheaacd_arth_decoding_level2() as "cumulative" value would not be updated after reaching end of bitbuffer. As a fix , we return whenever "cumulative" value remains zero even after bitbuffer is exhausted. And this patch adds a check for (cnt_bits < 0) in ixheaacd_bitbuffer.c and removes bitbuffer wrap around in ixheaacd_read_bidirection(). Added cnt_bits > 25 check in ixheaacd_read_bits_buf() & ixheaacd_show_bits_buf() as these functions cannot handle read of more than 25 bits at a time. Bug:123976878 Test: poc Change-Id: I4ec729fddb859ddd0add0045532e20fbfffd2e5c --- decoder/ixheaacd_arith_dec.c | 36 +++++++++++------------------ decoder/ixheaacd_bitbuffer.c | 10 ++++---- decoder/ixheaacd_common_initfuncs.c | 25 ++++++++------------ 3 files changed, 28 insertions(+), 43 deletions(-) diff --git a/decoder/ixheaacd_arith_dec.c b/decoder/ixheaacd_arith_dec.c index 14bb414..f2951ed 100644 --- a/decoder/ixheaacd_arith_dec.c +++ b/decoder/ixheaacd_arith_dec.c @@ -1730,34 +1730,26 @@ static WORD32 ixheaacd_arith_decode(ia_bit_buf_struct *it_bit_buff, UWORD16 const *cum_freq, WORD32 cfl) { register WORD32 symbol; register WORD32 low, high, range, value; - register WORD32 cum; + register WORD32 cumulative; register UWORD16 const *p; register UWORD16 const *q; - WORD32 short_value, i = 16; - int shift_value; - if (it_bit_buff->cnt_bits < 16) { - shift_value = 16 - it_bit_buff->cnt_bits; - short_value = ixheaacd_read_bits_buf(it_bit_buff, it_bit_buff->cnt_bits); - short_value <<= shift_value; - } else { - shift_value = 0; - short_value = ixheaacd_read_bits_buf(it_bit_buff, 16); - } - low = s->low; high = s->high; value = s->value; range = high - low + 1; - cum = ((((WORD32)(value - low + 1)) << 14) - ((WORD32)1)) / ((WORD32)range); + cumulative = ((((WORD32)(value - low + 1)) << 14) - ((WORD32)1)) / ((WORD32)range); + + if (it_bit_buff->cnt_bits == 0) + if (cumulative <= 0) return -1; p = cum_freq - 1; do { q = p + (cfl >> 1); - if (*q > cum) { + if (*q > cumulative) { p = q; cfl++; } @@ -1786,12 +1778,14 @@ static WORD32 ixheaacd_arith_decode(ia_bit_buf_struct *it_bit_buff, low += low; high += high + 1; - i--; - value = (value << 1) | ((short_value >> i) & 1); + if (it_bit_buff->cnt_bits > 0) + value = (value << 1) | ixheaacd_read_bits_buf(it_bit_buff, 1); + else + value = (value << 1); + bit_count++; } - ixheaacd_read_bidirection(it_bit_buff, -(i - shift_value)); s->low = low; s->high = high; s->value = value; @@ -1847,10 +1841,6 @@ WORD32 ixheaacd_arth_decoding_level2(ia_bit_buf_struct *it_bit_buff, if (esc_nb > 7) { esc_nb = 7; } - if (esc_nb < 0) { - esc_nb = 0; - return -1; - } } if (m == 0) { @@ -1891,8 +1881,8 @@ WORD32 ixheaacd_arth_decoding_level2(ia_bit_buf_struct *it_bit_buff, if (bit_count > 0) { bit_count_5 = bit_count >> 5; - for (i = 0; i < bit_count_5; i++) ixheaacd_read_bits_buf(it_bit_buff, 32); - ixheaacd_read_bits_buf(it_bit_buff, bit_count & 31); + bit_count_5 = (bit_count_5 * 32) + (bit_count & 31); + ixheaacd_skip_bits_buf(it_bit_buff, bit_count_5); } for (i = 0; i < pres_n; i++) { diff --git a/decoder/ixheaacd_bitbuffer.c b/decoder/ixheaacd_bitbuffer.c index 0882689..4909e84 100644 --- a/decoder/ixheaacd_bitbuffer.c +++ b/decoder/ixheaacd_bitbuffer.c @@ -50,7 +50,7 @@ WORD32 ixheaacd_skip_bits_buf(ia_bit_buf_struct *it_bit_buff, WORD no_of_bits) { UWORD8 *ptr_read_next = it_bit_buff->ptr_read_next; WORD bit_pos = it_bit_buff->bit_pos; - if (it_bit_buff->cnt_bits < no_of_bits) + if (it_bit_buff->cnt_bits < no_of_bits || it_bit_buff->cnt_bits < 0) longjmp(*(it_bit_buff->xaac_jmp_buf), IA_ENHAACPLUS_DEC_EXE_NONFATAL_INSUFFICIENT_INPUT_BYTES); it_bit_buff->cnt_bits -= no_of_bits; @@ -73,7 +73,8 @@ WORD32 ixheaacd_show_bits_buf(ia_bit_buf_struct *it_bit_buff, WORD no_of_bits) { UWORD8 *ptr_read_next = it_bit_buff->ptr_read_next; WORD bit_pos = it_bit_buff->bit_pos; - if (it_bit_buff->cnt_bits < no_of_bits) { + if (it_bit_buff->cnt_bits < no_of_bits || it_bit_buff->cnt_bits < 0 || + no_of_bits > 25) { longjmp(*(it_bit_buff->xaac_jmp_buf), IA_ENHAACPLUS_DEC_EXE_NONFATAL_INSUFFICIENT_INPUT_BYTES); } @@ -104,7 +105,8 @@ WORD32 ixheaacd_read_bits_buf(ia_bit_buf_struct *it_bit_buff, WORD no_of_bits) { return 0; } - if (it_bit_buff->cnt_bits < no_of_bits) { + if (it_bit_buff->cnt_bits < no_of_bits || it_bit_buff->cnt_bits < 0 || + no_of_bits > 25) { longjmp(*(it_bit_buff->xaac_jmp_buf), IA_ENHAACPLUS_DEC_EXE_NONFATAL_INSUFFICIENT_INPUT_BYTES); } @@ -232,7 +234,7 @@ WORD32 ixheaacd_aac_read_bit_rev(ia_bit_buf_struct *it_bit_buff) { UWORD32 temp; WORD no_of_bits = 1; - if (it_bit_buff->cnt_bits < no_of_bits) { + if (it_bit_buff->cnt_bits < no_of_bits || it_bit_buff->cnt_bits < 0) { longjmp(*(it_bit_buff->xaac_jmp_buf), IA_ENHAACPLUS_DEC_EXE_NONFATAL_INSUFFICIENT_INPUT_BYTES); } diff --git a/decoder/ixheaacd_common_initfuncs.c b/decoder/ixheaacd_common_initfuncs.c index bc5f5ff..ac0e9c7 100644 --- a/decoder/ixheaacd_common_initfuncs.c +++ b/decoder/ixheaacd_common_initfuncs.c @@ -158,30 +158,23 @@ ia_bit_buf_struct *ixheaacd_create_init_bit_buf(ia_bit_buf_struct *it_bit_buff, VOID ixheaacd_read_bidirection(ia_bit_buf_struct *it_bit_buff, WORD32 ixheaacd_drc_offset) { if (ixheaacd_drc_offset != 0) { - WORD32 bit_offset; + WORD32 byte_offset; it_bit_buff->cnt_bits = it_bit_buff->cnt_bits - ixheaacd_drc_offset; + if (it_bit_buff->cnt_bits < 0) { + longjmp(*(it_bit_buff->xaac_jmp_buf), + IA_ENHAACPLUS_DEC_EXE_NONFATAL_INSUFFICIENT_INPUT_BYTES); + } it_bit_buff->bit_pos = it_bit_buff->bit_pos - ixheaacd_drc_offset; - bit_offset = it_bit_buff->bit_pos >> 3; - it_bit_buff->bit_pos = it_bit_buff->bit_pos - (bit_offset << 3); + byte_offset = it_bit_buff->bit_pos >> 3; + it_bit_buff->bit_pos = it_bit_buff->bit_pos - (byte_offset << 3); - if (bit_offset) { + if (byte_offset) { UWORD8 *ptr_read_next; - WORD32 temp; ptr_read_next = it_bit_buff->ptr_read_next; - ptr_read_next = ptr_read_next - (bit_offset); - - temp = it_bit_buff->ptr_bit_buf_end - it_bit_buff->ptr_bit_buf_base + 1; - - if (ptr_read_next > it_bit_buff->ptr_bit_buf_end) { - ptr_read_next -= temp; - } - - if (ptr_read_next < it_bit_buff->ptr_bit_buf_base) { - ptr_read_next += temp; - } + ptr_read_next = ptr_read_next - (byte_offset); it_bit_buff->ptr_read_next = ptr_read_next; }