Merge branch 'dev/11/fp3/security-aosp-rvc-release' into int/11/fp3
* dev/11/fp3/security-aosp-rvc-release:
Fix UAF in gatt_cl.cc
Fix potential abort in btu_av_act.cc
Fix integer overflow in build_read_multi_rsp
Fix an integer overflow bug in avdt_msg_asmbl
Change-Id: I91504b7d45510b318ce32df1c5c12f4e05f17691
diff --git a/bta/av/bta_av_act.cc b/bta/av/bta_av_act.cc
index 533b15d..20696bd 100644
--- a/bta/av/bta_av_act.cc
+++ b/bta/av/bta_av_act.cc
@@ -1008,7 +1008,10 @@
av.remote_cmd.rc_handle = p_data->rc_msg.handle;
(*p_cb->p_cback)(evt, &av);
/* If browsing message, then free the browse message buffer */
- bta_av_rc_free_browse_msg(p_cb, p_data);
+ if (p_data->rc_msg.opcode == AVRC_OP_BROWSE &&
+ p_data->rc_msg.msg.browse.p_browse_pkt != NULL) {
+ bta_av_rc_free_browse_msg(p_cb, p_data);
+ }
}
}
diff --git a/stack/avdt/avdt_msg.cc b/stack/avdt/avdt_msg.cc
index bf83d19..3f8713c 100644
--- a/stack/avdt/avdt_msg.cc
+++ b/stack/avdt/avdt_msg.cc
@@ -1289,14 +1289,14 @@
* NOTE: The buffer is allocated above at the beginning of the
* reassembly, and is always of size BT_DEFAULT_BUFFER_SIZE.
*/
- uint16_t buf_len = BT_DEFAULT_BUFFER_SIZE - sizeof(BT_HDR);
+ size_t buf_len = BT_DEFAULT_BUFFER_SIZE - sizeof(BT_HDR);
/* adjust offset and len of fragment for header byte */
p_buf->offset += AVDT_LEN_TYPE_CONT;
p_buf->len -= AVDT_LEN_TYPE_CONT;
/* verify length */
- if ((p_ccb->p_rx_msg->offset + p_buf->len) > buf_len) {
+ if (((size_t) p_ccb->p_rx_msg->offset + (size_t) p_buf->len) > buf_len) {
/* won't fit; free everything */
AVDT_TRACE_WARNING("%s: Fragmented message too big!", __func__);
osi_free_and_reset((void**)&p_ccb->p_rx_msg);
diff --git a/stack/gatt/gatt_cl.cc b/stack/gatt/gatt_cl.cc
index ff1e5af..14732cd 100644
--- a/stack/gatt/gatt_cl.cc
+++ b/stack/gatt/gatt_cl.cc
@@ -586,12 +586,17 @@
memcpy(value.value, p, value.len);
+ bool subtype_is_write_prepare = (p_clcb->op_subtype == GATT_WRITE_PREPARE);
+
if (!gatt_check_write_long_terminate(tcb, p_clcb, &value)) {
gatt_send_prepare_write(tcb, p_clcb);
return;
}
- if (p_clcb->op_subtype == GATT_WRITE_PREPARE) {
+ // We now know that we have not terminated, or else we would have returned
+ // early. We free the buffer only if the subtype is not equal to
+ // GATT_WRITE_PREPARE, so checking here is adequate to prevent UAF.
+ if (subtype_is_write_prepare) {
/* application should verify handle offset
and value are matched or not */
gatt_end_operation(p_clcb, p_clcb->status, &value);
diff --git a/stack/gatt/gatt_sr.cc b/stack/gatt/gatt_sr.cc
index 4b4b5da..252732c 100644
--- a/stack/gatt/gatt_sr.cc
+++ b/stack/gatt/gatt_sr.cc
@@ -114,7 +114,8 @@
******************************************************************************/
static bool process_read_multi_rsp(tGATT_SR_CMD* p_cmd, tGATT_STATUS status,
tGATTS_RSP* p_msg, uint16_t mtu) {
- uint16_t ii, total_len, len;
+ uint16_t ii;
+ size_t total_len, len;
uint8_t* p;
bool is_overflow = false;
@@ -169,16 +170,22 @@
len = p_rsp->attr_value.len - (total_len - mtu);
is_overflow = true;
VLOG(1) << StringPrintf(
- "multi read overflow available len=%d val_len=%d", len,
+ "multi read overflow available len=%zu val_len=%d", len,
p_rsp->attr_value.len);
} else {
len = p_rsp->attr_value.len;
}
if (p_rsp->attr_value.handle == p_cmd->multi_req.handles[ii]) {
- memcpy(p, p_rsp->attr_value.value, len);
- if (!is_overflow) p += len;
- p_buf->len += len;
+ // check for possible integer overflow
+ if (p_buf->len + len <= UINT16_MAX) {
+ memcpy(p, p_rsp->attr_value.value, len);
+ if (!is_overflow) p += len;
+ p_buf->len += len;
+ } else {
+ p_cmd->status = GATT_NOT_FOUND;
+ break;
+ }
} else {
p_cmd->status = GATT_NOT_FOUND;
break;