6 months worth of security patches!

bta/av/bta_av_act.cc

@@ -1975,8 +1975,23 @@ void bta_av_rc_disc_done(UNUSED_ATTR tBTA_AV_DATA* p_data) {
         if (p_lcb) {
           rc_handle = bta_av_rc_create(p_cb, AVCT_INT,
                                        (uint8_t)(p_scb->hdi + 1), p_lcb->lidx);
-          p_cb->rcb[rc_handle].peer_features = peer_features;
-          p_cb->rcb[rc_handle].cover_art_psm = cover_art_psm;
+          if (rc_handle < BTA_AV_NUM_RCB) {
+            p_cb->rcb[rc_handle].peer_features = peer_features;
+            p_cb->rcb[rc_handle].cover_art_psm = cover_art_psm;
+          } else {
+            /* cannot create valid rc_handle for current device. report failure
+             */
+            APPL_TRACE_ERROR("%s: no link resources available", __func__);
+            p_scb->use_rc = false;
+            tBTA_AV_RC_OPEN rc_open;
+            rc_open.peer_addr = p_scb->PeerAddress();
+            rc_open.peer_features = 0;
+            rc_open.cover_art_psm = 0;
+            rc_open.status = BTA_AV_FAIL_RESOURCES;
+            tBTA_AV bta_av_data;
+            bta_av_data.rc_open = rc_open;
+            (*p_cb->p_cback)(BTA_AV_RC_OPEN_EVT, &bta_av_data);
+          }
         } else {
           APPL_TRACE_ERROR("%s: can not find LCB!!", __func__);
         }

btif/src/btif_hh.cc

@@ -1092,6 +1092,38 @@ static void btif_hh_upstreams_evt(uint16_t event, char* p_param) {
   }
 }
 
+/*******************************************************************************
+ *
+ * Function         btif_hh_hsdata_rpt_copy_cb
+ *
+ * Description      Deep copies the tBTA_HH_HSDATA structure
+ *
+ * Returns          void
+ *
+ ******************************************************************************/
+
+static void btif_hh_hsdata_rpt_copy_cb(uint16_t event, char* p_dest,
+                                       char* p_src) {
+  tBTA_HH_HSDATA* p_dst_data = (tBTA_HH_HSDATA*)p_dest;
+  tBTA_HH_HSDATA* p_src_data = (tBTA_HH_HSDATA*)p_src;
+  BT_HDR* hdr;
+
+  if (!p_src) {
+    BTIF_TRACE_ERROR("%s: Nothing to copy", __func__);
+    return;
+  }
+
+  memcpy(p_dst_data, p_src_data, sizeof(tBTA_HH_HSDATA));
+
+  hdr = p_src_data->rsp_data.p_rpt_data;
+  if (hdr != NULL) {
+    uint8_t* p_data = ((uint8_t*)p_dst_data) + sizeof(tBTA_HH_HSDATA);
+    memcpy(p_data, hdr, BT_HDR_SIZE + hdr->offset + hdr->len);
+
+    p_dst_data->rsp_data.p_rpt_data = (BT_HDR*)p_data;
+  }
+}
+
 /*******************************************************************************
  *
  * Function         bte_hh_evt
@@ -1105,6 +1137,7 @@ static void btif_hh_upstreams_evt(uint16_t event, char* p_param) {
 void bte_hh_evt(tBTA_HH_EVT event, tBTA_HH* p_data) {
   bt_status_t status;
   int param_len = 0;
+  tBTIF_COPY_CBACK* p_copy_cback = NULL;
 
   if (BTA_HH_ENABLE_EVT == event)
     param_len = sizeof(tBTA_HH_STATUS);
@@ -1116,11 +1149,18 @@ void bte_hh_evt(tBTA_HH_EVT event, tBTA_HH* p_data) {
     param_len = sizeof(tBTA_HH_CBDATA);
   else if (BTA_HH_GET_DSCP_EVT == event)
     param_len = sizeof(tBTA_HH_DEV_DSCP_INFO);
-  else if ((BTA_HH_GET_PROTO_EVT == event) || (BTA_HH_GET_RPT_EVT == event) ||
-           (BTA_HH_GET_IDLE_EVT == event))
+  else if ((BTA_HH_GET_PROTO_EVT == event) || (BTA_HH_GET_IDLE_EVT == event))
+    param_len = sizeof(tBTA_HH_HSDATA);
+  else if (BTA_HH_GET_RPT_EVT == event) {
+    BT_HDR* hdr = p_data->hs_data.rsp_data.p_rpt_data;
     param_len = sizeof(tBTA_HH_HSDATA);
-  else if ((BTA_HH_SET_PROTO_EVT == event) || (BTA_HH_SET_RPT_EVT == event) ||
-           (BTA_HH_VC_UNPLUG_EVT == event) || (BTA_HH_SET_IDLE_EVT == event))
+
+    if (hdr != NULL) {
+      p_copy_cback = btif_hh_hsdata_rpt_copy_cb;
+      param_len += BT_HDR_SIZE + hdr->offset + hdr->len;
+    }
+  } else if ((BTA_HH_SET_PROTO_EVT == event) || (BTA_HH_SET_RPT_EVT == event) ||
+             (BTA_HH_VC_UNPLUG_EVT == event) || (BTA_HH_SET_IDLE_EVT == event))
     param_len = sizeof(tBTA_HH_CBDATA);
   else if ((BTA_HH_ADD_DEV_EVT == event) || (BTA_HH_RMV_DEV_EVT == event))
     param_len = sizeof(tBTA_HH_DEV_INFO);
@@ -1129,7 +1169,7 @@ void bte_hh_evt(tBTA_HH_EVT event, tBTA_HH* p_data) {
   /* switch context to btif task context (copy full union size for convenience)
    */
   status = btif_transfer_context(btif_hh_upstreams_evt, (uint16_t)event,
-                                 (char*)p_data, param_len, NULL);
+                                 (char*)p_data, param_len, p_copy_cback);
 
   /* catch any failed context transfers */
   ASSERTC(status == BT_STATUS_SUCCESS, "context transfer failed", status);

btif/src/btif_rc.cc

@@ -1955,6 +1955,11 @@ static bt_status_t register_notification_rsp(
                    dump_rc_notification_event_id(event_id));
   std::unique_lock<std::mutex> lock(btif_rc_cb.lock);
 
+  if (event_id > MAX_RC_NOTIFICATIONS) {
+    BTIF_TRACE_ERROR("Invalid event id");
+    return BT_STATUS_PARM_INVALID;
+  }
+
   memset(&(avrc_rsp.reg_notif), 0, sizeof(tAVRC_REG_NOTIF_RSP));
 
   avrc_rsp.reg_notif.event_id = event_id;

gd/Android.bp

@@ -335,6 +335,7 @@ cc_test {
         ":BluetoothShimTestSources",
         ":BluetoothSecurityUnitTestSources",
         ":BluetoothStorageUnitTestSources",
+        ":BluetoothBtaaSources_linux_generic_tests",
     ],
     generated_headers: [
         "BluetoothGeneratedBundlerSchema_h_bfbs",

gd/btaa/Android.bp

@@ -30,3 +30,10 @@ filegroup {
         "linux_generic/wakelock_processor.cc",
     ],
 }
+
+filegroup {
+    name: "BluetoothBtaaSources_linux_generic_tests",
+    srcs: [
+        "linux_generic/attribution_processor_tests.cc",
+    ],
+}

gd/btaa/attribution_processor.h

@@ -58,7 +58,20 @@ class AttributionProcessor {
   void Dump(
       std::promise<flatbuffers::Offset<ActivityAttributionData>> promise, flatbuffers::FlatBufferBuilder* fb_builder);
 
+  using ClockType = std::chrono::time_point<std::chrono::system_clock>;
+  using NowFunc = ClockType (*)();
+
+  // by default, we use the std::chrono::system_clock::now implementation to
+  // get the current timestamp
+  AttributionProcessor() : now_func_(std::chrono::system_clock::now) {}
+  // in other cases, we may need to use different implementation
+  // e.g., for testing purposes
+  AttributionProcessor(NowFunc func) : now_func_(func) {}
+
  private:
+  // this function is added for testing support in
+  // OnWakelockReleased
+  NowFunc now_func_ = std::chrono::system_clock::now;
   bool wakeup_ = false;
   std::unordered_map<AddressActivityKey, BtaaAggregationEntry, AddressActivityKeyHasher> btaa_aggregator_;
   std::unordered_map<AddressActivityKey, BtaaAggregationEntry, AddressActivityKeyHasher> wakelock_duration_aggregator_;

gd/btaa/linux_generic/attribution_processor.cc

@@ -64,7 +64,7 @@ void AttributionProcessor::OnWakelockReleased(uint32_t duration_ms) {
   }
 
   ms_per_byte = duration_ms / total_byte_count;
-  auto cur_time = std::chrono::system_clock::now();
+  auto cur_time = now_func_();
   for (auto& it : wakelock_duration_aggregator_) {
     it.second.wakelock_duration_ms = ms_per_byte * it.second.byte_count;
     if (btaa_aggregator_.find(it.first) == btaa_aggregator_.end()) {
@@ -91,12 +91,15 @@ void AttributionProcessor::OnWakelockReleased(uint32_t duration_ms) {
     return;
   }
   // Trim down the transient entries in the aggregator to avoid that it overgrows
-  for (auto& it : btaa_aggregator_) {
+  auto it = btaa_aggregator_.begin();
+  while (it != btaa_aggregator_.end()) {
     auto elapsed_time_sec =
-        std::chrono::duration_cast<std::chrono::seconds>(cur_time - it.second.creation_time).count();
+        std::chrono::duration_cast<std::chrono::seconds>(cur_time - it->second.creation_time).count();
     if (elapsed_time_sec > kDurationTransientDeviceActivityEntrySecs &&
-        it.second.byte_count < kByteCountTransientDeviceActivityEntry) {
-      btaa_aggregator_.erase(it.first);
+        it->second.byte_count < kByteCountTransientDeviceActivityEntry) {
+      it = btaa_aggregator_.erase(it);
+    } else {
+      it++;
     }
   }
 }

gd/btaa/linux_generic/attribution_processor_tests.cc

@@ -0,0 +1,84 @@
+/*
+ * Copyright 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <base/strings/stringprintf.h>
+#include <gtest/gtest.h>
+#include <unistd.h>
+
+#include <chrono>
+#include <functional>
+#include <memory>
+#include <vector>
+
+#include "btaa/activity_attribution.h"
+#include "btaa/attribution_processor.h"
+
+using bluetooth::hci::Address;
+using namespace bluetooth::activity_attribution;
+using namespace std::chrono;
+
+// mock for std::chrono::system_clock::now
+static AttributionProcessor::ClockType now_ret_val;
+static AttributionProcessor::ClockType fake_now() {
+  return now_ret_val;
+}
+
+class AttributionProcessorTest : public ::testing::Test {
+ protected:
+  void SetUp() override {
+    pAttProc = std::make_unique<AttributionProcessor>(fake_now);
+  }
+  void TearDown() override {
+    pAttProc.reset();
+  }
+
+  std::unique_ptr<AttributionProcessor> pAttProc;
+};
+
+static void fake_now_set_current() {
+  now_ret_val = system_clock::now();
+}
+
+static void fake_now_advance_1000sec() {
+  now_ret_val += seconds(1000s);
+}
+
+TEST_F(AttributionProcessorTest, UAFInOnWakelockReleasedRegressionTest) {
+  std::vector<BtaaHciPacket> btaaPackets;
+  Address addr;
+
+  fake_now_set_current();
+
+  // setup the condition 1 for triggering erase operation
+  // add 220 entries in app_activity_aggregator_
+  // and btaa_aggregator_
+  for (int i = 0; i < 220; i++) {
+    std::string addrStr = base::StringPrintf("21:43:65:87:a9:%02x", i + 10);
+    ASSERT_TRUE(Address::FromString(addrStr, addr));
+    BtaaHciPacket packet(Activity::ACL, addr, 30 * i);
+    btaaPackets.push_back(packet);
+  }
+
+  pAttProc->OnBtaaPackets(btaaPackets);
+  pAttProc->OnWakelockReleased(100);
+
+  // setup the condition 2 for triggering erase operation
+  // make elapsed_time_sec > 900s
+  fake_now_advance_1000sec();
+
+  pAttProc->OnBtaaPackets(btaaPackets);
+  pAttProc->OnWakelockReleased(100);
+}

stack/a2dp/a2dp_sbc.cc

@@ -696,6 +696,11 @@ bool A2DP_BuildCodecHeaderSbc(UNUSED_ATTR const uint8_t* p_codec_info,
     return false;
   }
 
+  // there is a timestamp right following p_buf
+  if (p_buf->offset < 4 + A2DP_SBC_MPL_HDR_LEN) {
+    return false;
+  }
+
   p_buf->offset -= A2DP_SBC_MPL_HDR_LEN;
   uint8_t* p = (uint8_t*)(p_buf + 1) + p_buf->offset;
   p_buf->len += A2DP_SBC_MPL_HDR_LEN;

stack/avdt/avdt_scb_act.cc

@@ -257,19 +257,24 @@ void avdt_scb_hdl_pkt_no_frag(AvdtpScb* p_scb, tAVDT_SCB_EVT* p_data) {
     if (offset > len) goto length_error;
     p += 2;
     BE_STREAM_TO_UINT16(ex_len, p);
-    offset += ex_len * 4;
     p += ex_len * 4;
   }
 
+  if ((p - p_start) >= len) {
+    AVDT_TRACE_WARNING("%s: handling malformatted packet: ex_len too large", __func__);
+    osi_free_and_reset((void**)&p_data->p_pkt);
+    return;
+  }
+  offset = p - p_start;
+
   /* adjust length for any padding at end of packet */
   if (o_p) {
     /* padding length in last byte of packet */
-    pad_len = *(p_start + p_data->p_pkt->len);
+    pad_len = *(p_start + len - 1);
   }
 
   /* do sanity check */
-  if ((offset > p_data->p_pkt->len) ||
-      ((pad_len + offset) > p_data->p_pkt->len)) {
+  if (pad_len >= (len - offset)) {
     AVDT_TRACE_WARNING("Got bad media packet");
     osi_free_and_reset((void**)&p_data->p_pkt);
   }

stack/btm/btm_sec.cc

@@ -3300,22 +3300,6 @@ void btm_sec_encrypt_change(uint16_t handle, tHCI_STATUS status,
         BTM_TRACE_DEBUG("%s start SM over BR/EDR", __func__);
         SMP_BR_PairWith(p_dev_rec->bd_addr);
       }
-    } else {
-      // BR/EDR is successfully encrypted. Correct LK type if needed
-      // (BR/EDR LK derived from LE LTK was used for encryption)
-      if ((encr_enable == 1) && /* encryption is ON for SSP */
-          /* LK type is for BR/EDR SC */
-          (p_dev_rec->link_key_type == BTM_LKEY_TYPE_UNAUTH_COMB_P_256 ||
-           p_dev_rec->link_key_type == BTM_LKEY_TYPE_AUTH_COMB_P_256)) {
-        if (p_dev_rec->link_key_type == BTM_LKEY_TYPE_UNAUTH_COMB_P_256)
-          p_dev_rec->link_key_type = BTM_LKEY_TYPE_UNAUTH_COMB;
-        else /* BTM_LKEY_TYPE_AUTH_COMB_P_256 */
-          p_dev_rec->link_key_type = BTM_LKEY_TYPE_AUTH_COMB;
-
-        BTM_TRACE_DEBUG("updated link key type to %d",
-                        p_dev_rec->link_key_type);
-        btm_send_link_key_notif(p_dev_rec);
-      }
     }
   }
 

stack/gatt/gatt_cl.cc

@@ -587,7 +587,8 @@ void gatt_process_prep_write_rsp(tGATT_TCB& tcb, tGATT_CLCB* p_clcb,
   VLOG(1) << StringPrintf("value resp op_code = %s len = %d",
                           gatt_dbg_op_name(op_code), len);
 
-  if (len < GATT_PREP_WRITE_RSP_MIN_LEN) {
+  if (len < GATT_PREP_WRITE_RSP_MIN_LEN ||
+      len > GATT_PREP_WRITE_RSP_MIN_LEN + sizeof(value.value)) {
     LOG(ERROR) << "illegal prepare write response length, discard";
     gatt_end_operation(p_clcb, GATT_INVALID_PDU, &value);
     return;
@@ -596,7 +597,7 @@ void gatt_process_prep_write_rsp(tGATT_TCB& tcb, tGATT_CLCB* p_clcb,
   STREAM_TO_UINT16(value.handle, p);
   STREAM_TO_UINT16(value.offset, p);
 
-  value.len = len - 4;
+  value.len = len - GATT_PREP_WRITE_RSP_MIN_LEN;
 
   memcpy(value.value, p, value.len);
 

stack/sdp/sdp_db.cc

@@ -349,6 +349,11 @@ bool SDP_AddAttribute(uint32_t handle, uint16_t attr_id, uint8_t attr_type,
   uint16_t xx, yy, zz;
   tSDP_RECORD* p_rec = &sdp_cb.server_db.record[0];
 
+  if (p_val == nullptr) {
+    SDP_TRACE_WARNING("Trying to add attribute with p_val == nullptr, skipped");
+    return (false);
+  }
+
   if (sdp_cb.trace_level >= BT_TRACE_LEVEL_DEBUG) {
     if ((attr_type == UINT_DESC_TYPE) ||
         (attr_type == TWO_COMP_INT_DESC_TYPE) ||
@@ -396,6 +401,13 @@ bool SDP_AddAttribute(uint32_t handle, uint16_t attr_id, uint8_t attr_type,
     if (p_rec->record_handle == handle) {
       tSDP_ATTRIBUTE* p_attr = &p_rec->attribute[0];
 
+      // error out early, no need to look up
+      if (p_rec->free_pad_ptr >= SDP_MAX_PAD_LEN) {
+        SDP_TRACE_ERROR("the free pad for SDP record with handle %d is "
+                        "full, skip adding the attribute", handle);
+        return (false);
+      }
+
       /* Found the record. Now, see if the attribute already exists */
       for (xx = 0; xx < p_rec->num_attributes; xx++, p_attr++) {
         /* The attribute exists. replace it */
@@ -435,15 +447,13 @@ bool SDP_AddAttribute(uint32_t handle, uint16_t attr_id, uint8_t attr_type,
           attr_len = 0;
       }
 
-      if ((attr_len > 0) && (p_val != 0)) {
+      if (attr_len > 0) {
         p_attr->len = attr_len;
         memcpy(&p_rec->attr_pad[p_rec->free_pad_ptr], p_val, (size_t)attr_len);
         p_attr->value_ptr = &p_rec->attr_pad[p_rec->free_pad_ptr];
         p_rec->free_pad_ptr += attr_len;
-      } else if ((attr_len == 0 &&
-                  p_attr->len !=
-                      0) || /* if truncate to 0 length, simply don't add */
-                 p_val == 0) {
+      } else if (attr_len == 0 && p_attr->len != 0) {
+        /* if truncate to 0 length, simply don't add */
         SDP_TRACE_ERROR(
             "SDP_AddAttribute fail, length exceed maximum: ID %d: attr_len:%d ",
             attr_id, attr_len);