aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrian Delwiche <delwiche@google.com>2023-05-23 23:09:48 +0000
committerJulian Veit <Claymore1298@gmail.com>2024-01-09 07:17:03 +0100
commit1818968925214a3f7dc5a9a11280c5ce34d1fbb0 (patch)
tree551d6d86e3fd4e93627a986302ae78ffb45763d9
parent91e75155c948fbfdd7f747b4df01732bf0e03aaf (diff)
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:66e2be0585514de92e8a31df09ab31528fd67e20) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:5d1a3febede9f835797cf5feff978a9f007f2593) Merged-In: If7752f6edd749d6d5a4bb957b4824c22b5602737 Change-Id: If7752f6edd749d6d5a4bb957b4824c22b5602737
-rw-r--r--system/stack/btm/btm_ble_gap.cc62
-rw-r--r--system/stack/btu/btu_hcif.cc6
2 files changed, 54 insertions, 14 deletions
diff --git a/system/stack/btm/btm_ble_gap.cc b/system/stack/btm/btm_ble_gap.cc
index e781bca8d2..9fba1a5af5 100644
--- a/system/stack/btm/btm_ble_gap.cc
+++ b/system/stack/btm/btm_ble_gap.cc
@@ -1433,6 +1433,16 @@ void btm_ble_biginfo_adv_report_rcvd(uint8_t* p, uint16_t param_len) {
uint16_t sync_handle, iso_interval, max_pdu, max_sdu;
uint8_t num_bises, nse, bn, pto, irc, phy, framing, encryption;
uint32_t sdu_interval;
+
+ // 2 bytes for sync handle, 1 byte for num_bises, 1 byte for nse, 2 bytes for
+ // iso_interval, 1 byte each for bn, pto, irc, 2 bytes for max_pdu, 3 bytes
+ // for sdu_interval, 2 bytes for max_sdu, 1 byte each for phy, framing,
+ // encryption
+ if (param_len < 19) {
+ LOG_ERROR("Insufficient data");
+ return;
+ }
+
STREAM_TO_UINT16(sync_handle, p);
STREAM_TO_UINT8(num_bises, p);
STREAM_TO_UINT8(nse, p);
@@ -2562,20 +2572,27 @@ void btm_ble_process_ext_adv_pkt(uint8_t data_len, const uint8_t* data) {
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_cb.ble_ctr_cb.is_ble_scan_active()) 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);
- constexpr int extended_report_header_size = 24;
while (num_reports--) {
- if (p + extended_report_header_size > 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;
}
@@ -2595,8 +2612,11 @@ void btm_ble_process_ext_adv_pkt(uint8_t data_len, const uint8_t* data) {
const 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;
}
@@ -2629,18 +2649,28 @@ void btm_ble_process_adv_pkt(uint8_t data_len, const uint8_t* data) {
const 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_cb.ble_ctr_cb.is_ble_scan_active()) 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);
- constexpr int report_header_size = 10;
while (num_reports--) {
- if (p + report_header_size > 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;
}
@@ -2652,8 +2682,12 @@ void btm_ble_process_adv_pkt(uint8_t data_len, const uint8_t* data) {
const 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/system/stack/btu/btu_hcif.cc b/system/stack/btu/btu_hcif.cc
index ec05df2e13..f00effe87d 100644
--- a/system/stack/btu/btu_hcif.cc
+++ b/system/stack/btu/btu_hcif.cc
@@ -1693,6 +1693,12 @@ static void btu_ble_data_length_change_evt(uint8_t* p, uint16_t evt_len) {
return;
}
+ // 2 bytes each for handle, tx_data_len, TxTimer, rx_data_len
+ if (evt_len < 8) {
+ LOG_ERROR("Event packet too short");
+ return;
+ }
+
STREAM_TO_UINT16(handle, p);
STREAM_TO_UINT16(tx_data_len, p);
p += 2; /* Skip the TxTimer */