Fix some OOB errors in BTM parsing
Some HCI BLE events are missing bounds checks, leading to possible OOB
access. Add the appropriate bounds checks on the packets.
Bug: 279169188
Test: atest bluetooth_test_gd_unit, net_test_stack_btm
Tag: #security
Ignore-AOSP-First: Security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:949eb6b355f1bdcfb5567ebe1b7f00a61b6fb066)
Merged-In: Icf2953c687d9c4e2ca9629474151b8deab6c5f57
Change-Id: Icf2953c687d9c4e2ca9629474151b8deab6c5f57
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 */