Fix more memory-unsafe logging In various locations around the stack, log statements use structures that may, in exceptional cases, have been freed by preceding calls. This can lead to use after free and potentially to security vulnerabilities. Use local variables instead, or store the length before the call if no local variable is already convenient. Bug: 375404242 Bug: 375398779 Bug: 375397720 Bug: 375397164 Bug: 375397370 Bug: 375396810 Bug: 375159652 Bug: 375160214 Bug: 375159480 Test: m libbluetooth Test: researcher POC Flag: EXEMPT trivial logic fix Ignore-AOSP-First: security Tag: #security (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:43cfd234de9ba9557118b0014513269cc1aeefda) Merged-In: I6289907e86786eb2e10a163f7fb5d2557eab00bc Change-Id: I6289907e86786eb2e10a163f7fb5d2557eab00bc
diff --git a/system/stack/avct/avct_lcb_act.cc b/system/stack/avct/avct_lcb_act.cc index b035721..14537d1 100644 --- a/system/stack/avct/avct_lcb_act.cc +++ b/system/stack/avct/avct_lcb_act.cc
@@ -723,9 +723,12 @@ p = (uint8_t*)(p_buf + 1) + p_buf->offset; AVCT_BUILD_HDR(p, label, AVCT_PKT_TYPE_SINGLE, AVCT_REJ); UINT16_TO_BE_STREAM(p, pid); + + uint16_t len = p_buf->len; + if (L2CA_DataWrite(p_lcb->ch_lcid, p_buf) != L2CAP_DW_SUCCESS) { log::warn("Unable to write L2CAP data peer:{} cid:{} len:{}", - p_lcb->peer_addr, p_lcb->ch_lcid, p_buf->len); + p_lcb->peer_addr, p_lcb->ch_lcid, len); } } }
diff --git a/system/stack/bnep/bnep_main.cc b/system/stack/bnep/bnep_main.cc index e761f12..ea33c23 100644 --- a/system/stack/bnep/bnep_main.cc +++ b/system/stack/bnep/bnep_main.cc
@@ -304,9 +304,11 @@ if (!p_buf) break; + uint16_t len = p_buf->len; + if (L2CA_DataWrite(l2cap_cid, p_buf) != L2CAP_DW_SUCCESS) { log::warn("Unable to write L2CAP data peer:{} cid:{} len:{}", - p_bcb->rem_bda, l2cap_cid, p_buf->len); + p_bcb->rem_bda, l2cap_cid, len); } } }
diff --git a/system/stack/bnep/bnep_utils.cc b/system/stack/bnep/bnep_utils.cc index 7129ccf..5213e09 100644 --- a/system/stack/bnep/bnep_utils.cc +++ b/system/stack/bnep/bnep_utils.cc
@@ -415,9 +415,11 @@ fixed_queue_enqueue(p_bcb->xmit_q, p_buf); } } else { + uint16_t len = p_buf->len; + if (L2CA_DataWrite(p_bcb->l2cap_cid, p_buf) != L2CAP_DW_SUCCESS) { log::warn("Unable to write L2CAP data peer:{} cid:{} len:{}", - p_bcb->rem_bda, p_bcb->l2cap_cid, p_buf->len); + p_bcb->rem_bda, p_bcb->l2cap_cid, len); } } }
diff --git a/system/stack/hid/hidd_conn.cc b/system/stack/hid/hidd_conn.cc index 54ffdd6..e03f11e 100644 --- a/system/stack/hid/hidd_conn.cc +++ b/system/stack/hid/hidd_conn.cc
@@ -88,10 +88,11 @@ // send outstanding data on intr if (hd_cb.pending_data) { - if (L2CA_DataWrite(p_hcon->intr_cid, hd_cb.pending_data) != + uint16_t len = hd_cb.pending_data->len; + + if (L2CA_DataWrite(p_hcon->intr_cid, hd_cb.pending_data) != L2CAP_DW_SUCCESS) { - log::warn("Unable to write L2CAP data cid:{} len:{}", p_hcon->intr_cid, - hd_cb.pending_data->len); + log::warn("Unable to write L2CAP data cid:{} len:{}", p_hcon->intr_cid, len); } hd_cb.pending_data = NULL; }
diff --git a/system/stack/rfcomm/rfc_ts_frames.cc b/system/stack/rfcomm/rfc_ts_frames.cc index dd20e82..c1b3c27 100644 --- a/system/stack/rfcomm/rfc_ts_frames.cc +++ b/system/stack/rfcomm/rfc_ts_frames.cc
@@ -201,9 +201,11 @@ if (dlci == RFCOMM_MX_DLCI) { rfc_check_send_cmd(p_mcb, p_buf); } else { + uint16_t len = p_buf->len; + if (L2CA_DataWrite(p_mcb->lcid, p_buf) != L2CAP_DW_SUCCESS) { log::warn("Unable to write L2CAP data peer:{} cid:{} len:{}", - p_mcb->bd_addr, p_mcb->lcid, p_buf->len); + p_mcb->bd_addr, p_mcb->lcid, len); } } }
diff --git a/system/stack/rfcomm/rfc_utils.cc b/system/stack/rfcomm/rfc_utils.cc index ccb21a8..d32cb74 100644 --- a/system/stack/rfcomm/rfc_utils.cc +++ b/system/stack/rfcomm/rfc_utils.cc
@@ -426,9 +426,10 @@ while (!p_mcb->l2cap_congested) { BT_HDR* p = (BT_HDR*)fixed_queue_try_dequeue(p_mcb->cmd_q); if (p == NULL) break; + uint16_t len = p->len; if (L2CA_DataWrite(p_mcb->lcid, p) != L2CAP_DW_SUCCESS) { log::warn("Unable to write L2CAP data peer:{} cid:{} len:{}", - p_mcb->bd_addr, p_mcb->lcid, p->len); + p_mcb->bd_addr, p_mcb->lcid, len); } } }
diff --git a/system/stack/sdp/sdp_discovery.cc b/system/stack/sdp/sdp_discovery.cc index ff6bbd4..8deaa98 100644 --- a/system/stack/sdp/sdp_discovery.cc +++ b/system/stack/sdp/sdp_discovery.cc
@@ -863,7 +863,7 @@ if (L2CA_DataWrite(p_ccb->connection_id, p_msg) != L2CAP_DW_SUCCESS) { log::warn("Unable to write L2CAP data peer:{} cid:{} len:{}", - p_ccb->device_address, p_ccb->connection_id, p_msg->len); + p_ccb->device_address, p_ccb->connection_id, p - p_start); } /* Start inactivity timer */
diff --git a/system/stack/sdp/sdp_server.cc b/system/stack/sdp/sdp_server.cc index 2a127b7..409d3ec 100644 --- a/system/stack/sdp/sdp_server.cc +++ b/system/stack/sdp/sdp_server.cc
@@ -305,7 +305,7 @@ /* Send the buffer through L2CAP */ if (L2CA_DataWrite(p_ccb->connection_id, p_buf) != L2CAP_DW_SUCCESS) { log::warn("Unable to write L2CAP data peer:{} cid:{} len:{}", - p_ccb->device_address, p_ccb->connection_id, p_buf->len); + p_ccb->device_address, p_ccb->connection_id, p_rsp - p_rsp_start); } } @@ -703,7 +703,7 @@ /* Send the buffer through L2CAP */ if (L2CA_DataWrite(p_ccb->connection_id, p_buf) != L2CAP_DW_SUCCESS) { log::warn("Unable to write L2CAP data peer:{} cid:{} len:{}", - p_ccb->device_address, p_ccb->connection_id, p_buf->len); + p_ccb->device_address, p_ccb->connection_id, p_rsp - p_rsp_start); } }