From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Rocky Liao Date: Mon, 19 Sep 2022 17:39:42 +0800 Subject: [PATCH] AVRCP: Fix potential buffer overflow There will be buffer overflow if remote response exceeds AVRC_MAX_APP_ATTR_SIZE, add array index check to avoid buffer overflow issue. CRs-fixed: 3278869 Change-Id: Ia93690e0dc4b28fd01af3a406678d43d426d3be8 --- btif/src/btif_rc.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/btif/src/btif_rc.c b/btif/src/btif_rc.c index 632ec9c33..7f17a5191 100644 --- a/btif/src/btif_rc.c +++ b/btif/src/btif_rc.c @@ -88,6 +88,7 @@ #define MAX_CMD_QUEUE_LEN 16 #define ERR_PLAYER_NOT_ADDRESED 0x13 #define BTRC_FEAT_AVRC_UI_UPDATE 0x08 +#define BTRC_MAX_APP_ATTR_SIZE 16 #if (defined(AVCT_COVER_ART_INCLUDED) && (AVCT_COVER_ART_INCLUDED == TRUE)) #define MAX_ELEM_ATTR_SIZE 8 @@ -4768,7 +4769,7 @@ static void handle_app_attr_txt_response (tBTA_AV_META_MSG *pmeta_msg, tAVRC_GET * for standard attributes. */ p_app_settings->num_ext_attrs = 0; - for (xx = 0; xx < p_app_settings->ext_attr_index; xx++) + for (xx = 0; xx < p_app_settings->ext_attr_index && xx < AVRC_MAX_APP_ATTR_SIZE; xx++) osi_free_and_reset((void **)&p_app_settings->ext_attrs[xx].p_str); p_app_settings->ext_attr_index = 0; /* Klockwork Fix for below issue at line 4765 @@ -4787,7 +4788,7 @@ static void handle_app_attr_txt_response (tBTA_AV_META_MSG *pmeta_msg, tAVRC_GET for (xx = 0; xx < p_rsp->num_attr; xx++) { UINT8 x; - for (x = 0; x < p_app_settings->num_ext_attrs; x++) + for (x = 0; x < p_app_settings->num_ext_attrs && x < AVRC_MAX_APP_ATTR_SIZE; x++) { if (p_app_settings->ext_attrs[x].attr_id == p_rsp->p_attrs[xx].attr_id) { @@ -4843,12 +4844,12 @@ static void handle_app_attr_val_txt_response (tBTA_AV_META_MSG *pmeta_msg, tAVRC * for standard attributes. */ p_app_settings->num_ext_attrs = 0; - for (xx = 0; xx < p_app_settings->ext_attr_index; xx++) + for (xx = 0; xx < p_app_settings->ext_attr_index && xx < AVRC_MAX_APP_ATTR_SIZE; xx++) { int x; btrc_player_app_ext_attr_t *p_ext_attr = &p_app_settings->ext_attrs[xx]; - for (x = 0; x < p_ext_attr->num_val; x++) + for (x = 0; x < p_ext_attr->num_val && x < BTRC_MAX_APP_ATTR_SIZE; x++) osi_free_and_reset((void **)&p_ext_attr->ext_attr_val[x].p_str); p_ext_attr->num_val = 0; osi_free_and_reset((void **)&p_app_settings->ext_attrs[xx].p_str); @@ -4868,12 +4869,19 @@ static void handle_app_attr_val_txt_response (tBTA_AV_META_MSG *pmeta_msg, tAVRC return; } + if (p_app_settings->ext_val_index >= AVRC_MAX_APP_ATTR_SIZE) + { + BTIF_TRACE_ERROR("%s: ext_val_index is 0x%02x, overflow!", + __func__, p_app_settings->ext_val_index); + return; + } + for (xx = 0; xx < p_rsp->num_attr; xx++) { UINT8 x; btrc_player_app_ext_attr_t *p_ext_attr; p_ext_attr = &p_app_settings->ext_attrs[p_app_settings->ext_val_index]; - for (x = 0; x < p_rsp->num_attr; x++) + for (x = 0; x < p_rsp->num_attr && x < BTRC_MAX_APP_ATTR_SIZE; x++) { if (p_ext_attr->ext_attr_val[x].val == p_rsp->p_attrs[xx].attr_id) { @@ -4924,12 +4932,12 @@ static void handle_app_attr_val_txt_response (tBTA_AV_META_MSG *pmeta_msg, tAVRC /* Free the application settings information after sending to * application. */ - for (xx = 0; xx < p_app_settings->ext_attr_index; xx++) + for (xx = 0; xx < p_app_settings->ext_attr_index && xx < AVRC_MAX_APP_ATTR_SIZE; xx++) { int x; btrc_player_app_ext_attr_t *p_ext_attr = &p_app_settings->ext_attrs[xx]; - for (x = 0; x < p_ext_attr->num_val; x++) + for (x = 0; x < p_ext_attr->num_val && x < BTRC_MAX_APP_ATTR_SIZE; x++) osi_free_and_reset((void **)&p_ext_attr->ext_attr_val[x].p_str); p_ext_attr->num_val = 0; osi_free_and_reset((void **)&p_app_settings->ext_attrs[xx].p_str);