From ed646d54a565504a7f66e6d52a3830e60fe90f8e Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Thu, 23 Aug 2018 12:24:35 +0530 Subject: [PATCH 01/16] Fix for out of bound write memory access in xheaac The maximum number of channels supported for usac profile in libxaac decoder is two only.Input streams of the below reported issues contain multi channel,because of multi channel the write offset is incremented by greater 2, which causes out of bound memory access while writing. Bug:112858010 Bug:112859113 Test: re-run POC Change-Id: Ide57cb8ee39d77d0f386298e899683d460a3c18b (cherry picked from commit 2491a0748426913802d1240d907cb77c9ccb0091) --- decoder/ixheaacd_cnst.h | 1 + decoder/ixheaacd_process.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/decoder/ixheaacd_cnst.h b/decoder/ixheaacd_cnst.h index b96422b..cee0a58 100644 --- a/decoder/ixheaacd_cnst.h +++ b/decoder/ixheaacd_cnst.h @@ -108,6 +108,7 @@ #define MAX_SHORT_WINDOWS 8 #define MAX_NUM_CHANNELS 6 +#define MAX_NUM_CHANNELS_USAC_LVL2 2 #define SFB_NUM_MAX ((NSFB_SHORT + 1) * MAX_SHORT_IN_LONG_BLOCK) diff --git a/decoder/ixheaacd_process.c b/decoder/ixheaacd_process.c index b87b61a..2347834 100644 --- a/decoder/ixheaacd_process.c +++ b/decoder/ixheaacd_process.c @@ -316,7 +316,7 @@ WORD32 ixheaacd_usac_process(ia_dec_data_struct *pstr_dec_data, nr_core_coder_channels = 1; core_data_extracting: - if (ch_offset >= MAX_NUM_CHANNELS) return -1; + if (ch_offset >= MAX_NUM_CHANNELS_USAC_LVL2) return -1; err = ixheaacd_core_coder_data(ele_id, pstr_usac_data, elem_idx, &ch_offset, it_bit_buff, nr_core_coder_channels); From 3195d044e9bdf7497a6cbbada90720ea86d6af7b Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Thu, 23 Aug 2018 11:25:24 +0530 Subject: [PATCH 02/16] DO NOT MERGE Fix for stack-buffer-overflow in mps module While parsing the input stream in mps_pre_matrix function, there was an error.This error was not handled properly, because of which further values which are read from bit stream are getting wrong values. We use these value in iteration in ixheaacd_mps_getstridemap() which is causing stack-buffer-overflow Bug:112857941 Test: poc+ASAN Change-Id: I9549b06e0e4e362f517869aef75d579e3e4140a6 (cherry picked from commit 28a1411d7235c5ed0a8289f0e276a31ec669ae70) --- decoder/ixheaacd_mps_dec.c | 6 +++-- decoder/ixheaacd_mps_dec.h | 6 ++--- decoder/ixheaacd_mps_pre_mix.c | 45 +++++++++++++++++++++------------- 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/decoder/ixheaacd_mps_dec.c b/decoder/ixheaacd_mps_dec.c index 890ce15..3075c1b 100644 --- a/decoder/ixheaacd_mps_dec.c +++ b/decoder/ixheaacd_mps_dec.c @@ -308,11 +308,13 @@ WORD32 ixheaacd_mps_apply(ia_mps_dec_state_struct* self, ixheaacd_mps_pre_matrix_mix_matrix_smoothing(self); - ixheaacd_mps_apply_pre_matrix(self); + err = ixheaacd_mps_apply_pre_matrix(self); + if (err < 0) return err; ixheaacd_mps_create_w(self); - ixheaacd_mps_apply_mix_matrix(self); + err = ixheaacd_mps_apply_mix_matrix(self); + if (err < 0) return err; if (self->config->bs_temp_shape_config == 2) { ixheaacd_mps_time_env_shaping(self); diff --git a/decoder/ixheaacd_mps_dec.h b/decoder/ixheaacd_mps_dec.h index 3a13d65..9aaa64f 100644 --- a/decoder/ixheaacd_mps_dec.h +++ b/decoder/ixheaacd_mps_dec.h @@ -339,8 +339,8 @@ typedef struct ia_mps_dec_state_struct { VOID ixheaacd_mps_init_pre_and_post_matrix(ia_mps_dec_state_struct *self); VOID ixheaacd_pre_and_mix_matrix_calculation(ia_mps_dec_state_struct *self); -VOID ixheaacd_mps_apply_pre_matrix(ia_mps_dec_state_struct *self); -VOID ixheaacd_mps_apply_mix_matrix(ia_mps_dec_state_struct *self); +WORD32 ixheaacd_mps_apply_pre_matrix(ia_mps_dec_state_struct *self); +WORD32 ixheaacd_mps_apply_mix_matrix(ia_mps_dec_state_struct *self); VOID ixheaacd_mps_config(ia_mps_dec_state_struct *self, WORD32 frame_len, WORD32 residual_coding, @@ -371,7 +371,7 @@ VOID ixheaacd_mps_par2umx_pred(ia_mps_dec_state_struct *self, WORD32 *h_real, WORD32 param_set_idx, WORD32 res_bands); -VOID ixheaacd_mps_upmix_interp( +WORD32 ixheaacd_mps_upmix_interp( WORD32 m_matrix[MAX_PARAMETER_SETS_MPS][MAX_PARAMETER_BANDS][MAX_M_OUTPUT] [MAX_M_INPUT], WORD32 r_matrix[MAX_TIME_SLOTS][MAX_PARAMETER_BANDS][MAX_M_OUTPUT] diff --git a/decoder/ixheaacd_mps_pre_mix.c b/decoder/ixheaacd_mps_pre_mix.c index 3c31a0b..5ea8910 100644 --- a/decoder/ixheaacd_mps_pre_mix.c +++ b/decoder/ixheaacd_mps_pre_mix.c @@ -348,15 +348,17 @@ VOID ixheaacd_mps_par2umx_pred(ia_mps_dec_state_struct *self, } } -VOID ixheaacd_mps_apply_pre_matrix(ia_mps_dec_state_struct *self) { +WORD32 ixheaacd_mps_apply_pre_matrix(ia_mps_dec_state_struct *self) { WORD32 ts, qs, row, col = 0; - - ixheaacd_mps_upmix_interp( + WORD32 err = 0; + err = ixheaacd_mps_upmix_interp( self->m1_param_re, self->r_out_re_scratch_m1, self->m1_param_re_prev, (self->dir_sig_count + self->decor_sig_count), 1, self); - ixheaacd_mps_upmix_interp( + if (err < 0) return err; + err = ixheaacd_mps_upmix_interp( self->m1_param_im, self->r_out_im_scratch_m1, self->m1_param_im_prev, (self->dir_sig_count + self->decor_sig_count), 1, self); + if (err < 0) return err; ixheaacd_fix_to_float_int( (WORD32 *)(self->r_out_re_scratch_m1), (FLOAT32 *)(self->r_out_re_in_m1), @@ -417,19 +419,22 @@ VOID ixheaacd_mps_apply_pre_matrix(ia_mps_dec_state_struct *self) { } } } + return 0; } -VOID ixheaacd_mps_apply_mix_matrix(ia_mps_dec_state_struct *self) { +WORD32 ixheaacd_mps_apply_mix_matrix(ia_mps_dec_state_struct *self) { WORD32 ts, qs, row, col; WORD32 complex_m2 = ((self->config->bs_phase_coding != 0)); WORD32 phase_interpolation = (self->config->bs_phase_coding == 1); - - ixheaacd_mps_upmix_interp( + WORD32 err = 0; + err = ixheaacd_mps_upmix_interp( self->m2_decor_re, self->r_diff_out_re_fix_in_m2, self->m2_decor_re_prev, self->out_ch_count, (self->dir_sig_count + self->decor_sig_count), self); - ixheaacd_mps_upmix_interp( + if (err < 0) return err; + err = ixheaacd_mps_upmix_interp( self->m2_resid_re, self->r_out_re_fix_in_m2, self->m2_resid_re_prev, self->out_ch_count, (self->dir_sig_count + self->decor_sig_count), self); + if (err < 0) return err; ixheaacd_fix_to_float_int( (WORD32 *)self->r_out_re_fix_in_m2, (FLOAT32 *)self->r_out_re_in_m2, MAX_TIME_SLOTS * MAX_PARAMETER_BANDS * MAX_M_OUTPUT * MAX_M_INPUT, @@ -441,14 +446,16 @@ VOID ixheaacd_mps_apply_mix_matrix(ia_mps_dec_state_struct *self) { 268435456); if (complex_m2 && !phase_interpolation) { - ixheaacd_mps_upmix_interp(self->m2_decor_im, self->r_diff_out_im_fix_in_m2, - self->m2_decor_im_prev, self->out_ch_count, - (self->dir_sig_count + self->decor_sig_count), - self); - ixheaacd_mps_upmix_interp(self->m2_resid_im, self->r_out_im_fix_in_m2, - self->m2_resid_im_prev, self->out_ch_count, - (self->dir_sig_count + self->decor_sig_count), - self); + err = ixheaacd_mps_upmix_interp( + self->m2_decor_im, self->r_diff_out_im_fix_in_m2, + self->m2_decor_im_prev, self->out_ch_count, + (self->dir_sig_count + self->decor_sig_count), self); + if (err < 0) return err; + err = ixheaacd_mps_upmix_interp( + self->m2_resid_im, self->r_out_im_fix_in_m2, self->m2_resid_im_prev, + self->out_ch_count, (self->dir_sig_count + self->decor_sig_count), + self); + if (err < 0) return err; ixheaacd_fix_to_float_int( (WORD32 *)self->r_diff_out_im_fix_in_m2, (FLOAT32 *)self->r_out_diff_im_in_m2, @@ -569,6 +576,7 @@ VOID ixheaacd_mps_apply_mix_matrix(ia_mps_dec_state_struct *self) { } } } + return 0; } static PLATFORM_INLINE WORD32 ixheaacd_mult32_shl2(WORD32 a, WORD32 b) { @@ -581,7 +589,7 @@ static PLATFORM_INLINE WORD32 ixheaacd_mult32_shl2(WORD32 a, WORD32 b) { return (result); } -VOID ixheaacd_mps_upmix_interp( +WORD32 ixheaacd_mps_upmix_interp( WORD32 m_matrix[MAX_PARAMETER_SETS_MPS][MAX_PARAMETER_BANDS][MAX_M_OUTPUT] [MAX_M_INPUT], WORD32 r_matrix[MAX_TIME_SLOTS][MAX_PARAMETER_BANDS][MAX_M_OUTPUT] @@ -595,6 +603,7 @@ VOID ixheaacd_mps_upmix_interp( for (col = 0; col < num_cols; col++) { ps = 0; ts = 0; + if (MAX_TIME_SLOTS < (self->param_slot_diff[0])) return -1; for (i = 1; i <= (WORD32)self->param_slot_diff[0]; i++) { WORD32 alpha = i * self->inv_param_slot_diff_Q30[ps]; WORD32 one_minus_alpha = 1073741824 - alpha; @@ -606,6 +615,7 @@ VOID ixheaacd_mps_upmix_interp( } for (ps = 1; ps < self->num_parameter_sets; ps++) { + if (MAX_TIME_SLOTS < (ts + self->param_slot_diff[ps])) return -1; for (i = 1; i <= (WORD32)self->param_slot_diff[ps]; i++) { WORD32 alpha = i * self->inv_param_slot_diff_Q30[ps]; WORD32 one_minus_alpha = 1073741824 - alpha; @@ -619,6 +629,7 @@ VOID ixheaacd_mps_upmix_interp( } } } + return 0; } static FLOAT32 ixheaacd_mps_angle_interpolation(FLOAT32 angle1, FLOAT32 angle2, From 171b2f9d33730d3707d4a81b7bc6b28d2000ecba Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Thu, 30 Aug 2018 14:58:47 +0530 Subject: [PATCH 03/16] Fix for heap buffer overflow in xaac decoder init Number of elements allowed in usac profile are 16. Erroneous input stream in this use case has 63336 elements.We have an error check for this max number of elements while parsing the decoder configuration.This returned error code was not handled properly. Maximum 16 config elements can be used while codec creation,because of number of elements values is coming as 63336, during creation time OOB read is happening. Bug:112766520 Bug:112857468 Bug:112913145 Bug:112918261 Test: re-ran poc Change-Id: If9413546371f72a6896f5c7e7d22a314e484cf76 (cherry picked from commit 4878ef09c7db2b2d96505c285b5ca96920b8f312) --- decoder/ixheaacd_headerdecode.c | 2 +- decoder/ixheaacd_init_config.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/decoder/ixheaacd_headerdecode.c b/decoder/ixheaacd_headerdecode.c index f68ce5f..c28833e 100644 --- a/decoder/ixheaacd_headerdecode.c +++ b/decoder/ixheaacd_headerdecode.c @@ -606,7 +606,7 @@ WORD32 ixheaacd_ga_hdr_dec(ia_aac_dec_state_struct *aac_state_struct, err = ixheaacd_config(it_bit_buff, &(pstr_audio_specific_config->str_usac_config), &(pstr_audio_specific_config->channel_configuration)); - if (err != 0) return -1; + if (err != 0) return err; if (pstr_audio_specific_config->audio_object_type == AOT_USAC) { pstr_audio_specific_config->sbr_present_flag = 1; diff --git a/decoder/ixheaacd_init_config.c b/decoder/ixheaacd_init_config.c index 0fe2fa7..d7650fc 100644 --- a/decoder/ixheaacd_init_config.c +++ b/decoder/ixheaacd_init_config.c @@ -63,6 +63,8 @@ #include "ixheaacd_struct.h" #include "ixheaacd_constants.h" +#include "ixheaacd_error_codes.h" + UWORD32 ixheaacd_sbr_ratio(UWORD32 core_sbr_framelength_idx) { UWORD32 sbr_ratio_index = 0x0FF; @@ -517,7 +519,7 @@ WORD32 ixheaacd_config(ia_bit_buf_struct *it_bit_buff, (UWORD32 *)(&(pstr_usac_conf->num_out_channels)), 5, 8, 16); if (BS_MAX_NUM_OUT_CHANNELS < pstr_usac_conf->num_out_channels) { - return -1; + return IA_ENHAACPLUS_DEC_INIT_FATAL_STREAM_CHAN_GT_MAX; } for (i = 0; i < pstr_usac_conf->num_out_channels; i++) pstr_usac_conf->output_channel_pos[i] = From 4b4d251b4ab1012747ee31286db641a48b392674 Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Thu, 23 Aug 2018 16:04:03 +0530 Subject: [PATCH 04/16] Fix for NPD in case of single coupling channel element. Reason for crash: For the below reported issues input stream has only one coupling channel element. As per the specification coupling channel element should be associated with at least one main channel element. We have different output buffers for main channel and coupling channel. In this error case the coupling channel output buffer is not getting updated because there is no main channel, so main decode is called with output buffer pointing to NULL. This is the reason for all the issues listed below. Bug:112551721 Bug:112704700 Bug:112706520 Bug:112710190 Bug:112712274 Bug:112717301 Test: re-run poc Change-Id: Ife593ca4ae21f05555b6a89092ff76b974e28a67 (cherry picked from commit b5597cea85fa41525c99ac68017d94226a2c897f) --- decoder/ixheaacd_api.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/decoder/ixheaacd_api.c b/decoder/ixheaacd_api.c index afc4510..d77627f 100644 --- a/decoder/ixheaacd_api.c +++ b/decoder/ixheaacd_api.c @@ -2424,7 +2424,11 @@ IA_ERRORCODE ixheaacd_dec_execute( (WORD8 *)p_state_enhaacplus_dec->aac_scratch_mem_v + (8 * 1024) + pers_used; } - actual_out_buffer = p_state_enhaacplus_dec->coup_ch_output; + if (p_obj_exhaacplus_dec->aac_config.element_type[1] < 3 && + p_obj_exhaacplus_dec->aac_config.element_type[1] > 0 && + p_obj_exhaacplus_dec->aac_config.ui_max_channels > 2) { + actual_out_buffer = p_state_enhaacplus_dec->coup_ch_output; + } ch_fac = 1; slot_ele = 0; } From c01adbdf8eaebcbfe300a872d248fa91a8089178 Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Fri, 31 Aug 2018 16:35:41 +0530 Subject: [PATCH 05/16] Fix for crc related issues Maximum crc registers allowed is 7. Crc registers are accessed and updated inside the function ixheaacd_adts_crc_start_reg(). Check has been added before the function call so that if the register value is less than 7 then only the function gets called. Bug:112551726 Bug:112551874 Bug:112609715 Bug:112713720 Bug:112715795 Bug:113261928 Test: poc Change-Id: I3935546b8fb3dc5c82bee16639df771349e6d2b6 (cherry picked from commit 55c1da8c375fb236f669a20c17ac9faf8e14fb07) (cherry picked from commit b0768b05646cbdb34d0c62ab9657d43ed148eb4b) --- decoder/ixheaacd_aacdecoder.c | 9 ++++++--- decoder/ixheaacd_channel.c | 3 ++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/decoder/ixheaacd_aacdecoder.c b/decoder/ixheaacd_aacdecoder.c index 8af03ee..d47c244 100644 --- a/decoder/ixheaacd_aacdecoder.c +++ b/decoder/ixheaacd_aacdecoder.c @@ -347,7 +347,8 @@ WORD32 ixheaacd_aacdec_decodeframe( prev_data_ele_present = 1; - if (ptr_adts_crc_info->crc_active == 1) { + if (ptr_adts_crc_info->crc_active == 1 && + ptr_adts_crc_info->no_reg < 7) { crc_reg = ixheaacd_adts_crc_start_reg( ptr_adts_crc_info, it_bit_buff, CRC_ADTS_RAW_DATA_BLK_LEN); } @@ -485,7 +486,8 @@ WORD32 ixheaacd_aacdec_decodeframe( { WORD32 flag = 1; - if ((ele_type != ID_FIL) && (ptr_adts_crc_info->crc_active == 1)) { + if ((ele_type != ID_FIL) && (ptr_adts_crc_info->crc_active == 1) && + (ptr_adts_crc_info->no_reg < 7)) { crc_reg = ixheaacd_adts_crc_start_reg(ptr_adts_crc_info, it_bit_buff, 0); } @@ -586,7 +588,8 @@ WORD32 ixheaacd_aacdec_decodeframe( prev_data_ele_present = 1; - if (ptr_adts_crc_info->crc_active == 1) { + if ((ptr_adts_crc_info->crc_active == 1) && + (ptr_adts_crc_info->no_reg < 7)) { crc_reg = ixheaacd_adts_crc_start_reg( ptr_adts_crc_info, it_bit_buff, CRC_ADTS_RAW_DATA_BLK_LEN); } diff --git a/decoder/ixheaacd_channel.c b/decoder/ixheaacd_channel.c index efe1cf7..83803de 100644 --- a/decoder/ixheaacd_channel.c +++ b/decoder/ixheaacd_channel.c @@ -460,7 +460,8 @@ WORD16 ixheaacd_individual_ch_stream( ia_ics_info_struct *ptr_ics_info = &ptr_aac_dec_ch_info->str_ics_info; if (ch == 1) { - if (it_bit_buff->pstr_adts_crc_info->crc_active == 1) { + if (it_bit_buff->pstr_adts_crc_info->crc_active == 1 && + (it_bit_buff->pstr_adts_crc_info->no_reg < 7)) { crc_reg = ixheaacd_adts_crc_start_reg(it_bit_buff->pstr_adts_crc_info, it_bit_buff, CRC_ADTS_RAW_IIND_ICS); From aed3058dd7a6f5705984f76e4a7a3fd4881e1880 Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Thu, 30 Aug 2018 18:53:58 +0530 Subject: [PATCH 06/16] Fix for global buffer overflow in scale factor processing Number of envelopes is equal to 2^n, where n obtained by reading 2 bits from bit stream,so maximum value for number of envelopes is 8. Time slot array table is accessed using number of envelopes. The Minimum and Maximum values are 0 and 6,based on these values the table is modified. Bug:112765917 Test: re-run poc Change-Id: I42a44fc2376536d5119a8290a14726c9c5badd19 (cherry picked from commit 61a09f1063237a23127874bf27d5a72f1d03d3fe) --- decoder/ixheaacd_env_extr.c | 8 +------- decoder/ixheaacd_sbr_const.h | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/decoder/ixheaacd_env_extr.c b/decoder/ixheaacd_env_extr.c index 67f3499..4224d9f 100644 --- a/decoder/ixheaacd_env_extr.c +++ b/decoder/ixheaacd_env_extr.c @@ -1328,8 +1328,7 @@ int ixheaacd_extract_frame_info_ld( WORD16 time_border[MAX_ENVELOPES + 1]; WORD16 time_border_noise[2 + 1]; WORD16 f[MAX_ENVELOPES + 1]; - int rel_bord_lead[3]; - int rel_bord_trail[3] = {0}; + int rel_bord_lead[7] ={0}; ia_frame_info_struct *v_frame_info = &h_frame_data->str_frame_info_details; @@ -1382,11 +1381,6 @@ int ixheaacd_extract_frame_info_ld( time_border[env] = abs_bord_lead; for (k = 0; k <= env - 1; k++) time_border[env] += rel_bord_lead[k]; } - for (env = num_rel_lead + 1; env < bs_num_env; env++) { - time_border[env] = abs_bord_trail; - for (k = 0; k <= bs_num_env - env - 1; k++) - time_border[env] -= rel_bord_trail[k]; - } break; case LD_TRAN: diff --git a/decoder/ixheaacd_sbr_const.h b/decoder/ixheaacd_sbr_const.h index 10b0959..c75fa0c 100644 --- a/decoder/ixheaacd_sbr_const.h +++ b/decoder/ixheaacd_sbr_const.h @@ -223,7 +223,7 @@ static const int ixheaacd_ld_env_table_480[15][4] = { {2, 12, -1, 1}, {2, 13, -1, 1}, {2, 14, -1, 1}, }; -static const int ixheaacd_ld_env_table_time_slot[4] = {8, 5, 0, 0}; +static const int ixheaacd_ld_env_table_time_slot[7] = {8, 5, 0, 0, 0, 0, 0}; #define SBR_CLA_BITS 2 #define SBR_ABS_BITS 2 From d004184d5aedbabde623c337e2b356c44236bed4 Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Thu, 27 Sep 2018 17:07:15 +0530 Subject: [PATCH 07/16] Fix for OOB write in gain set param's parsing Check extracted size against array sizing before proceeding. Bug: 116715937 Test: poc Change-Id: Ic26b85683342fa5f508b66f4ad71badb06540f17 (cherry picked from commit 90b76d9431668877a2831c659f25e4b96a450031) --- decoder/drc_src/impd_drc_static_payload.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/decoder/drc_src/impd_drc_static_payload.c b/decoder/drc_src/impd_drc_static_payload.c index 3f73f09..1bf677c 100644 --- a/decoder/drc_src/impd_drc_static_payload.c +++ b/decoder/drc_src/impd_drc_static_payload.c @@ -548,6 +548,10 @@ static WORD32 impd_parametic_drc_parse_coeff( str_drc_coeff_param_drc->reset_parametric_drc = (temp >> 6) & 1; str_drc_coeff_param_drc->parametric_drc_gain_set_count = temp & 0x3f; + if (str_drc_coeff_param_drc->parametric_drc_gain_set_count > + SEQUENCE_COUNT_MAX) + return (UNEXPECTED_ERROR); + for (i = 0; i < str_drc_coeff_param_drc->parametric_drc_gain_set_count; i++) { err = impd_parametric_drc_parse_gain_set_params( it_bit_buff, drc_config, @@ -2376,4 +2380,4 @@ impd_parse_loudness_info(ia_bit_buf_struct* it_bit_buff, WORD32 version, } return (0); -} \ No newline at end of file +} From 7f0108e464f1c611d33b5c7e5491174939cc88a7 Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Thu, 27 Sep 2018 16:23:14 +0530 Subject: [PATCH 08/16] Fix for OOB write in split drc characteristic parsing added bounds check on values parsed from input stream. Bug: 116619337 Test: vendor Change-Id: Ia938ce45cb0503c1ddcbeaa5d036c0f57521a38f (cherry picked from commit 599ca4428a8a357f0b47116a710f474c5ec51356) --- decoder/drc_src/impd_drc_static_payload.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/decoder/drc_src/impd_drc_static_payload.c b/decoder/drc_src/impd_drc_static_payload.c index 1bf677c..d62db45 100644 --- a/decoder/drc_src/impd_drc_static_payload.c +++ b/decoder/drc_src/impd_drc_static_payload.c @@ -1696,6 +1696,11 @@ impd_drc_parse_coeff( str_p_loc_drc_coefficients_uni_drc->characteristic_left_count = impd_read_bits_buf(it_bit_buff, 4); if (it_bit_buff->error) return it_bit_buff->error; + + if (str_p_loc_drc_coefficients_uni_drc->characteristic_left_count > + SPLIT_CHARACTERISTIC_COUNT_MAX) + return (UNEXPECTED_ERROR); + for (i = 1; i <= str_p_loc_drc_coefficients_uni_drc->characteristic_left_count; i++) { @@ -1713,6 +1718,10 @@ impd_drc_parse_coeff( str_p_loc_drc_coefficients_uni_drc->characteristic_right_count = impd_read_bits_buf(it_bit_buff, 4); if (it_bit_buff->error) return it_bit_buff->error; + + if (str_p_loc_drc_coefficients_uni_drc->characteristic_right_count > + SPLIT_CHARACTERISTIC_COUNT_MAX) + return (UNEXPECTED_ERROR); for (i = 1; i <= str_p_loc_drc_coefficients_uni_drc->characteristic_right_count; i++) { From e2f71f3e8681534817a33093b40ab18faadb3111 Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Thu, 27 Sep 2018 16:44:15 +0530 Subject: [PATCH 09/16] Fix of OOB write in drc downmix instruction count parsing Check bounds of parsed value. Bug: 116619387 Test: vendor Change-Id: Iada4937f7d99744594a1d457ae1bddefe961ba4f (cherry picked from commit df1030d8b9b3f9dd71a29fdfc6c23fffdaf3cdbc) --- decoder/drc_src/impd_drc_static_payload.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/decoder/drc_src/impd_drc_static_payload.c b/decoder/drc_src/impd_drc_static_payload.c index d62db45..cde5c1c 100644 --- a/decoder/drc_src/impd_drc_static_payload.c +++ b/decoder/drc_src/impd_drc_static_payload.c @@ -1124,6 +1124,9 @@ impd_parse_drc_config(ia_bit_buf_struct* it_bit_buff, if (it_bit_buff->error) return it_bit_buff->error; drc_config->dwnmix_instructions_count = (temp >> 1) & 0x7f; + if (drc_config->dwnmix_instructions_count > DOWNMIX_INSTRUCTION_COUNT_MAX) + return (UNEXPECTED_ERROR); + drc_config->drc_description_basic_present = temp & 1; if (drc_config->drc_description_basic_present == 1) { From 6741db7ec85b1090bba4a4308ee2c2022b2a0710 Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Thu, 27 Sep 2018 16:56:09 +0530 Subject: [PATCH 10/16] Fix for OOB write in parametric drc instruction parsing Bug: 116715245 Test: vendor Change-Id: I24c7ce7cd8c928d53a9914d116de4c6b408cfb09 (cherry picked from commit d735e2e329ee61aeed9519f743bce45b20051f81) --- decoder/drc_src/impd_drc_static_payload.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/decoder/drc_src/impd_drc_static_payload.c b/decoder/drc_src/impd_drc_static_payload.c index cde5c1c..de4ceec 100644 --- a/decoder/drc_src/impd_drc_static_payload.c +++ b/decoder/drc_src/impd_drc_static_payload.c @@ -914,6 +914,10 @@ impd_parse_drc_config_ext(ia_bit_buf_struct* it_bit_buff, str_drc_config_ext->parametric_drc_instructions_count = impd_read_bits_buf(it_bit_buff, 4); if (it_bit_buff->error) return it_bit_buff->error; + if (str_drc_config_ext->parametric_drc_instructions_count > + PARAM_DRC_INSTRUCTIONS_COUNT_MAX) + return (UNEXPECTED_ERROR); + for (i = 0; i < str_drc_config_ext->parametric_drc_instructions_count; i++) { err = impd_parse_parametric_drc_instructions( From dbf5e31aac78859e64dea61baa2c653076b0dfd2 Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Thu, 27 Sep 2018 15:46:40 +0530 Subject: [PATCH 11/16] Fix for OOB write in filter block parsing in drc Bug: 116467350 Bug: 116469592 Test: vendor Change-Id: I2f7bff1cec3d0d60e9d43217290392bf4e23d207 (cherry picked from commit 69a69acbc99226338f7c7cabce08bde9b2742bd7) --- decoder/drc_src/impd_drc_dynamic_payload.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/decoder/drc_src/impd_drc_dynamic_payload.c b/decoder/drc_src/impd_drc_dynamic_payload.c index 70fa829..e8284e9 100644 --- a/decoder/drc_src/impd_drc_dynamic_payload.c +++ b/decoder/drc_src/impd_drc_dynamic_payload.c @@ -625,7 +625,8 @@ WORD32 impd_parse_filt_block(ia_bit_buf_struct* it_bit_buff, for (j = 0; j < block_count; j++) { str_filter_block->filter_element_count = impd_read_bits_buf(it_bit_buff, 6); if (it_bit_buff->error) return it_bit_buff->error; - + if (str_filter_block->filter_element_count > FILTER_ELEMENT_COUNT_MAX) + return UNEXPECTED_ERROR; str_filter_element = &str_filter_block->str_filter_element[0]; for (k = 0; k < str_filter_block->filter_element_count; k++) { temp = impd_read_bits_buf(it_bit_buff, 7); @@ -923,6 +924,10 @@ WORD32 impd_parse_eq_coefficients(ia_bit_buf_struct* it_bit_buff, str_eq_coeff->unique_filter_block_count = impd_read_bits_buf(it_bit_buff, 6); if (it_bit_buff->error) return it_bit_buff->error; + if (str_eq_coeff->unique_filter_block_count > FILTER_BLOCK_COUNT_MAX) { + return (UNEXPECTED_ERROR); + } + err = impd_parse_filt_block(it_bit_buff, &(str_eq_coeff->str_filter_block[0]), str_eq_coeff->unique_filter_block_count); if (err) return (err); From ed3339bc46cf2f86fe7c1341d46212c8dfe57037 Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Fri, 21 Sep 2018 17:58:03 +0530 Subject: [PATCH 12/16] Fix for OOB loudness eq instruction parsing Bounds checking on value parsed from input stream. Bug: 116020594 Test: vendor Change-Id: I915f36ca27b982c8f1b11a533969e40fbff3b765 (cherry picked from commit cd74db5553ba8c967309aa63830fdfed73236a83) --- decoder/drc_src/impd_drc_dynamic_payload.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/decoder/drc_src/impd_drc_dynamic_payload.c b/decoder/drc_src/impd_drc_dynamic_payload.c index e8284e9..abaface 100644 --- a/decoder/drc_src/impd_drc_dynamic_payload.c +++ b/decoder/drc_src/impd_drc_dynamic_payload.c @@ -1281,7 +1281,8 @@ WORD32 impd_parse_loud_eq_instructions( if (additional_eq_set_id_present) { additional_eq_set_id_cnt = impd_read_bits_buf(it_bit_buff, 6); if (it_bit_buff->error) return it_bit_buff->error; - + if (additional_eq_set_id_cnt >= EQ_SET_ID_COUNT_MAX) + return UNEXPECTED_ERROR; for (i = 0; i < additional_eq_set_id_cnt; i++) { loud_eq_instructions->eq_set_id[i + 1] = impd_read_bits_buf(it_bit_buff, 6); From 316b8bbec38a7cf717186b4681e2145b6de79876 Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Thu, 20 Sep 2018 19:12:27 +0530 Subject: [PATCH 13/16] Fix for heap buffer over flow in drc bit stream parsing Bound values that we parse from the input stream. Bug: 115375616 Test: vendor Change-Id: I357d8e19e377fbe5156e5a639ed9ab99cbfeed52 (cherry picked from commit c90eeb6e6181e80e753692690176cf5ee2dbb38e) --- decoder/drc_src/impd_drc_dynamic_payload.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/decoder/drc_src/impd_drc_dynamic_payload.c b/decoder/drc_src/impd_drc_dynamic_payload.c index abaface..65d3576 100644 --- a/decoder/drc_src/impd_drc_dynamic_payload.c +++ b/decoder/drc_src/impd_drc_dynamic_payload.c @@ -587,6 +587,10 @@ WORD32 impd_parse_drc_ext_v1(ia_bit_buf_struct* it_bit_buff, if (str_drc_config_ext->loud_eq_instructions_flag == 1) { str_drc_config_ext->loud_eq_instructions_count = impd_read_bits_buf(it_bit_buff, 4); + if (str_drc_config_ext->loud_eq_instructions_count > + LOUD_EQ_INSTRUCTIONS_COUNT_MAX) + return UNEXPECTED_ERROR; + if (it_bit_buff->error) return it_bit_buff->error; for (i = 0; i < str_drc_config_ext->loud_eq_instructions_count; i++) { err = impd_parse_loud_eq_instructions( @@ -605,6 +609,8 @@ WORD32 impd_parse_drc_ext_v1(ia_bit_buf_struct* it_bit_buff, if (err) return (err); str_drc_config_ext->eq_instructions_count = impd_read_bits_buf(it_bit_buff, 4); + if (str_drc_config_ext->eq_instructions_count > EQ_INSTRUCTIONS_COUNT_MAX) + return UNEXPECTED_ERROR; if (it_bit_buff->error) return it_bit_buff->error; for (i = 0; i < str_drc_config_ext->eq_instructions_count; i++) { err = impd_parse_eq_instructions( From 9349825e6f9e3cef22698ea802cbc0d838c48a68 Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Tue, 18 Sep 2018 12:04:31 +0530 Subject: [PATCH 14/16] Fix for crash due to un-initailized variables in drc module Nested loop used wrong subscript in inner loop, leading to bad iteration count and haphazard clearing of data structure. Bug: 113885537 Test: vendor Change-Id: Ia9cb53205f4e91ee99268202114fc2001eae2de3 (cherry picked from commit 988f5bd17c64c2efcb6e0a36c633065488ca2b79) --- decoder/drc_src/impd_drc_gain_decoder.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/decoder/drc_src/impd_drc_gain_decoder.c b/decoder/drc_src/impd_drc_gain_decoder.c index 43ae7b2..4b48f54 100644 --- a/decoder/drc_src/impd_drc_gain_decoder.c +++ b/decoder/drc_src/impd_drc_gain_decoder.c @@ -148,25 +148,25 @@ IA_ERRORCODE impd_init_drc_decode_post_config( p_drc_gain_dec_structs->drc_gain_buffers.pstr_gain_buf[k] .buf_interpolation_count = drc_instruction_str->gain_element_count; for (i = 0; - i < p_drc_gain_dec_structs->drc_gain_buffers.pstr_gain_buf[i] + i < p_drc_gain_dec_structs->drc_gain_buffers.pstr_gain_buf[k] .buf_interpolation_count; i++) { - p_drc_gain_dec_structs->drc_gain_buffers.pstr_gain_buf[i] + p_drc_gain_dec_structs->drc_gain_buffers.pstr_gain_buf[k] .buf_interpolation[i] .str_node.time = 0; - p_drc_gain_dec_structs->drc_gain_buffers.pstr_gain_buf[i] + p_drc_gain_dec_structs->drc_gain_buffers.pstr_gain_buf[k] .buf_interpolation[i] .prev_node.time = -1; - p_drc_gain_dec_structs->drc_gain_buffers.pstr_gain_buf[i] + p_drc_gain_dec_structs->drc_gain_buffers.pstr_gain_buf[k] .buf_interpolation[i] .str_node.loc_db_gain = 0.0f; - p_drc_gain_dec_structs->drc_gain_buffers.pstr_gain_buf[i] + p_drc_gain_dec_structs->drc_gain_buffers.pstr_gain_buf[k] .buf_interpolation[i] .str_node.slope = 0.0f; for (j = 0; j < 2 * AUDIO_CODEC_FRAME_SIZE_MAX + MAX_SIGNAL_DELAY; j++) { - p_drc_gain_dec_structs->drc_gain_buffers.pstr_gain_buf[i] + p_drc_gain_dec_structs->drc_gain_buffers.pstr_gain_buf[k] .buf_interpolation[i] .lpcm_gains[j] = 1.f; } From 06100c3f053b27a4f16a84aa1d43e6d407e3bec6 Mon Sep 17 00:00:00 2001 From: Ramesh Katuri Date: Fri, 21 Sep 2018 11:48:54 +0530 Subject: [PATCH 15/16] Fix for OOB write in parsing drc ext Validate drc coefficient counts as we parse them from the stream. Bug: 116224432 Test: vendor Change-Id: I5a78521b8acfcdc7af96b91e5687d4f02ce49e54 (cherry picked from commit 17825d4a751010965140e3f2c9d478a223d5441f) --- decoder/drc_src/impd_drc_dynamic_payload.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/decoder/drc_src/impd_drc_dynamic_payload.c b/decoder/drc_src/impd_drc_dynamic_payload.c index 65d3576..68583b2 100644 --- a/decoder/drc_src/impd_drc_dynamic_payload.c +++ b/decoder/drc_src/impd_drc_dynamic_payload.c @@ -558,6 +558,10 @@ WORD32 impd_parse_drc_ext_v1(ia_bit_buf_struct* it_bit_buff, if (it_bit_buff->error) return it_bit_buff->error; if (drc_coeffs_and_instructions_uni_drc_v1_flag == 1) { drc_coefficients_uni_drc_v1_count = impd_read_bits_buf(it_bit_buff, 3); + if ((drc_coefficients_uni_drc_v1_count + + drc_config->drc_coefficients_drc_count) > DRC_COEFF_COUNT_MAX) { + return (UNEXPECTED_ERROR); + } if (it_bit_buff->error) return it_bit_buff->error; for (i = 0; i < drc_coefficients_uni_drc_v1_count; i++) { err = impd_drc_parse_coeff( From 4856048d42f365c75b5e97bae4aa97ebbca9b13f Mon Sep 17 00:00:00 2001 From: Ray Essick Date: Tue, 16 Oct 2018 11:23:07 -0700 Subject: [PATCH 16/16] Mark xaac codec experimental Bug: 117786798 Test: presence in source tree Change-Id: I89475f9b8b09b22d6924ae8e97275206c2d85be8 (cherry picked from commit e5f65556cc592d9faa6a225223cf7697b2880b7b) --- README.experimental | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 README.experimental diff --git a/README.experimental b/README.experimental new file mode 100644 index 0000000..27fd798 --- /dev/null +++ b/README.experimental @@ -0,0 +1,5 @@ +This xaac codec (external/xaac) is experimental; it is not yet intended +to be used on production devices. + +This codec should not be configured into any production Android Pie +(Android 9) device that will be shipped.