Merge branch 'dev/11/fp3/security-aosp-rvc-release' into int/11/fp3
* dev/11/fp3/security-aosp-rvc-release:
Fix an OOB write bug in attp_build_read_by_type_value_cmd
Fix an OOB bug in btif_to_bta_response and attp_build_value_cmd
Fix some OOB errors in BTM parsing
Change-Id: Id8f0796329cf8e7003ab2d2a8b9372a702d923ee
diff --git a/btif/src/btif_gatt_util.cc b/btif/src/btif_gatt_util.cc
index 16f2275..a0798df 100644
--- a/btif/src/btif_gatt_util.cc
+++ b/btif/src/btif_gatt_util.cc
@@ -18,6 +18,8 @@
#define LOG_TAG "bt_btif_gatt"
+#include <algorithm>
+
#include "btif_gatt_util.h"
#include <errno.h>
@@ -48,9 +50,9 @@
void btif_to_bta_response(tGATTS_RSP* p_dest, btgatt_response_t* p_src) {
p_dest->attr_value.auth_req = p_src->attr_value.auth_req;
p_dest->attr_value.handle = p_src->attr_value.handle;
- p_dest->attr_value.len = p_src->attr_value.len;
+ p_dest->attr_value.len = std::min<uint16_t>(p_src->attr_value.len, GATT_MAX_ATTR_LEN);
p_dest->attr_value.offset = p_src->attr_value.offset;
- memcpy(p_dest->attr_value.value, p_src->attr_value.value, GATT_MAX_ATTR_LEN);
+ memcpy(p_dest->attr_value.value, p_src->attr_value.value, p_dest->attr_value.len);
}
/*******************************************************************************
diff --git a/stack/btm/btm_ble_gap.cc b/stack/btm/btm_ble_gap.cc
index 37cfeb5..dbb92d2 100644
--- a/stack/btm/btm_ble_gap.cc
+++ b/stack/btm/btm_ble_gap.cc
@@ -1795,19 +1795,27 @@
advertising_sid;
int8_t rssi, tx_power;
uint16_t event_type, periodic_adv_int, direct_address_type;
+ size_t bytes_to_process;
/* Only process the results if the inquiry is still active */
if (!BTM_BLE_IS_SCAN_ACTIVE(btm_cb.ble_ctr_cb.scan_activity)) return;
+ bytes_to_process = 1;
+
+ if (data_len < bytes_to_process) {
+ LOG(ERROR) << "Malformed LE extended advertising packet: not enough room "
+ "for num reports";
+ return;
+ }
+
/* Extract the number of reports in this event. */
STREAM_TO_UINT8(num_reports, p);
while (num_reports--) {
- if (p > data + data_len) {
- // TODO(jpawlowski): we should crash the stack here
- BTM_TRACE_ERROR(
- "Malformed LE Extended Advertising Report Event from controller - "
- "can't loop the data");
+ bytes_to_process += 24;
+ if (data_len < bytes_to_process) {
+ LOG(ERROR) << "Malformed LE extended advertising packet: not enough room "
+ "for metadata";
return;
}
@@ -1827,8 +1835,11 @@
uint8_t* pkt_data = p;
p += pkt_data_len; /* Advance to the the next packet*/
- if (p > data + data_len) {
- LOG(ERROR) << "Invalid pkt_data_len: " << +pkt_data_len;
+
+ bytes_to_process += pkt_data_len;
+ if (data_len < bytes_to_process) {
+ LOG(ERROR) << "Malformed LE extended advertising packet: not enough room "
+ "for packet data";
return;
}
@@ -1857,17 +1868,28 @@
uint8_t* p = data;
uint8_t legacy_evt_type, addr_type, num_reports, pkt_data_len;
int8_t rssi;
+ size_t bytes_to_process;
/* Only process the results if the inquiry is still active */
if (!BTM_BLE_IS_SCAN_ACTIVE(btm_cb.ble_ctr_cb.scan_activity)) return;
+ bytes_to_process = 1;
+
+ if (data_len < bytes_to_process) {
+ LOG(ERROR)
+ << "Malformed LE advertising packet: not enough room for num reports";
+ return;
+ }
+
/* Extract the number of reports in this event. */
STREAM_TO_UINT8(num_reports, p);
while (num_reports--) {
- if (p > data + data_len) {
- // TODO(jpawlowski): we should crash the stack here
- BTM_TRACE_ERROR("Malformed LE Advertising Report Event from controller");
+ bytes_to_process += 9;
+
+ if (data_len < bytes_to_process) {
+ LOG(ERROR)
+ << "Malformed LE advertising packet: not enough room for metadata";
return;
}
@@ -1879,8 +1901,12 @@
uint8_t* pkt_data = p;
p += pkt_data_len; /* Advance to the the rssi byte */
- if (p > data + data_len - sizeof(rssi)) {
- LOG(ERROR) << "Invalid pkt_data_len: " << +pkt_data_len;
+
+ // include rssi for this check
+ bytes_to_process += pkt_data_len + 1;
+ if (data_len < bytes_to_process) {
+ LOG(ERROR) << "Malformed LE advertising packet: not enough room for "
+ "packet data and/or RSSI";
return;
}
diff --git a/stack/btu/btu_hcif.cc b/stack/btu/btu_hcif.cc
index 2d7033d..c33faa1 100644
--- a/stack/btu/btu_hcif.cc
+++ b/stack/btu/btu_hcif.cc
@@ -2226,6 +2226,12 @@
return;
}
+ // 2 bytes each for handle, tx_data_len, TxTimer, rx_data_len
+ if (evt_len < 8) {
+ LOG_ERROR(LOG_TAG, "Event packet too short");
+ return;
+ }
+
STREAM_TO_UINT16(handle, p);
STREAM_TO_UINT16(tx_data_len, p);
p += 2; /* Skip the TxTimer */
diff --git a/stack/gatt/att_protocol.cc b/stack/gatt/att_protocol.cc
index 5d3d4a8..cdf472e 100644
--- a/stack/gatt/att_protocol.cc
+++ b/stack/gatt/att_protocol.cc
@@ -157,8 +157,14 @@
tGATT_FIND_TYPE_VALUE* p_value_type) {
uint8_t* p;
uint16_t len = p_value_type->value_len;
- BT_HDR* p_buf =
- (BT_HDR*)osi_malloc(sizeof(BT_HDR) + payload_size + L2CAP_MIN_OFFSET);
+ BT_HDR* p_buf = nullptr;
+
+ if (payload_size < 5) {
+ return nullptr;
+ }
+
+ p_buf =
+ (BT_HDR*)osi_malloc(sizeof(BT_HDR) + payload_size + L2CAP_MIN_OFFSET);
p = (uint8_t*)(p_buf + 1) + L2CAP_MIN_OFFSET;
p_buf->offset = L2CAP_MIN_OFFSET;