Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b32ccd82 authored by Brian Delwiche's avatar Brian Delwiche Committed by Android Build Coastguard Worker
Browse files

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
parent 1682cdf5

system/stack/btm/btm_ble_gap.cc

+48 −14
Original line number Diff line number Diff line
@@ -1482,6 +1482,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);
@@ -2602,20 +2612,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;

    }
@@ -2635,8 +2652,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;

    }
@@ -2669,18 +2689,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;

    }
@@ -2692,8 +2722,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;

    }

system/stack/btu/btu_hcif.cc

+6 −0
Original line number Diff line number Diff line
@@ -1585,6 +1585,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 */