From baf48ff4bd2defb8c3c8d37962e25cbeebbac3e6 Mon Sep 17 00:00:00 2001
From: Chris Manton <cmanton@google.com>
Date: Wed, 29 Sep 2021 17:49:25 -0700
Subject: [PATCH 01/31] osi: Prevent memory allocations with MSB set

Limit allocations on 32bit to 2 GB
Limit allocations on 64bit to 8 Exabyte

Bug: 197868577
Tag: #refactor
Test: gd/cert/run
Ignore-AOSP-First: Security
Change-Id: I1c347084d7617b1e364a3241f1b37b398a2a6c6a
(cherry picked from commit e435404a7d2afa6b4cb9a59319667bf72af4df1f)
---
 osi/src/allocator.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/osi/src/allocator.cc b/osi/src/allocator.cc
index 1c0449e148..e2c356dd34 100644
--- a/osi/src/allocator.cc
+++ b/osi/src/allocator.cc
@@ -56,6 +56,7 @@ char* osi_strndup(const char* str, size_t len) {
 }
 
 void* osi_malloc(size_t size) {
+  CHECK(static_cast<ssize_t>(size) >= 0);
   size_t real_size = allocation_tracker_resize_for_canary(size);
   void* ptr = malloc(real_size);
   CHECK(ptr);
@@ -63,6 +64,7 @@ void* osi_malloc(size_t size) {
 }
 
 void* osi_calloc(size_t size) {
+  CHECK(static_cast<ssize_t>(size) >= 0);
   size_t real_size = allocation_tracker_resize_for_canary(size);
   void* ptr = calloc(1, real_size);
   CHECK(ptr);

From 0f30ea009c858fa3d8e4a6a12b98bfca6405878e Mon Sep 17 00:00:00 2001
From: Chris Manton <cmanton@google.com>
Date: Mon, 8 Nov 2021 16:45:42 -0800
Subject: [PATCH 02/31] security: Use-After-Free in btm_sec_[dis]connected

Bug: 201083442
Tag: #security
Test: gd/cert/run
Ignore-AOSP-First: Security

Change-Id: I69c362d1eb644a3b7fd967cd526a8a58c3b4d975
(cherry picked from commit c08175b5f15b161a6ba1444e1071e92b03552915)
Merged-In:I69c362d1eb644a3b7fd967cd526a8a58c3b4d975
---
 stack/btm/btm_sec.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/stack/btm/btm_sec.cc b/stack/btm/btm_sec.cc
index bdda174ac7..eecbed5ea8 100644
--- a/stack/btm/btm_sec.cc
+++ b/stack/btm/btm_sec.cc
@@ -3913,7 +3913,6 @@ static void btm_sec_connect_after_reject_timeout(UNUSED_ATTR void* data) {
  ******************************************************************************/
 void btm_sec_connected(const RawAddress& bda, uint16_t handle, uint8_t status,
                        uint8_t enc_mode) {
-  tBTM_SEC_DEV_REC* p_dev_rec = btm_find_dev(bda);
   uint8_t res;
   bool is_pairing_device = false;
   bool addr_matched;
@@ -3922,6 +3921,7 @@ void btm_sec_connected(const RawAddress& bda, uint16_t handle, uint8_t status,
 
   btm_acl_resubmit_page();
 
+  tBTM_SEC_DEV_REC* p_dev_rec = btm_find_dev(bda);
   if (p_dev_rec) {
     VLOG(2) << __func__ << ": Security Manager: in state: "
             << btm_pair_state_descr(btm_cb.pairing_state)
@@ -4258,7 +4258,6 @@ tBTM_STATUS btm_sec_disconnect(uint16_t handle, uint8_t reason) {
  *
  ******************************************************************************/
 void btm_sec_disconnected(uint16_t handle, uint8_t reason) {
-  tBTM_SEC_DEV_REC* p_dev_rec = btm_find_dev_by_handle(handle);
   uint8_t old_pairing_flags = btm_cb.pairing_flags;
   int result = HCI_ERR_AUTH_FAILURE;
   tBTM_SEC_CALLBACK* p_callback = NULL;
@@ -4269,6 +4268,7 @@ void btm_sec_disconnected(uint16_t handle, uint8_t reason) {
 
   btm_acl_resubmit_page();
 
+  tBTM_SEC_DEV_REC* p_dev_rec = btm_find_dev_by_handle(handle);
   if (!p_dev_rec) return;
 
   transport =

From 6a94760f38911b6bf41c11f71cdbf52e79240a04 Mon Sep 17 00:00:00 2001
From: Martin Brabham <optedoblivion@google.com>
Date: Fri, 29 Oct 2021 21:27:27 +0000
Subject: [PATCH 03/31] Reset the IRK after all devices are unpaired

Bug: 204355134
Bug: 195410559
Test: Check IRK, pair devices, unpair all devices, Check IRK
Tag: #security
Change-Id: I8e44f010a72dcdec595d81293a05f49ccc054065
Merged-In: I8e44f010a72dcdec595d81293a05f49ccc054065
(cherry picked from commit d6d753d733cdefad3c9edfda5cb2e857b0fdf5f4)
Merged-In:I8e44f010a72dcdec595d81293a05f49ccc054065
---
 bta/dm/bta_dm_act.cc | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/bta/dm/bta_dm_act.cc b/bta/dm/bta_dm_act.cc
index d1b260c192..3900594bf3 100644
--- a/bta/dm/bta_dm_act.cc
+++ b/bta/dm/bta_dm_act.cc
@@ -39,6 +39,7 @@
 #include "bta_dm_int.h"
 #include "bta_sys.h"
 #include "btif_storage.h"
+#include "btif_config.h"
 #include "btm_api.h"
 #include "btm_int.h"
 #include "btu.h"
@@ -48,6 +49,7 @@
 #include "osi/include/log.h"
 #include "osi/include/osi.h"
 #include "sdp_api.h"
+#include "stack/btm/btm_ble_int.h"
 #include "stack/gatt/connection_manager.h"
 #include "stack/include/gatt_api.h"
 #include "utl.h"
@@ -705,6 +707,12 @@ void bta_dm_remove_device(const RawAddress& bd_addr) {
   if (!other_address_connected && !other_address.IsEmpty()) {
     bta_dm_process_remove_device(other_address);
   }
+
+  /* Check the length of the paired devices, and if 0 then reset IRK */
+  if (btif_storage_get_num_bonded_devices() < 1) {
+    LOG(INFO) << "Last paired device removed, resetting IRK";
+    btm_ble_reset_id();
+  }
 }
 
 /*******************************************************************************

From 4e54538afaf942c12812f6ae588bc0aa418a5e13 Mon Sep 17 00:00:00 2001
From: Ted Wang <tedwang@google.com>
Date: Thu, 13 Jan 2022 15:00:32 +0800
Subject: [PATCH 04/31] Security fix OOB read due to invalid count in
 stack/avrc/avrc_pars_ct

Bug: 205837191
Tag: #security
Test: PoC test program
Ignore-AOSP-First: Security
Change-Id: I7b5bcb6551a8c0c015566327e13ba719271ce374
Merged-In: I7b5bcb6551a8c0c015566327e13ba719271ce374
(cherry picked from commit 697942b47f92b173e1cd87bb404e2386db958630)
Merged-In:I7b5bcb6551a8c0c015566327e13ba719271ce374
---
 stack/avrc/avrc_pars_ct.cc | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/stack/avrc/avrc_pars_ct.cc b/stack/avrc/avrc_pars_ct.cc
index 08ab66e729..8a4cf30347 100644
--- a/stack/avrc/avrc_pars_ct.cc
+++ b/stack/avrc/avrc_pars_ct.cc
@@ -581,6 +581,10 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(tAVRC_MSG_VENDOR* p_msg,
                        p_result->get_caps.capability_id,
                        p_result->get_caps.count);
       if (p_result->get_caps.capability_id == AVRC_CAP_COMPANY_ID) {
+        if (p_result->get_caps.count > AVRC_CAP_MAX_NUM_COMP_ID) {
+          android_errorWriteLog(0x534e4554, "205837191");
+          return AVRC_STS_INTERNAL_ERR;
+        }
         min_len += MIN(p_result->get_caps.count, AVRC_CAP_MAX_NUM_COMP_ID) * 3;
         if (len < min_len) goto length_error;
         for (int xx = 0; ((xx < p_result->get_caps.count) &&
@@ -590,6 +594,10 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(tAVRC_MSG_VENDOR* p_msg,
         }
       } else if (p_result->get_caps.capability_id ==
                  AVRC_CAP_EVENTS_SUPPORTED) {
+        if (p_result->get_caps.count > AVRC_CAP_MAX_NUM_EVT_ID) {
+          android_errorWriteLog(0x534e4554, "205837191");
+          return AVRC_STS_INTERNAL_ERR;
+        }
         min_len += MIN(p_result->get_caps.count, AVRC_CAP_MAX_NUM_EVT_ID);
         if (len < min_len) goto length_error;
         for (int xx = 0; ((xx < p_result->get_caps.count) &&

From 92cd3cf551da388f9d6c23c87c57b51c8d540f36 Mon Sep 17 00:00:00 2001
From: Chen Chen <cncn@google.com>
Date: Fri, 15 Apr 2022 14:24:48 -0700
Subject: [PATCH 05/31] Security: Fix out of bound write in HFP client

Bug: 224536184
Test: build
Tag: #security
Ignore-AOSP-First: Security bug
Change-Id: I9f0be0de6c4e1569095a43e92e9d8f9d73ca5fda
(cherry picked from commit ea2815973590018a6df5a3e88fa582eb4c8ff04e)
Merged-In: I9f0be0de6c4e1569095a43e92e9d8f9d73ca5fda
---
 bta/hf_client/bta_hf_client_at.cc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/bta/hf_client/bta_hf_client_at.cc b/bta/hf_client/bta_hf_client_at.cc
index 6e4fe26962..da40a297ee 100644
--- a/bta/hf_client/bta_hf_client_at.cc
+++ b/bta/hf_client/bta_hf_client_at.cc
@@ -332,6 +332,10 @@ static void bta_hf_client_handle_cind_list_item(tBTA_HF_CLIENT_CB* client_cb,
 
   APPL_TRACE_DEBUG("%s: %lu.%s <%lu:%lu>", __func__, index, name, min, max);
 
+  if (index >= BTA_HF_CLIENT_AT_INDICATOR_COUNT) {
+    return;
+  }
+
   /* look for a matching indicator on list of supported ones */
   for (i = 0; i < BTA_HF_CLIENT_AT_SUPPORTED_INDICATOR_COUNT; i++) {
     if (strcmp(name, BTA_HF_CLIENT_INDICATOR_SERVICE) == 0) {

From 2083fe49ad875748e0063d3b60dc9f66e47be751 Mon Sep 17 00:00:00 2001
From: William Escande <wescande@google.com>
Date: Mon, 2 May 2022 09:48:59 -0700
Subject: [PATCH 06/31] Check Avrcp packet vendor length before extracting
 length

Bug: 205571133
Test: build + ag/18105403 for sts test
Ignore-AOSP-First: Security vulnerability
Change-Id: Ic9fa9400ab15785cfdb251af66b1867daf09570e
(cherry picked from commit 003e42896493afb7a0cd7406720987725d4e9da3)
Merged-In: Ic9fa9400ab15785cfdb251af66b1867daf09570e
---
 stack/avrc/avrc_pars_tg.cc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/stack/avrc/avrc_pars_tg.cc b/stack/avrc/avrc_pars_tg.cc
index 190a88d75f..5bae32e52d 100644
--- a/stack/avrc/avrc_pars_tg.cc
+++ b/stack/avrc/avrc_pars_tg.cc
@@ -43,6 +43,12 @@ static tAVRC_STS avrc_ctrl_pars_vendor_cmd(tAVRC_MSG_VENDOR* p_msg,
                                            tAVRC_COMMAND* p_result) {
   tAVRC_STS status = AVRC_STS_NO_ERROR;
 
+  if (p_msg->vendor_len < 4) {  // 4 == pdu + reserved byte + len as uint16
+    AVRC_TRACE_WARNING("%s: message length %d too short: must be at least 4",
+                       __func__, p_msg->vendor_len);
+    android_errorWriteLog(0x534e4554, "205571133");
+    return AVRC_STS_INTERNAL_ERR;
+  }
   uint8_t* p = p_msg->p_vendor_data;
   p_result->pdu = *p++;
   AVRC_TRACE_DEBUG("%s pdu:0x%x", __func__, p_result->pdu);

From 55ed17a9d76c25db28cc66d06d37f220ecaedde1 Mon Sep 17 00:00:00 2001
From: Josh Wu <joshwu@google.com>
Date: Fri, 29 Apr 2022 00:02:23 -0700
Subject: [PATCH 07/31] Security: Fix out of bound read in AT_SKIP_REST

Bug: 220732646
Test: build
Tag: #security
Ignore-AOSP-First: Security bug
Change-Id: Ia49f26e4979f9e57c448190a52d0d01b70e342c4
(cherry picked from commit 863a0f417f6358892783860e08bf093d027764cf)
Merged-In: Ia49f26e4979f9e57c448190a52d0d01b70e342c4
---
 bta/hf_client/bta_hf_client_at.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/bta/hf_client/bta_hf_client_at.cc b/bta/hf_client/bta_hf_client_at.cc
index da40a297ee..725d6ed16e 100644
--- a/bta/hf_client/bta_hf_client_at.cc
+++ b/bta/hf_client/bta_hf_client_at.cc
@@ -797,9 +797,9 @@ void bta_hf_client_binp(tBTA_HF_CLIENT_CB* client_cb, char* number) {
   } while (0)
 
 /* skip rest of AT string up to <cr> */
-#define AT_SKIP_REST(buf)           \
-  do {                              \
-    while (*(buf) != '\r') (buf)++; \
+#define AT_SKIP_REST(buf)                             \
+  do {                                                \
+    while (*(buf) != '\r' && *(buf) != '\0') (buf)++; \
   } while (0)
 
 static char* bta_hf_client_parse_ok(tBTA_HF_CLIENT_CB* client_cb,

From 035f9b2f51d7c090baa9e194f9cb6b98e9bcb097 Mon Sep 17 00:00:00 2001
From: Roopa Sattiraju <sattiraju@google.com>
Date: Wed, 25 May 2022 21:55:24 +0000
Subject: [PATCH 08/31] Removing bonded device when auth fails due to missing
 keys

Bug: 231161832
Test: Test against trying to connect using the same address
Change-Id: I2a23440303758faf281989abdb2a614708f05d36
Merged-In: I2a23440303758faf281989abdb2a614708f05d36
(cherry picked from commit d9a9f9aaecd5bc46827b40db5a2e5745056440fd)
Merged-In: I2a23440303758faf281989abdb2a614708f05d36
---
 btif/src/btif_dm.cc | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/btif/src/btif_dm.cc b/btif/src/btif_dm.cc
index 467470db3b..3d556f9201 100644
--- a/btif/src/btif_dm.cc
+++ b/btif/src/btif_dm.cc
@@ -1185,16 +1185,13 @@ static void btif_dm_auth_cmpl_evt(tBTA_DM_AUTH_CMPL* p_auth_cmpl) {
         break;
 
       case HCI_ERR_PAIRING_NOT_ALLOWED:
-        is_bonded_device_removed =
-            (btif_storage_remove_bonded_device(&bd_addr) == BT_STATUS_SUCCESS);
         status = BT_STATUS_AUTH_REJECTED;
         break;
 
       /* map the auth failure codes, so we can retry pairing if necessary */
       case HCI_ERR_AUTH_FAILURE:
       case HCI_ERR_KEY_MISSING:
-        is_bonded_device_removed =
-            (btif_storage_remove_bonded_device(&bd_addr) == BT_STATUS_SUCCESS);
+        is_bonded_device_removed = false;
         [[fallthrough]];
       case HCI_ERR_HOST_REJECT_SECURITY:
       case HCI_ERR_ENCRY_MODE_NOT_ACCEPTABLE:
@@ -1225,8 +1222,6 @@ static void btif_dm_auth_cmpl_evt(tBTA_DM_AUTH_CMPL* p_auth_cmpl) {
       /* Remove Device as bonded in nvram as authentication failed */
       BTIF_TRACE_DEBUG("%s(): removing hid pointing device from nvram",
                        __func__);
-      is_bonded_device_removed =
-          (btif_storage_remove_bonded_device(&bd_addr) == BT_STATUS_SUCCESS);
     }
     // Report bond state change to java only if we are bonding to a device or
     // a device is removed from the pairing list.

From 954528b23e2814e21918fb6854767a47cf92e425 Mon Sep 17 00:00:00 2001
From: Ted Wang <tedwang@google.com>
Date: Fri, 1 Apr 2022 11:22:34 +0800
Subject: [PATCH 09/31] Fix potential interger overflow when parsing vendor
 response

Add check for str_len to prevent potential OOB read in vendor response.

Bug: 205570663
Tag: #security
Test: net_test_stack:StackAvrcpTest
Ignore-AOSP-First: Security
Change-Id: Iea2c3e17c2c8cc56468c4456822e1c4c5c15f5bc
Merged-In: Iea2c3e17c2c8cc56468c4456822e1c4c5c15f5bc
(cherry picked from commit 53aff7d1e018c5d5f4eb5d09eecfaad760e92ec4)
Merged-In: Iea2c3e17c2c8cc56468c4456822e1c4c5c15f5bc
---
 stack/avrc/avrc_pars_ct.cc     | 27 ++++++++++++++----
 stack/test/stack_avrcp_test.cc | 50 ++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 5 deletions(-)

diff --git a/stack/avrc/avrc_pars_ct.cc b/stack/avrc/avrc_pars_ct.cc
index 8a4cf30347..09828bffdc 100644
--- a/stack/avrc/avrc_pars_ct.cc
+++ b/stack/avrc/avrc_pars_ct.cc
@@ -237,7 +237,7 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
   }
   BE_STREAM_TO_UINT8(pdu, p);
   uint16_t pkt_len;
-  int min_len = 0;
+  uint16_t min_len = 0;
   /* read the entire packet len */
   BE_STREAM_TO_UINT16(pkt_len, p);
 
@@ -380,8 +380,14 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
               /* Parse the name now */
               BE_STREAM_TO_UINT16(attr_entry->name.charset_id, p);
               BE_STREAM_TO_UINT16(attr_entry->name.str_len, p);
+              if (static_cast<uint16_t>(min_len + attr_entry->name.str_len) <
+                  min_len) {
+                // Check for overflow
+                android_errorWriteLog(0x534e4554, "205570663");
+              }
+              if (pkt_len - min_len < attr_entry->name.str_len)
+                goto browse_length_error;
               min_len += attr_entry->name.str_len;
-              if (pkt_len < min_len) goto browse_length_error;
               attr_entry->name.p_str = (uint8_t*)osi_malloc(
                   attr_entry->name.str_len * sizeof(uint8_t));
               BE_STREAM_TO_ARRAY(p, attr_entry->name.p_str,
@@ -444,8 +450,14 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
         BE_STREAM_TO_UINT32(attr_entry->attr_id, p);
         BE_STREAM_TO_UINT16(attr_entry->name.charset_id, p);
         BE_STREAM_TO_UINT16(attr_entry->name.str_len, p);
+        if (static_cast<uint16_t>(min_len + attr_entry->name.str_len) <
+            min_len) {
+          // Check for overflow
+          android_errorWriteLog(0x534e4554, "205570663");
+        }
+        if (pkt_len - min_len < attr_entry->name.str_len)
+          goto browse_length_error;
         min_len += attr_entry->name.str_len;
-        if (pkt_len < min_len) goto browse_length_error;
         attr_entry->name.p_str =
             (uint8_t*)osi_malloc(attr_entry->name.str_len * sizeof(uint8_t));
         BE_STREAM_TO_ARRAY(p, attr_entry->name.p_str, attr_entry->name.str_len);
@@ -815,8 +827,12 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(tAVRC_MSG_VENDOR* p_msg,
           BE_STREAM_TO_UINT32(p_attrs[i].attr_id, p);
           BE_STREAM_TO_UINT16(p_attrs[i].name.charset_id, p);
           BE_STREAM_TO_UINT16(p_attrs[i].name.str_len, p);
-          min_len += p_attrs[i].name.str_len;
-          if (len < min_len) {
+          if (static_cast<uint16_t>(min_len + p_attrs[i].name.str_len) <
+              min_len) {
+            // Check for overflow
+            android_errorWriteLog(0x534e4554, "205570663");
+          }
+          if (len - min_len < p_attrs[i].name.str_len) {
             for (int j = 0; j < i; j++) {
               osi_free(p_attrs[j].name.p_str);
             }
@@ -824,6 +840,7 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(tAVRC_MSG_VENDOR* p_msg,
             p_result->get_attrs.num_attrs = 0;
             goto length_error;
           }
+          min_len += p_attrs[i].name.str_len;
           if (p_attrs[i].name.str_len > 0) {
             p_attrs[i].name.p_str =
                 (uint8_t*)osi_calloc(p_attrs[i].name.str_len);
diff --git a/stack/test/stack_avrcp_test.cc b/stack/test/stack_avrcp_test.cc
index 72ec45f290..e731e98b76 100644
--- a/stack/test/stack_avrcp_test.cc
+++ b/stack/test/stack_avrcp_test.cc
@@ -27,6 +27,56 @@ class StackAvrcpTest : public ::testing::Test {
   virtual ~StackAvrcpTest() = default;
 };
 
+TEST_F(StackAvrcpTest, test_avrcp_ctrl_parse_vendor_rsp) {
+  uint8_t scratch_buf[512]{};
+  uint16_t scratch_buf_len = 512;
+  tAVRC_MSG msg{};
+  tAVRC_RESPONSE result{};
+  uint8_t vendor_rsp_buf[512]{};
+
+  msg.hdr.opcode = AVRC_OP_VENDOR;
+  msg.hdr.ctype = AVRC_CMD_STATUS;
+
+  memset(vendor_rsp_buf, 0, sizeof(vendor_rsp_buf));
+  vendor_rsp_buf[0] = AVRC_PDU_GET_ELEMENT_ATTR;
+  uint8_t* p = &vendor_rsp_buf[2];
+  UINT16_TO_BE_STREAM(p, 0x0009);   // parameter length
+  UINT8_TO_STREAM(p, 0x01);         // number of attributes
+  UINT32_TO_STREAM(p, 0x00000000);  // attribute ID
+  UINT16_TO_STREAM(p, 0x0000);      // character set ID
+  UINT16_TO_STREAM(p, 0xffff);      // attribute value length
+  msg.vendor.p_vendor_data = vendor_rsp_buf;
+  msg.vendor.vendor_len = 13;
+  EXPECT_EQ(
+      AVRC_Ctrl_ParsResponse(&msg, &result, scratch_buf, &scratch_buf_len),
+      AVRC_STS_INTERNAL_ERR);
+}
+
+TEST_F(StackAvrcpTest, test_avrcp_parse_browse_rsp) {
+  uint8_t scratch_buf[512]{};
+  uint16_t scratch_buf_len = 512;
+  tAVRC_MSG msg{};
+  tAVRC_RESPONSE result{};
+  uint8_t browse_rsp_buf[512]{};
+
+  msg.hdr.opcode = AVRC_OP_BROWSE;
+
+  memset(browse_rsp_buf, 0, sizeof(browse_rsp_buf));
+  browse_rsp_buf[0] = AVRC_PDU_GET_ITEM_ATTRIBUTES;
+  uint8_t* p = &browse_rsp_buf[1];
+  UINT16_TO_BE_STREAM(p, 0x000a);   // parameter length;
+  UINT8_TO_STREAM(p, 0x04);         // status
+  UINT8_TO_STREAM(p, 0x01);         // number of attribute
+  UINT32_TO_STREAM(p, 0x00000000);  // attribute ID
+  UINT16_TO_STREAM(p, 0x0000);      // character set ID
+  UINT16_TO_STREAM(p, 0xffff);      // attribute value length
+  msg.browse.p_browse_data = browse_rsp_buf;
+  msg.browse.browse_len = 13;
+  EXPECT_EQ(
+      AVRC_Ctrl_ParsResponse(&msg, &result, scratch_buf, &scratch_buf_len),
+      AVRC_STS_BAD_CMD);
+}
+
 TEST_F(StackAvrcpTest, test_avrcp_parse_browse_cmd) {
   uint8_t scratch_buf[512]{};
   tAVRC_MSG msg{};

From 65929c1c62bceb2a23828dd4eb740a8d6c6ab241 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Fri, 12 Aug 2022 17:26:19 +0000
Subject: [PATCH 10/31] Add negative length check in process_service_search_rsp

Bug: 225876506
Test: run supplied POC (updated to Android T)
Tag: #security
Ignore-AOSP-First: Security
Change-Id: I0054806e47ed9d6eb8b034a41c8c872fee7f1eca
(cherry picked from commit 864460a945fe47b417def4017fb3d791e829753c)
Merged-In: I0054806e47ed9d6eb8b034a41c8c872fee7f1eca
---
 stack/sdp/sdp_discovery.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/stack/sdp/sdp_discovery.cc b/stack/sdp/sdp_discovery.cc
index bdc15bd1ce..f5938e4234 100644
--- a/stack/sdp/sdp_discovery.cc
+++ b/stack/sdp/sdp_discovery.cc
@@ -285,7 +285,7 @@ static void process_service_search_rsp(tCONN_CB* p_ccb, uint8_t* p_reply,
 
   orig = p_ccb->num_handles;
   p_ccb->num_handles += cur_handles;
-  if (p_ccb->num_handles == 0) {
+  if (p_ccb->num_handles == 0 || p_ccb->num_handles < orig) {
     SDP_TRACE_WARNING("SDP - Rcvd ServiceSearchRsp, no matches");
     sdp_disconnect(p_ccb, SDP_NO_RECS_MATCH);
     return;

From ddae17f77726e1f173bbb89bda07daacb8b30c33 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Sat, 13 Aug 2022 02:01:14 +0000
Subject: [PATCH 11/31] Add buffer in pin_reply in bluetooth.cc

Bug: 228602963
Test: make
Tag: #security
Ignore-AOSP-First: Security
Change-Id: I2a2c9a106a485c319841491f7acc2d667e4d0e75
(cherry picked from commit 5f1d6ac9a6adc287b8d10bb8241fe21615913c4b)
Merged-In: I2a2c9a106a485c319841491f7acc2d667e4d0e75
---
 btif/src/bluetooth.cc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/btif/src/bluetooth.cc b/btif/src/bluetooth.cc
index a00c0c662f..e4e50b6b21 100644
--- a/btif/src/bluetooth.cc
+++ b/btif/src/bluetooth.cc
@@ -309,10 +309,12 @@ static int get_connection_state(const RawAddress* bd_addr) {
 
 static int pin_reply(const RawAddress* bd_addr, uint8_t accept, uint8_t pin_len,
                      bt_pin_code_t* pin_code) {
+  bt_pin_code_t tmp_pin_code;
   /* sanity check */
   if (!interface_ready()) return BT_STATUS_NOT_READY;
 
-  return btif_dm_pin_reply(bd_addr, accept, pin_len, pin_code);
+  memcpy(&tmp_pin_code, pin_code, pin_len);
+  return btif_dm_pin_reply(bd_addr, accept, pin_len, &tmp_pin_code);
 }
 
 static int ssp_reply(const RawAddress* bd_addr, bt_ssp_variant_t variant,

From 87ab541200010c22334c62d6324847aab3c0a8aa Mon Sep 17 00:00:00 2001
From: Ted Wang <tedwang@google.com>
Date: Thu, 4 Aug 2022 09:41:24 +0800
Subject: [PATCH 12/31] Add length check when copy AVDTP packet

Bug: 232023771
Test: make
Tag: #security
Ignore-AOSP-First: Security
Change-Id: I68dd78c747eeafee5190dc56d7c71e9eeed08a5b
Merged-In: I68dd78c747eeafee5190dc56d7c71e9eeed08a5b
(cherry picked from commit 324c3065f863b8484847bbdfd91ef4709d407c8c)
Merged-In: I68dd78c747eeafee5190dc56d7c71e9eeed08a5b
---
 stack/avdt/avdt_msg.cc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/stack/avdt/avdt_msg.cc b/stack/avdt/avdt_msg.cc
index 33fbfa744c..406f7f7ba5 100644
--- a/stack/avdt/avdt_msg.cc
+++ b/stack/avdt/avdt_msg.cc
@@ -1251,6 +1251,10 @@ BT_HDR* avdt_msg_asmbl(AvdtpCcb* p_ccb, BT_HDR* p_buf) {
      * would have allocated smaller buffer.
      */
     p_ccb->p_rx_msg = (BT_HDR*)osi_malloc(BT_DEFAULT_BUFFER_SIZE);
+    if (sizeof(BT_HDR) + p_buf->offset + p_buf->len > BT_DEFAULT_BUFFER_SIZE) {
+      android_errorWriteLog(0x534e4554, "232023771");
+      return NULL;
+    }
     memcpy(p_ccb->p_rx_msg, p_buf, sizeof(BT_HDR) + p_buf->offset + p_buf->len);
 
     /* Free original buffer */

From ea9feccc192a198c1bdb8f46b9e6ac17e03efacf Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Thu, 25 Aug 2022 18:52:28 +0000
Subject: [PATCH 13/31] RESTRICT AUTOMERGE Added max buffer length check

Bug: 230867224
Test: Manual -- paired Bluetooth headset and played audio
Tags: #security
Ignore-AOSP-First: Security
Change-Id: I740038288143715a1c06db781efd674b269a7f3e
(cherry picked from commit f67ea88c64d62e81c9a804c67ff06c52a6920d39)
Merged-In: I740038288143715a1c06db781efd674b269a7f3e
---
 stack/avct/avct_lcb_act.cc | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/stack/avct/avct_lcb_act.cc b/stack/avct/avct_lcb_act.cc
index eec049df99..5ea8e6551c 100644
--- a/stack/avct/avct_lcb_act.cc
+++ b/stack/avct/avct_lcb_act.cc
@@ -30,6 +30,7 @@
 #include "bt_types.h"
 #include "bt_utils.h"
 #include "btm_api.h"
+#include "osi/include/log.h"
 #include "osi/include/osi.h"
 
 /* packet header length lookup table */
@@ -64,7 +65,12 @@ static BT_HDR* avct_lcb_msg_asmbl(tAVCT_LCB* p_lcb, BT_HDR* p_buf) {
   pkt_type = AVCT_PKT_TYPE(p);
 
   /* quick sanity check on length */
-  if (p_buf->len < avct_lcb_pkt_type_len[pkt_type]) {
+  if (p_buf->len < avct_lcb_pkt_type_len[pkt_type] ||
+      (sizeof(BT_HDR) + p_buf->offset + p_buf->len) > BT_DEFAULT_BUFFER_SIZE) {
+    if ((sizeof(BT_HDR) + p_buf->offset + p_buf->len) >
+        BT_DEFAULT_BUFFER_SIZE) {
+      android_errorWriteWithInfoLog(0x534e4554, "230867224", -1, NULL, 0);
+    }
     osi_free(p_buf);
     AVCT_TRACE_WARNING("Bad length during reassembly");
     p_ret = NULL;

From 490836a87b604f6833b566f0d9991ef299436196 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Thu, 25 Aug 2022 20:39:08 +0000
Subject: [PATCH 14/31] Add missing increment in bnep_api.cc

Bug: 228450451
Test: manual, pair BT and play audio
Tag: #security
Ignore-AOSP-First: Security
Change-Id: I681878508feae3d0526ed3e928af7a415e7d5c36
(cherry picked from commit 0fa54c7d8a2c061202e61d75b805661c1e89a76d)
Merged-In: I681878508feae3d0526ed3e928af7a415e7d5c36
---
 stack/bnep/bnep_api.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/stack/bnep/bnep_api.cc b/stack/bnep/bnep_api.cc
index 809d0b2376..9bfc6046a7 100644
--- a/stack/bnep/bnep_api.cc
+++ b/stack/bnep/bnep_api.cc
@@ -259,6 +259,7 @@ tBNEP_RESULT BNEP_ConnectResp(uint16_t handle, tBNEP_RESULT resp) {
     p = (uint8_t*)(p_bcb->p_pending_data + 1) + p_bcb->p_pending_data->offset;
     while (extension_present && p && rem_len) {
       ext_type = *p++;
+      rem_len--;
       extension_present = ext_type >> 7;
       ext_type &= 0x7F;
 

From f622b7bd9ad540f5303c05fa09cdb5b4fece5839 Mon Sep 17 00:00:00 2001
From: Keith Mok <keithmok@google.com>
Date: Tue, 16 Aug 2022 21:41:03 +0000
Subject: [PATCH 15/31] Add length check when copy AVDT and AVCT packet

Previous fix for AVDT causing memory leak.
And missing similar fix for AVCT packet.

Bug: 232023771
Test: make
Tag: #security
Ignore-AOSP-First: Security
Merged-In: Ifa8ed1cd9ea118acba78bdfdf6d5861fad254a90
Change-Id: Ifa8ed1cd9ea118acba78bdfdf6d5861fad254a90
(cherry picked from commit 240baf57ea9a112c153af0b53082c6951c636653)
Merged-In: Ifa8ed1cd9ea118acba78bdfdf6d5861fad254a90
---
 stack/avct/avct_lcb_act.cc | 8 +++++++-
 stack/avdt/avdt_msg.cc     | 6 ++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/stack/avct/avct_lcb_act.cc b/stack/avct/avct_lcb_act.cc
index 5ea8e6551c..c980f0a224 100644
--- a/stack/avct/avct_lcb_act.cc
+++ b/stack/avct/avct_lcb_act.cc
@@ -91,13 +91,19 @@ static BT_HDR* avct_lcb_msg_asmbl(tAVCT_LCB* p_lcb, BT_HDR* p_buf) {
     if (p_lcb->p_rx_msg != NULL)
       AVCT_TRACE_WARNING("Got start during reassembly");
 
-    osi_free(p_lcb->p_rx_msg);
+    osi_free_and_reset((void**)&p_lcb->p_rx_msg);
 
     /*
      * Allocate bigger buffer for reassembly. As lower layers are
      * not aware of possible packet size after reassembly, they
      * would have allocated smaller buffer.
      */
+    if (sizeof(BT_HDR) + p_buf->offset + p_buf->len > BT_DEFAULT_BUFFER_SIZE) {
+      android_errorWriteLog(0x534e4554, "232023771");
+      osi_free(p_buf);
+      p_ret = NULL;
+      return p_ret;
+    }
     p_lcb->p_rx_msg = (BT_HDR*)osi_malloc(BT_DEFAULT_BUFFER_SIZE);
     memcpy(p_lcb->p_rx_msg, p_buf, sizeof(BT_HDR) + p_buf->offset + p_buf->len);
 
diff --git a/stack/avdt/avdt_msg.cc b/stack/avdt/avdt_msg.cc
index 406f7f7ba5..bf83d191e3 100644
--- a/stack/avdt/avdt_msg.cc
+++ b/stack/avdt/avdt_msg.cc
@@ -1250,11 +1250,13 @@ BT_HDR* avdt_msg_asmbl(AvdtpCcb* p_ccb, BT_HDR* p_buf) {
      * not aware of possible packet size after reassembly, they
      * would have allocated smaller buffer.
      */
-    p_ccb->p_rx_msg = (BT_HDR*)osi_malloc(BT_DEFAULT_BUFFER_SIZE);
     if (sizeof(BT_HDR) + p_buf->offset + p_buf->len > BT_DEFAULT_BUFFER_SIZE) {
       android_errorWriteLog(0x534e4554, "232023771");
-      return NULL;
+      osi_free(p_buf);
+      p_ret = NULL;
+      return p_ret;
     }
+    p_ccb->p_rx_msg = (BT_HDR*)osi_malloc(BT_DEFAULT_BUFFER_SIZE);
     memcpy(p_ccb->p_rx_msg, p_buf, sizeof(BT_HDR) + p_buf->offset + p_buf->len);
 
     /* Free original buffer */

From b26bb42bdc454f0c9ceb2a4393b271af996eded9 Mon Sep 17 00:00:00 2001
From: Keith Mok <keithmok@google.com>
Date: Sun, 14 Aug 2022 23:08:20 +0000
Subject: [PATCH 16/31] Fix integer overflow when parsing avrc response

Convert min_len from 16 bits to 32 bits to avoid
length checking overflow.
Also, use calloc instead of malloc for list allocation
since caller need to clean up string memory in the list items

Bug: 242459126
Test: fuzz_avrc
Tag: #security
Ignore-AOSP-First: Security
Merged-In: I7250509f2b320774926a8b24fd28828c5217d8a4
Change-Id: I7250509f2b320774926a8b24fd28828c5217d8a4
(cherry picked from commit 18fd685cfcc2690a9748a29721a1c275ec18448b)
Merged-In: I7250509f2b320774926a8b24fd28828c5217d8a4
---
 stack/avdt/avdt_scb_act.cc |  2 +-
 stack/avrc/avrc_pars_ct.cc | 39 +++++++++++---------------------------
 stack/avrc/avrc_pars_tg.cc |  2 +-
 3 files changed, 13 insertions(+), 30 deletions(-)

diff --git a/stack/avdt/avdt_scb_act.cc b/stack/avdt/avdt_scb_act.cc
index 9ff9265099..31745bb2f9 100644
--- a/stack/avdt/avdt_scb_act.cc
+++ b/stack/avdt/avdt_scb_act.cc
@@ -308,7 +308,7 @@ uint8_t* avdt_scb_hdl_report(AvdtpScb* p_scb, uint8_t* p, uint16_t len) {
   uint8_t* p_start = p;
   uint32_t ssrc;
   uint8_t o_v, o_p, o_cc;
-  uint16_t min_len = 0;
+  uint32_t min_len = 0;
   AVDT_REPORT_TYPE pt;
   tAVDT_REPORT_DATA report;
 
diff --git a/stack/avrc/avrc_pars_ct.cc b/stack/avrc/avrc_pars_ct.cc
index 09828bffdc..816547274f 100644
--- a/stack/avrc/avrc_pars_ct.cc
+++ b/stack/avrc/avrc_pars_ct.cc
@@ -141,7 +141,7 @@ static tAVRC_STS avrc_pars_vendor_rsp(tAVRC_MSG_VENDOR* p_msg,
 
 tAVRC_STS avrc_parse_notification_rsp(uint8_t* p_stream, uint16_t len,
                                       tAVRC_REG_NOTIF_RSP* p_rsp) {
-  uint16_t min_len = 1;
+  uint32_t min_len = 1;
 
   if (len < min_len) goto length_error;
   BE_STREAM_TO_UINT8(p_rsp->event_id, p_stream);
@@ -237,7 +237,7 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
   }
   BE_STREAM_TO_UINT8(pdu, p);
   uint16_t pkt_len;
-  uint16_t min_len = 0;
+  uint32_t min_len = 0;
   /* read the entire packet len */
   BE_STREAM_TO_UINT16(pkt_len, p);
 
@@ -279,7 +279,7 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
           get_item_rsp->uid_counter, get_item_rsp->item_count);
 
       /* get each of the items */
-      get_item_rsp->p_item_list = (tAVRC_ITEM*)osi_malloc(
+      get_item_rsp->p_item_list = (tAVRC_ITEM*)osi_calloc(
           get_item_rsp->item_count * (sizeof(tAVRC_ITEM)));
       tAVRC_ITEM* curr_item = get_item_rsp->p_item_list;
       for (int i = 0; i < get_item_rsp->item_count; i++) {
@@ -369,7 +369,7 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
                              __func__, media->type, media->name.charset_id,
                              media->name.str_len, media->attr_count);
 
-            media->p_attr_list = (tAVRC_ATTR_ENTRY*)osi_malloc(
+            media->p_attr_list = (tAVRC_ATTR_ENTRY*)osi_calloc(
                 media->attr_count * sizeof(tAVRC_ATTR_ENTRY));
             for (int jk = 0; jk < media->attr_count; jk++) {
               tAVRC_ATTR_ENTRY* attr_entry = &(media->p_attr_list[jk]);
@@ -380,14 +380,8 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
               /* Parse the name now */
               BE_STREAM_TO_UINT16(attr_entry->name.charset_id, p);
               BE_STREAM_TO_UINT16(attr_entry->name.str_len, p);
-              if (static_cast<uint16_t>(min_len + attr_entry->name.str_len) <
-                  min_len) {
-                // Check for overflow
-                android_errorWriteLog(0x534e4554, "205570663");
-              }
-              if (pkt_len - min_len < attr_entry->name.str_len)
-                goto browse_length_error;
               min_len += attr_entry->name.str_len;
+              if (pkt_len < min_len) goto browse_length_error;
               attr_entry->name.p_str = (uint8_t*)osi_malloc(
                   attr_entry->name.str_len * sizeof(uint8_t));
               BE_STREAM_TO_ARRAY(p, attr_entry->name.p_str,
@@ -441,7 +435,7 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
       }
       BE_STREAM_TO_UINT8(get_attr_rsp->status, p)
       BE_STREAM_TO_UINT8(get_attr_rsp->num_attrs, p);
-      get_attr_rsp->p_attrs = (tAVRC_ATTR_ENTRY*)osi_malloc(
+      get_attr_rsp->p_attrs = (tAVRC_ATTR_ENTRY*)osi_calloc(
           get_attr_rsp->num_attrs * sizeof(tAVRC_ATTR_ENTRY));
       for (int i = 0; i < get_attr_rsp->num_attrs; i++) {
         tAVRC_ATTR_ENTRY* attr_entry = &(get_attr_rsp->p_attrs[i]);
@@ -450,14 +444,8 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
         BE_STREAM_TO_UINT32(attr_entry->attr_id, p);
         BE_STREAM_TO_UINT16(attr_entry->name.charset_id, p);
         BE_STREAM_TO_UINT16(attr_entry->name.str_len, p);
-        if (static_cast<uint16_t>(min_len + attr_entry->name.str_len) <
-            min_len) {
-          // Check for overflow
-          android_errorWriteLog(0x534e4554, "205570663");
-        }
-        if (pkt_len - min_len < attr_entry->name.str_len)
-          goto browse_length_error;
         min_len += attr_entry->name.str_len;
+        if (pkt_len < min_len) goto browse_length_error;
         attr_entry->name.p_str =
             (uint8_t*)osi_malloc(attr_entry->name.str_len * sizeof(uint8_t));
         BE_STREAM_TO_ARRAY(p, attr_entry->name.p_str, attr_entry->name.str_len);
@@ -493,7 +481,7 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg,
           __func__, set_br_pl_rsp->status, set_br_pl_rsp->num_items,
           set_br_pl_rsp->charset_id, set_br_pl_rsp->folder_depth);
 
-      set_br_pl_rsp->p_folders = (tAVRC_NAME*)osi_malloc(
+      set_br_pl_rsp->p_folders = (tAVRC_NAME*)osi_calloc(
           set_br_pl_rsp->folder_depth * sizeof(tAVRC_NAME));
 
       /* Read each of the folder in the depth */
@@ -553,7 +541,7 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(tAVRC_MSG_VENDOR* p_msg,
   p++; /* skip the reserved/packe_type byte */
 
   uint16_t len;
-  uint16_t min_len = 0;
+  uint32_t min_len = 0;
   BE_STREAM_TO_UINT16(len, p);
   AVRC_TRACE_DEBUG("%s ctype:0x%x pdu:0x%x, len:%d  vendor_len=0x%x", __func__,
                    p_msg->hdr.ctype, p_result->pdu, len, p_msg->vendor_len);
@@ -827,12 +815,8 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(tAVRC_MSG_VENDOR* p_msg,
           BE_STREAM_TO_UINT32(p_attrs[i].attr_id, p);
           BE_STREAM_TO_UINT16(p_attrs[i].name.charset_id, p);
           BE_STREAM_TO_UINT16(p_attrs[i].name.str_len, p);
-          if (static_cast<uint16_t>(min_len + p_attrs[i].name.str_len) <
-              min_len) {
-            // Check for overflow
-            android_errorWriteLog(0x534e4554, "205570663");
-          }
-          if (len - min_len < p_attrs[i].name.str_len) {
+          min_len += p_attrs[i].name.str_len;
+          if (len < min_len) {
             for (int j = 0; j < i; j++) {
               osi_free(p_attrs[j].name.p_str);
             }
@@ -840,7 +824,6 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(tAVRC_MSG_VENDOR* p_msg,
             p_result->get_attrs.num_attrs = 0;
             goto length_error;
           }
-          min_len += p_attrs[i].name.str_len;
           if (p_attrs[i].name.str_len > 0) {
             p_attrs[i].name.p_str =
                 (uint8_t*)osi_calloc(p_attrs[i].name.str_len);
diff --git a/stack/avrc/avrc_pars_tg.cc b/stack/avrc/avrc_pars_tg.cc
index 5bae32e52d..b2b6aa99cf 100644
--- a/stack/avrc/avrc_pars_tg.cc
+++ b/stack/avrc/avrc_pars_tg.cc
@@ -436,7 +436,7 @@ static tAVRC_STS avrc_pars_browsing_cmd(tAVRC_MSG_BROWSE* p_msg,
   uint8_t* p = p_msg->p_browse_data;
   int count;
 
-  uint16_t min_len = 3;
+  uint32_t min_len = 3;
   RETURN_STATUS_IF_FALSE(AVRC_STS_BAD_CMD, (p_msg->browse_len >= min_len),
                          "msg too short");
 

From 9b9591b3fb8a394bdb5a086d82993f9573a9b55b Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Fri, 2 Dec 2022 00:41:24 +0000
Subject: [PATCH 17/31] Report failure when not able to connect to AVRCP

A crash may occur when creating a bluetooth AVRCP connection to a
device.

The code fails to check a return value from an AVRCP function
being used to index into an array. The return value may exceed the
size of the array causing memory outside the bounds of the array to be
accessed leading to memory corruption and a crash.

The fix is to ensure the return value is within the bounds of the
array before accessing the array contents. If the return value is
not within the bounds of the array report it as a failure to the
bluetooth stack.

This change is relevant for android automotive because the IVI
(in-vehicle infotainment system) acts as the an AVRCP controller
which still executes this code.

Note: this is a backport of b/214569798, inducted as a non-security
issue.  Per b/226927612 it has been found to have security impact
and should be backported to earlier branches.

Bug: 226927612
Test: Manual - set return value to be out of bounds, verify no crash
Tag: #security
Ignore-AOSP-First: Security
Change-Id: I03f89f894c759b85e555a024435b625397ef7e5c
(cherry picked from commit 6a543761f2dc3db0ebf541285a0b3b2afc83a6a6)
Merged-In: I03f89f894c759b85e555a024435b625397ef7e5c
---
 bta/av/bta_av_act.cc | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/bta/av/bta_av_act.cc b/bta/av/bta_av_act.cc
index 827cdbb5be..533b15dfa3 100644
--- a/bta/av/bta_av_act.cc
+++ b/bta/av/bta_av_act.cc
@@ -1973,8 +1973,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__);
         }

From 3116e6741f51422f9725b2e0e00443d8d1439f41 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Tue, 27 Sep 2022 22:05:08 +0000
Subject: [PATCH 18/31] Add bounds check in avdt_scb_act.cc

Bug: 242535997
Test: BT unit tests, validated against researcher POC
Tag: #security
Ignore-AOSP-First: Security
Change-Id: I3b982e5d447cb98ad269b3da3d7d591819b2e4e4
(cherry picked from commit eca4a3cdb0da240496341f546a57397434ec85dd)
Merged-In: I3b982e5d447cb98ad269b3da3d7d591819b2e4e4
---
 stack/avdt/avdt_scb_act.cc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/stack/avdt/avdt_scb_act.cc b/stack/avdt/avdt_scb_act.cc
index 31745bb2f9..ce53c45eb6 100644
--- a/stack/avdt/avdt_scb_act.cc
+++ b/stack/avdt/avdt_scb_act.cc
@@ -977,6 +977,11 @@ void avdt_scb_hdl_write_req(AvdtpScb* p_scb, tAVDT_SCB_EVT* p_data) {
 
   /* Build a media packet, and add an RTP header if required. */
   if (add_rtp_header) {
+    if (p_data->apiwrite.p_buf->offset < AVDT_MEDIA_HDR_SIZE) {
+      android_errorWriteWithInfoLog(0x534e4554, "242535997", -1, NULL, 0);
+      return;
+    }
+
     ssrc = avdt_scb_gen_ssrc(p_scb);
 
     p_data->apiwrite.p_buf->len += AVDT_MEDIA_HDR_SIZE;

From de967e4b406dcce2c6e47f5ca64867c961797fa2 Mon Sep 17 00:00:00 2001
From: Hui Peng <phui@google.com>
Date: Wed, 28 Dec 2022 00:32:37 +0000
Subject: [PATCH 19/31] Fix an OOB Write bug in gatt_check_write_long_terminate

this is the backport of Ifffa2c7f679c4ef72dbdb6b1f3378ca506680084

Bug: 258652631
Test: manual
Tag: #security
Ignore-AOSP-First: security
Change-Id: Ic84122f07cbc198c676d366e39606621b7cb4e66
(cherry picked from commit 9b17660bfd6f0f41cb9400ce0236d76c83605e03)
Merged-In: Ic84122f07cbc198c676d366e39606621b7cb4e66
---
 stack/gatt/gatt_cl.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/stack/gatt/gatt_cl.cc b/stack/gatt/gatt_cl.cc
index 842cb6c3e7..ff1e5af55e 100644
--- a/stack/gatt/gatt_cl.cc
+++ b/stack/gatt/gatt_cl.cc
@@ -572,7 +572,8 @@ void gatt_process_prep_write_rsp(tGATT_TCB& tcb, tGATT_CLCB* p_clcb,
   LOG(ERROR) << 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;
@@ -581,7 +582,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);
 

From 428c325acdba5ba74b630ec7a92f016aa894b940 Mon Sep 17 00:00:00 2001
From: Hui Peng <phui@google.com>
Date: Mon, 2 Jan 2023 22:05:45 +0000
Subject: [PATCH 20/31] Fix an OOB access bug in
 A2DP_BuildMediaPayloadHeaderSbc

In  A2DP_BuildCodecHeaderSbc when p_buf->offset is 0, the
`-=` operation on it may result in integer underflow and
OOB write with the computed pointer passed to
A2DP_BuildMediaPayloadHeaderSbc.

This is a backport of I45320085b1e458d3b0e0d86162a35aaaae7b34cb
Test: atest net_test_stack_a2dp_codecs_native
Ignore-AOSP-First: security
Tag:#security

Bug: 186803518
Change-Id: I4ff1a1de71884b8de23008b2569fdea3650e85ec
(cherry picked from commit a710300216be4a86373a65c6a685aeef8509cfa7)
Merged-In: I4ff1a1de71884b8de23008b2569fdea3650e85ec
---
 stack/a2dp/a2dp_sbc.cc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/stack/a2dp/a2dp_sbc.cc b/stack/a2dp/a2dp_sbc.cc
index 78148a43eb..8a55cd7984 100644
--- a/stack/a2dp/a2dp_sbc.cc
+++ b/stack/a2dp/a2dp_sbc.cc
@@ -709,6 +709,11 @@ bool A2DP_BuildCodecHeaderSbc(UNUSED_ATTR const uint8_t* p_codec_info,
                               BT_HDR* p_buf, uint16_t frames_per_packet) {
   uint8_t* p;
 
+  // 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;
   p = (uint8_t*)(p_buf + 1) + p_buf->offset;
   p_buf->len += A2DP_SBC_MPL_HDR_LEN;

From 9c3016552da16ede297fbf3e1dd98a7fe72341ba Mon Sep 17 00:00:00 2001
From: Hui Peng <phui@google.com>
Date: Wed, 4 Jan 2023 22:45:13 +0000
Subject: [PATCH 21/31] Fix an OOB write in SDP_AddAttribute

When the `attr_pad` becomes full, it is possible
that un index of `-1` is computed write
a zero byte to `p_val`, rusulting OOB write.

```
  p_val[SDP_MAX_PAD_LEN - p_rec->free_pad_ptr - 1] = '\0';
```

This is a backport of I937d22a2df26fca1d7f06b10182c4e713ddfed1b

Bug: 261867748
Test: manual
Tag: #security
Ignore-AOSP-First: security
Change-Id: Ibdda754e628cfc9d1706c14db114919a15d8d6b1
(cherry picked from commit cc527a97f78a2999a0156a579e488afe9e3675b2)
Merged-In: Ibdda754e628cfc9d1706c14db114919a15d8d6b1
---
 stack/sdp/sdp_db.cc | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/stack/sdp/sdp_db.cc b/stack/sdp/sdp_db.cc
index 2cc04b039d..dd06230db3 100644
--- a/stack/sdp/sdp_db.cc
+++ b/stack/sdp/sdp_db.cc
@@ -357,6 +357,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) ||
@@ -393,6 +398,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 */
@@ -432,15 +444,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);

From ddaec306d60af00c0a748a7c107444b6fafd7935 Mon Sep 17 00:00:00 2001
From: Hui Peng <phui@google.com>
Date: Thu, 8 Dec 2022 01:08:11 +0000
Subject: [PATCH 22/31] Fix OOB access in avdt_scb_hdl_pkt_no_frag

This is a back port of the following 2 CLs:
- Id13b1ebde8f603123c8b7a49922b2f1378ab788f
- If0c7b25f2e6cb4531bbb6254e176e8ad1b5c5fb4

Regression test: I9c87e30ed58e7ad6a34ab7c96b0a8fb06324ad54

Bug: 142546355 258057241
Test: atest net_test_stack_avdtp
Ignore-AOSP-First: security
Change-Id: Ie1707385d6452ece47915c153f4faaa1c8a287c9
(cherry picked from commit b0b968e8c6214e20a5dc3617d66567225df0884f)
Merged-In: Ie1707385d6452ece47915c153f4faaa1c8a287c9
---
 stack/avdt/avdt_scb_act.cc | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/stack/avdt/avdt_scb_act.cc b/stack/avdt/avdt_scb_act.cc
index ce53c45eb6..f2de4ba354 100644
--- a/stack/avdt/avdt_scb_act.cc
+++ b/stack/avdt/avdt_scb_act.cc
@@ -255,19 +255,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);
   }

From 928fccb9c4f6800a3ab4e9c28032a61995a800f6 Mon Sep 17 00:00:00 2001
From: Hui Peng <phui@google.com>
Date: Fri, 20 Jan 2023 19:39:30 +0000
Subject: [PATCH 23/31] Fix an OOB bug in register_notification_rsp

This is a backport of I901d973a736678d7f3cc816ddf0cbbcbbd1fe93f
to rvc-dev.

Bug: 245916076
Test: manual
Ignore-AOSP-First: security
Change-Id: I37a9f45e707702b2ec52b5a2d572f177f2911765
(cherry picked from commit 901e34203c6280d414cbfa3978de04fd6515ffdf)
Merged-In: I37a9f45e707702b2ec52b5a2d572f177f2911765
---
 btif/src/btif_rc.cc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/btif/src/btif_rc.cc b/btif/src/btif_rc.cc
index 9fc221e03f..b93a6bfd50 100755
--- a/btif/src/btif_rc.cc
+++ b/btif/src/btif_rc.cc
@@ -1968,6 +1968,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;

From 030c8ac6ea33ad29a85e5a8e9436832e065adf04 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Tue, 11 Oct 2022 21:23:22 +0000
Subject: [PATCH 24/31] Prevent use-after-free of HID reports

BTA sends the the HID report pointer to BTIF and deallocates it immediately.
This is now prevented by providing a deep copy callback function for HID
reports when tranferring context from BTA to BTIF.

This is a backport of change Icef7a7ed1185b4283ee4fe4f812ca154d8f1b825,
already merged on T for b/227620181.

Bug: 228837201
Test: Validated against researcher POC, ran BT unit tests, played audio
manually.
Tag: #security
Ignore-AOSP-First: Security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:874c495c886cd8722625756dc5fd0634b16b4f42)
Merged-In: Ib837f395883de2369207f1b3b974d6bff02dcb19
Change-Id: Ib837f395883de2369207f1b3b974d6bff02dcb19
---
 btif/src/btif_hh.cc | 50 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/btif/src/btif_hh.cc b/btif/src/btif_hh.cc
index 5380fa5662..97479e040b 100644
--- a/btif/src/btif_hh.cc
+++ b/btif/src/btif_hh.cc
@@ -1093,6 +1093,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
@@ -1106,6 +1138,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);
@@ -1117,11 +1150,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);
@@ -1130,7 +1170,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);

From ef364c635e33746be609f4d9120f9712dae04a89 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Tue, 21 Mar 2023 22:35:35 +0000
Subject: [PATCH 25/31] Revert "Revert "[RESTRICT AUTOMERGE] Validate buffer
 length in sdpu_build_uuid_seq""

This reverts commit 487a1079078f3717fdc4665c19a45eca5b3ec5e6.

Reason for revert: Reinstate original change for QPR
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:a681067af2ea4565543238db3025d749923f63ec)
Merged-In: If0528519a29dc73ff99163098da2a05592ab15d8
Change-Id: If0528519a29dc73ff99163098da2a05592ab15d8
---
 stack/sdp/sdp_discovery.cc | 65 ++++++++++++++++++++++++++++++++++----
 1 file changed, 58 insertions(+), 7 deletions(-)

diff --git a/stack/sdp/sdp_discovery.cc b/stack/sdp/sdp_discovery.cc
index f5938e4234..553e068f58 100644
--- a/stack/sdp/sdp_discovery.cc
+++ b/stack/sdp/sdp_discovery.cc
@@ -70,10 +70,15 @@ static uint8_t* add_attr(uint8_t* p, uint8_t* p_end, tSDP_DISCOVERY_DB* p_db,
  *
  ******************************************************************************/
 static uint8_t* sdpu_build_uuid_seq(uint8_t* p_out, uint16_t num_uuids,
-                                    Uuid* p_uuid_list) {
+                                    Uuid* p_uuid_list, uint16_t& bytes_left) {
   uint16_t xx;
   uint8_t* p_len;
 
+  if (bytes_left < 2) {
+    DCHECK(0) << "SDP: No space for data element header";
+    return (p_out);
+  }
+
   /* First thing is the data element header */
   UINT8_TO_BE_STREAM(p_out, (DATA_ELE_SEQ_DESC_TYPE << 3) | SIZE_IN_NEXT_BYTE);
 
@@ -81,9 +86,20 @@ static uint8_t* sdpu_build_uuid_seq(uint8_t* p_out, uint16_t num_uuids,
   p_len = p_out;
   p_out += 1;
 
+  /* Account for data element header and length */
+  bytes_left -= 2;
+
   /* Now, loop through and put in all the UUID(s) */
   for (xx = 0; xx < num_uuids; xx++, p_uuid_list++) {
     int len = p_uuid_list->GetShortestRepresentationSize();
+
+    if (len + 1 > bytes_left) {
+      DCHECK(0) << "SDP: Too many UUIDs for internal buffer";
+      break;
+    } else {
+      bytes_left -= (len + 1);
+    }
+
     if (len == Uuid::kNumBytes16) {
       UINT8_TO_BE_STREAM(p_out, (UUID_DESC_TYPE << 3) | SIZE_TWO_BYTES);
       UINT16_TO_BE_STREAM(p_out, p_uuid_list->As16Bit());
@@ -120,6 +136,7 @@ static void sdp_snd_service_search_req(tCONN_CB* p_ccb, uint8_t cont_len,
   uint8_t *p, *p_start, *p_param_len;
   BT_HDR* p_cmd = (BT_HDR*)osi_malloc(SDP_DATA_BUF_SIZE);
   uint16_t param_len;
+  uint16_t bytes_left = SDP_DATA_BUF_SIZE;
 
   /* Prepare the buffer for sending the packet to L2CAP */
   p_cmd->offset = L2CAP_MIN_OFFSET;
@@ -134,13 +151,30 @@ static void sdp_snd_service_search_req(tCONN_CB* p_ccb, uint8_t cont_len,
   p_param_len = p;
   p += 2;
 
-/* Build the UID sequence. */
+  /* Account for header size, max service record count and
+   * continuation state */
+  const uint16_t base_bytes = (sizeof(BT_HDR) + L2CAP_MIN_OFFSET +
+                               3u + /* service search request header */
+                               2u + /* param len */
+                               3u + ((p_cont) ? cont_len : 0));
+
+  if (base_bytes > bytes_left) {
+    DCHECK(0) << "SDP: Overran SDP data buffer";
+    osi_free(p_cmd);
+    return;
+  }
+
+  bytes_left -= base_bytes;
+
+  /* Build the UID sequence. */
 #if (SDP_BROWSE_PLUS == TRUE)
   p = sdpu_build_uuid_seq(p, 1,
-                          &p_ccb->p_db->uuid_filters[p_ccb->cur_uuid_idx]);
+                          &p_ccb->p_db->uuid_filters[p_ccb->cur_uuid_idx],
+                          bytes_left);
 #else
+  /* Build the UID sequence. */
   p = sdpu_build_uuid_seq(p, p_ccb->p_db->num_uuid_filters,
-                          p_ccb->p_db->uuid_filters);
+                          p_ccb->p_db->uuid_filters, bytes_left);
 #endif
 
   /* Set max service record count */
@@ -575,6 +609,7 @@ static void process_service_search_attr_rsp(tCONN_CB* p_ccb, uint8_t* p_reply,
   if ((cont_request_needed) || (!p_reply)) {
     BT_HDR* p_msg = (BT_HDR*)osi_malloc(SDP_DATA_BUF_SIZE);
     uint8_t* p;
+    uint16_t bytes_left = SDP_DATA_BUF_SIZE;
 
     p_msg->offset = L2CAP_MIN_OFFSET;
     p = p_start = (uint8_t*)(p_msg + 1) + L2CAP_MIN_OFFSET;
@@ -588,13 +623,29 @@ static void process_service_search_attr_rsp(tCONN_CB* p_ccb, uint8_t* p_reply,
     p_param_len = p;
     p += 2;
 
-/* Build the UID sequence. */
+    /* Account for header size, max service record count and
+     * continuation state */
+    const uint16_t base_bytes = (sizeof(BT_HDR) + L2CAP_MIN_OFFSET +
+                                 3u + /* service search request header */
+                                 2u + /* param len */
+                                 3u + /* max service record count */
+                                 ((p_reply) ? (*p_reply) : 0));
+
+    if (base_bytes > bytes_left) {
+      sdp_disconnect(p_ccb, SDP_INVALID_CONT_STATE);
+      return;
+    }
+
+    bytes_left -= base_bytes;
+
+    /* Build the UID sequence. */
 #if (SDP_BROWSE_PLUS == TRUE)
     p = sdpu_build_uuid_seq(p, 1,
-                            &p_ccb->p_db->uuid_filters[p_ccb->cur_uuid_idx]);
+                            &p_ccb->p_db->uuid_filters[p_ccb->cur_uuid_idx],
+                            bytes_left);
 #else
     p = sdpu_build_uuid_seq(p, p_ccb->p_db->num_uuid_filters,
-                            p_ccb->p_db->uuid_filters);
+                            p_ccb->p_db->uuid_filters, bytes_left);
 #endif
 
     /* Max attribute byte count */

From 88d9cd7df2bc1ce292bf596971735579753956b8 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Tue, 21 Mar 2023 22:39:16 +0000
Subject: [PATCH 26/31] Revert "Revert "Fix wrong BR/EDR link key downgrades
 (P_256->P_192)""

This reverts commit d733c86cbc06ce0ec72216b9d41e172d1939c46f.

Function btm_sec_encrypt_change() is called at most places
with argument "encr_enable" treated as bool and not as per
(tHCI_ENCRYPT_MODE = 0/1/2) expected by the function. The
function has special handling for "encr_enable=1" to downgrade
the link key type for BR/EDR case. This gets executed even
when the caller/context did not mean/expect so. It appears
this handling in btm_sec_encrypt_change() is not necessary and
is removed by this commit to prevent accidental execution of it.

Test: Verified re-pairing with an iPhone works fine now

Issue Reproduction Steps:
1. Enable Bluetooth Hotspot on Android device (DUT).
2. Pair and connect an iPhone to DUT.
3. Forget this pairing on DUT.
4. On iPhone settings, click on old DUT's paired entry to connect.
5. iPhone notifies to click 'Forget Device' and try fresh pairing.
6. On iPhone, after doing 'Forget Device', discover DUT again.
7. Attempt pairing to DUT by clicking on discovered DUT entry.
   Pairing will be unsuccessful.

Issue Cause:
During re-pairing, DUT is seen to downgrade
BR/EDR link key unexpectedly from link key type 0x8
(BTM_LKEY_TYPE_AUTH_COMB_P_256) to 0x5 (BTM_LKEY_TYPE_AUTH_COMB).

Log snippet (re-pairing time):
btm_sec_link_key_notification set new_encr_key_256 to 1
btif_dm_auth_cmpl_evt: Storing link key. key_type=0x8, bond_type=1
btm_sec_encrypt_change new_encr_key_256 is 1
--On DUT, HCI_Encryption_Key_Refresh_Complete event noticed---
btm_sec_encrypt_change new_encr_key_256 is 0
updated link key type to 5
btif_dm_auth_cmpl_evt: Storing link key. key_type=0x5, bond_type=1

This is a backport of the following patch: aosp/1890096

Bug: 258834033

Reason for revert: Reinstate original change for QPR
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:56891eedc68c86b40977191dad28d65ebf86a94f)
Merged-In: Iba0c220b82bcf6b15368762b7052a3987ccbc0c6
Change-Id: Iba0c220b82bcf6b15368762b7052a3987ccbc0c6
---
 stack/btm/btm_sec.cc | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/stack/btm/btm_sec.cc b/stack/btm/btm_sec.cc
index eecbed5ea8..ec120f68b3 100644
--- a/stack/btm/btm_sec.cc
+++ b/stack/btm/btm_sec.cc
@@ -3826,22 +3826,6 @@ void btm_sec_encrypt_change(uint16_t handle, uint8_t status,
           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);
-      }
     }
   }
 

From e463af8dfcea59f1a7c59ca604081034dfe27945 Mon Sep 17 00:00:00 2001
From: tyiu <tyiu@google.com>
Date: Tue, 28 Mar 2023 18:40:51 +0000
Subject: [PATCH 27/31] Fix gatt_end_operation buffer overflow

Added boundary check for gatt_end_operation to prevent writing out of
boundary.

Since response of the GATT server is handled in
gatt_client_handle_server_rsp() and gatt_process_read_rsp(), the maximum
lenth that can be passed into the handlers is bounded by
GATT_MAX_MTU_SIZE, which is set to 517, which is greater than
GATT_MAX_ATTR_LEN which is set to 512. The fact that there is no spec
that gaurentees MTU response to be less than or equal to 512 bytes can
cause a buffer overflow when performing memcpy without length check.

Bug: 261068592
Test: No test since not affecting behavior
Tag: #security
Ignore-AOSP-First: security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:dd7298e982e4bbf0138a490562679c9a4a755200)
Merged-In: I49e2797cd9300ee4cd69f2c7fa5f0073db78b873
Change-Id: I49e2797cd9300ee4cd69f2c7fa5f0073db78b873
---
 stack/gatt/gatt_utils.cc | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/stack/gatt/gatt_utils.cc b/stack/gatt/gatt_utils.cc
index 2bd4240001..013011778b 100644
--- a/stack/gatt/gatt_utils.cc
+++ b/stack/gatt/gatt_utils.cc
@@ -1198,6 +1198,13 @@ void gatt_end_operation(tGATT_CLCB* p_clcb, tGATT_STATUS status, void* p_data) {
       cb_data.att_value.handle = p_clcb->s_handle;
       cb_data.att_value.len = p_clcb->counter;
 
+      if (cb_data.att_value.len > GATT_MAX_ATTR_LEN) {
+        LOG(WARNING) << __func__
+                     << StringPrintf(" Large cb_data.att_value, size=%d",
+                                     cb_data.att_value.len);
+        cb_data.att_value.len = GATT_MAX_ATTR_LEN;
+      }
+
       if (p_data && p_clcb->counter)
         memcpy(cb_data.att_value.value, p_data, cb_data.att_value.len);
     }

From 623245102e876b96161594a7110cc0065d8f9cd2 Mon Sep 17 00:00:00 2001
From: Hui Peng <phui@google.com>
Date: Tue, 16 May 2023 21:24:07 +0000
Subject: [PATCH 28/31] Fix an integer overflow bug in avdt_msg_asmbl

This is a backport of
Iaa4d603921fc4ffb8cfb5783f99ec0963affd6a2
to rvc-dev

Bug: 280633699
Test: manual
Ignore-AOSP-First: security
Tag: #security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:26347d4bdba646bbba4d27337d2888a04de42639)
Merged-In: Iaa4d603921fc4ffb8cfb5783f99ec0963affd6a2
Change-Id: Iaa4d603921fc4ffb8cfb5783f99ec0963affd6a2
---
 stack/avdt/avdt_msg.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/stack/avdt/avdt_msg.cc b/stack/avdt/avdt_msg.cc
index bf83d191e3..3f8713c0b5 100644
--- a/stack/avdt/avdt_msg.cc
+++ b/stack/avdt/avdt_msg.cc
@@ -1289,14 +1289,14 @@ BT_HDR* avdt_msg_asmbl(AvdtpCcb* p_ccb, BT_HDR* p_buf) {
        * NOTE: The buffer is allocated above at the beginning of the
        * reassembly, and is always of size BT_DEFAULT_BUFFER_SIZE.
        */
-      uint16_t buf_len = BT_DEFAULT_BUFFER_SIZE - sizeof(BT_HDR);
+      size_t buf_len = BT_DEFAULT_BUFFER_SIZE - sizeof(BT_HDR);
 
       /* adjust offset and len of fragment for header byte */
       p_buf->offset += AVDT_LEN_TYPE_CONT;
       p_buf->len -= AVDT_LEN_TYPE_CONT;
 
       /* verify length */
-      if ((p_ccb->p_rx_msg->offset + p_buf->len) > buf_len) {
+      if (((size_t) p_ccb->p_rx_msg->offset + (size_t) p_buf->len) > buf_len) {
         /* won't fit; free everything */
         AVDT_TRACE_WARNING("%s: Fragmented message too big!", __func__);
         osi_free_and_reset((void**)&p_ccb->p_rx_msg);

From bdf9d250370e2ea141836248920f0ca9bff4f10b Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Fri, 19 May 2023 19:17:16 +0000
Subject: [PATCH 29/31] Fix integer overflow in build_read_multi_rsp

Local variables tracking structure size in build_read_multi_rsp are of
uint16 type but accept a full uint16 range from function arguments while
appending a fixed-length offset.  This can lead to an integer overflow
and unexpected behavior.

Change the locals to size_t, and add a check during reasssignment.

Bug: 273966636
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:53f64274cbf2268ad6db5af9c61ceead9ef64fb0)
Merged-In: Iff252f0dd06aac9776e8548631e0b700b3ed85b9
Change-Id: Iff252f0dd06aac9776e8548631e0b700b3ed85b9
---
 stack/gatt/gatt_sr.cc | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/stack/gatt/gatt_sr.cc b/stack/gatt/gatt_sr.cc
index 4b4b5da6fb..252732c739 100644
--- a/stack/gatt/gatt_sr.cc
+++ b/stack/gatt/gatt_sr.cc
@@ -114,7 +114,8 @@ void gatt_dequeue_sr_cmd(tGATT_TCB& tcb) {
  ******************************************************************************/
 static bool process_read_multi_rsp(tGATT_SR_CMD* p_cmd, tGATT_STATUS status,
                                    tGATTS_RSP* p_msg, uint16_t mtu) {
-  uint16_t ii, total_len, len;
+  uint16_t ii;
+  size_t total_len, len;
   uint8_t* p;
   bool is_overflow = false;
 
@@ -169,16 +170,22 @@ static bool process_read_multi_rsp(tGATT_SR_CMD* p_cmd, tGATT_STATUS status,
             len = p_rsp->attr_value.len - (total_len - mtu);
             is_overflow = true;
             VLOG(1) << StringPrintf(
-                "multi read overflow available len=%d val_len=%d", len,
+                "multi read overflow available len=%zu val_len=%d", len,
                 p_rsp->attr_value.len);
           } else {
             len = p_rsp->attr_value.len;
           }
 
           if (p_rsp->attr_value.handle == p_cmd->multi_req.handles[ii]) {
-            memcpy(p, p_rsp->attr_value.value, len);
-            if (!is_overflow) p += len;
-            p_buf->len += len;
+            // check for possible integer overflow
+            if (p_buf->len + len <= UINT16_MAX) {
+              memcpy(p, p_rsp->attr_value.value, len);
+              if (!is_overflow) p += len;
+              p_buf->len += len;
+            } else {
+              p_cmd->status = GATT_NOT_FOUND;
+              break;
+            }
           } else {
             p_cmd->status = GATT_NOT_FOUND;
             break;

From 71c7ac62d89daecadbeb588cc89eff17c981c310 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Thu, 27 Apr 2023 20:43:58 +0000
Subject: [PATCH 30/31] Fix potential abort in btu_av_act.cc

Partner analysis shows that bta_av_rc_msg does not respect handling
established for a null browse packet, instead dispatching the null
pointer to bta_av_rc_free_browse_msg.  Strictly speaking this does
not cause a UAF, as osi_free_and_reset will find the null and abort,
but it will lead to improper program termination.

Handle the case instead.

Bug: 269253349
Test: atest bluetooth_test_gd_unit
Tag: #security
Ignore-AOSP-First: Security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:91f6d6215c101acc99a7397c5fb5a12fe6d7b8e9)
Merged-In: I4df7045798b663fbefd7434288dc9383216171a7
Change-Id: I4df7045798b663fbefd7434288dc9383216171a7
---
 bta/av/bta_av_act.cc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/bta/av/bta_av_act.cc b/bta/av/bta_av_act.cc
index 533b15dfa3..20696bd90d 100644
--- a/bta/av/bta_av_act.cc
+++ b/bta/av/bta_av_act.cc
@@ -1008,7 +1008,10 @@ void bta_av_rc_msg(tBTA_AV_CB* p_cb, tBTA_AV_DATA* p_data) {
     av.remote_cmd.rc_handle = p_data->rc_msg.handle;
     (*p_cb->p_cback)(evt, &av);
     /* If browsing message, then free the browse message buffer */
-    bta_av_rc_free_browse_msg(p_cb, p_data);
+    if (p_data->rc_msg.opcode == AVRC_OP_BROWSE &&
+        p_data->rc_msg.msg.browse.p_browse_pkt != NULL) {
+      bta_av_rc_free_browse_msg(p_cb, p_data);
+    }
   }
 }
 

From d9606b98e22a0ecb6489eb71b332d090f797b99d Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Thu, 1 Jun 2023 23:57:58 +0000
Subject: [PATCH 31/31] Fix UAF in gatt_cl.cc

gatt_cl.cc accesses a header field after the buffer holding it may have
been freed.

Track the relevant state as a local variable instead.

Bug: 274617156
Test: atest: bluetooth, validated against fuzzer
Tag: #security
Ignore-AOSP-First: Security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:d7a7f7f3311202065de4b2c17b49994053dd1244)
Merged-In: I085ecfa1a9ba098ecbfecbd3cb3e263ae13f9724
Change-Id: I085ecfa1a9ba098ecbfecbd3cb3e263ae13f9724
---
 stack/gatt/gatt_cl.cc | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/stack/gatt/gatt_cl.cc b/stack/gatt/gatt_cl.cc
index ff1e5af55e..14732cdb04 100644
--- a/stack/gatt/gatt_cl.cc
+++ b/stack/gatt/gatt_cl.cc
@@ -586,12 +586,17 @@ void gatt_process_prep_write_rsp(tGATT_TCB& tcb, tGATT_CLCB* p_clcb,
 
   memcpy(value.value, p, value.len);
 
+  bool subtype_is_write_prepare = (p_clcb->op_subtype == GATT_WRITE_PREPARE);
+
   if (!gatt_check_write_long_terminate(tcb, p_clcb, &value)) {
     gatt_send_prepare_write(tcb, p_clcb);
     return;
   }
 
-  if (p_clcb->op_subtype == GATT_WRITE_PREPARE) {
+  // We now know that we have not terminated, or else we would have returned
+  // early.  We free the buffer only if the subtype is not equal to
+  // GATT_WRITE_PREPARE, so checking here is adequate to prevent UAF.
+  if (subtype_is_write_prepare) {
     /* application should verify handle offset
        and value are matched or not */
     gatt_end_operation(p_clcb, p_clcb->status, &value);