aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKeith Mok <keithmok@google.com>2022-08-14 23:08:20 +0000
committerAndroid Build Coastguard Worker <android-build-coastguard-worker@google.com>2022-10-19 16:40:54 +0000
commite6e191ebcc03d679d15df439970281a019f3eec9 (patch)
treec3ba3c4d31c4177f747c032b3fd8b803fd996a90
parent5c97adf0bdb8b0b9d431c37a3ae38995f04305be (diff)
downloadbt-e6e191ebcc03d679d15df439970281a019f3eec9.tar.gz
Fix integer overflow when parsing avrc response
Convert min_len from 16 bits to 32 bits to avoid length checking overflow. Also, use calloc instead of malloc for list allocation since caller need to clean up string memory in the list items Bug: 242459126 Test: fuzz_avrc Tag: #security Ignore-AOSP-First: Security Merged-In: I7250509f2b320774926a8b24fd28828c5217d8a4 Change-Id: I7250509f2b320774926a8b24fd28828c5217d8a4 (cherry picked from commit 1b455b40fa104acb2527b351b80c3b5e66f79bfb) Merged-In: I7250509f2b320774926a8b24fd28828c5217d8a4
-rw-r--r--stack/avdt/avdt_scb_act.cc2
-rw-r--r--stack/avrc/avrc_pars_ct.cc39
-rw-r--r--stack/avrc/avrc_pars_tg.cc2
3 files changed, 13 insertions, 30 deletions
diff --git a/stack/avdt/avdt_scb_act.cc b/stack/avdt/avdt_scb_act.cc
index 81293444b..2ffca8654 100644
--- a/stack/avdt/avdt_scb_act.cc
+++ b/stack/avdt/avdt_scb_act.cc
@@ -310,7 +310,7 @@ uint8_t* avdt_scb_hdl_report(AvdtpScb* p_scb, uint8_t* p, uint16_t len) {
uint8_t* p_start = p;
uint32_t ssrc;
uint8_t o_v, o_p, o_cc;
- uint16_t min_len = 0;
+ uint32_t min_len = 0;
AVDT_REPORT_TYPE pt;
tAVDT_REPORT_DATA report;
diff --git a/stack/avrc/avrc_pars_ct.cc b/stack/avrc/avrc_pars_ct.cc
index db3ee7594..2d6f8b264 100644
--- a/stack/avrc/avrc_pars_ct.cc
+++ b/stack/avrc/avrc_pars_ct.cc
@@ -141,7 +141,7 @@ static tAVRC_STS avrc_pars_vendor_rsp(tAVRC_MSG_VENDOR* p_msg,
tAVRC_STS avrc_parse_notification_rsp(uint8_t* p_stream, uint16_t len,
tAVRC_REG_NOTIF_RSP* p_rsp) {
- uint16_t min_len = 1;
+ uint32_t min_len = 1;
if (len < min_len) goto length_error;
BE_STREAM_TO_UINT8(p_rsp->event_id, p_stream);
@@ -237,7 +237,7 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
}
BE_STREAM_TO_UINT8(pdu, p);
uint16_t pkt_len;
- uint16_t min_len = 0;
+ uint32_t min_len = 0;
/* read the entire packet len */
BE_STREAM_TO_UINT16(pkt_len, p);
@@ -279,7 +279,7 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
get_item_rsp->uid_counter, get_item_rsp->item_count);
/* get each of the items */
- get_item_rsp->p_item_list = (tAVRC_ITEM*)osi_malloc(
+ get_item_rsp->p_item_list = (tAVRC_ITEM*)osi_calloc(
get_item_rsp->item_count * (sizeof(tAVRC_ITEM)));
tAVRC_ITEM* curr_item = get_item_rsp->p_item_list;
for (int i = 0; i < get_item_rsp->item_count; i++) {
@@ -369,7 +369,7 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
__func__, media->type, media->name.charset_id,
media->name.str_len, media->attr_count);
- media->p_attr_list = (tAVRC_ATTR_ENTRY*)osi_malloc(
+ media->p_attr_list = (tAVRC_ATTR_ENTRY*)osi_calloc(
media->attr_count * sizeof(tAVRC_ATTR_ENTRY));
for (int jk = 0; jk < media->attr_count; jk++) {
tAVRC_ATTR_ENTRY* attr_entry = &(media->p_attr_list[jk]);
@@ -380,14 +380,8 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
/* Parse the name now */
BE_STREAM_TO_UINT16(attr_entry->name.charset_id, p);
BE_STREAM_TO_UINT16(attr_entry->name.str_len, p);
- if (static_cast<uint16_t>(min_len + attr_entry->name.str_len) <
- min_len) {
- // Check for overflow
- android_errorWriteLog(0x534e4554, "205570663");
- }
- if (pkt_len - min_len < attr_entry->name.str_len)
- goto browse_length_error;
min_len += attr_entry->name.str_len;
+ if (pkt_len < min_len) goto browse_length_error;
attr_entry->name.p_str = (uint8_t*)osi_malloc(
attr_entry->name.str_len * sizeof(uint8_t));
BE_STREAM_TO_ARRAY(p, attr_entry->name.p_str,
@@ -441,7 +435,7 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
}
BE_STREAM_TO_UINT8(get_attr_rsp->status, p)
BE_STREAM_TO_UINT8(get_attr_rsp->num_attrs, p);
- get_attr_rsp->p_attrs = (tAVRC_ATTR_ENTRY*)osi_malloc(
+ get_attr_rsp->p_attrs = (tAVRC_ATTR_ENTRY*)osi_calloc(
get_attr_rsp->num_attrs * sizeof(tAVRC_ATTR_ENTRY));
for (int i = 0; i < get_attr_rsp->num_attrs; i++) {
tAVRC_ATTR_ENTRY* attr_entry = &(get_attr_rsp->p_attrs[i]);
@@ -450,14 +444,8 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
BE_STREAM_TO_UINT32(attr_entry->attr_id, p);
BE_STREAM_TO_UINT16(attr_entry->name.charset_id, p);
BE_STREAM_TO_UINT16(attr_entry->name.str_len, p);
- if (static_cast<uint16_t>(min_len + attr_entry->name.str_len) <
- min_len) {
- // Check for overflow
- android_errorWriteLog(0x534e4554, "205570663");
- }
- if (pkt_len - min_len < attr_entry->name.str_len)
- goto browse_length_error;
min_len += attr_entry->name.str_len;
+ if (pkt_len < min_len) goto browse_length_error;
attr_entry->name.p_str =
(uint8_t*)osi_malloc(attr_entry->name.str_len * sizeof(uint8_t));
BE_STREAM_TO_ARRAY(p, attr_entry->name.p_str, attr_entry->name.str_len);
@@ -493,7 +481,7 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
__func__, set_br_pl_rsp->status, set_br_pl_rsp->num_items,
set_br_pl_rsp->charset_id, set_br_pl_rsp->folder_depth);
- set_br_pl_rsp->p_folders = (tAVRC_NAME*)osi_malloc(
+ set_br_pl_rsp->p_folders = (tAVRC_NAME*)osi_calloc(
set_br_pl_rsp->folder_depth * sizeof(tAVRC_NAME));
/* Read each of the folder in the depth */
@@ -553,7 +541,7 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(tAVRC_MSG_VENDOR* p_msg,
p++; /* skip the reserved/packe_type byte */
uint16_t len;
- uint16_t min_len = 0;
+ uint32_t min_len = 0;
BE_STREAM_TO_UINT16(len, p);
AVRC_TRACE_DEBUG("%s ctype:0x%x pdu:0x%x, len:%d vendor_len=0x%x", __func__,
p_msg->hdr.ctype, p_result->pdu, len, p_msg->vendor_len);
@@ -827,12 +815,8 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(tAVRC_MSG_VENDOR* p_msg,
BE_STREAM_TO_UINT32(p_attrs[i].attr_id, p);
BE_STREAM_TO_UINT16(p_attrs[i].name.charset_id, p);
BE_STREAM_TO_UINT16(p_attrs[i].name.str_len, p);
- if (static_cast<uint16_t>(min_len + p_attrs[i].name.str_len) <
- min_len) {
- // Check for overflow
- android_errorWriteLog(0x534e4554, "205570663");
- }
- if (len - min_len < p_attrs[i].name.str_len) {
+ min_len += p_attrs[i].name.str_len;
+ if (len < min_len) {
for (int j = 0; j < i; j++) {
osi_free(p_attrs[j].name.p_str);
}
@@ -840,7 +824,6 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(tAVRC_MSG_VENDOR* p_msg,
p_result->get_attrs.num_attrs = 0;
goto length_error;
}
- min_len += p_attrs[i].name.str_len;
if (p_attrs[i].name.str_len > 0) {
p_attrs[i].name.p_str =
(uint8_t*)osi_calloc(p_attrs[i].name.str_len);
diff --git a/stack/avrc/avrc_pars_tg.cc b/stack/avrc/avrc_pars_tg.cc
index 98a6495de..22e32d4d9 100644
--- a/stack/avrc/avrc_pars_tg.cc
+++ b/stack/avrc/avrc_pars_tg.cc
@@ -444,7 +444,7 @@ static tAVRC_STS avrc_pars_browsing_cmd(tAVRC_MSG_BROWSE* p_msg,
uint8_t* p = p_msg->p_browse_data;
int count;
- uint16_t min_len = 3;
+ uint32_t min_len = 3;
RETURN_STATUS_IF_FALSE(AVRC_STS_BAD_CMD, (p_msg->browse_len >= min_len),
"msg too short");