Merge "AVDTP: Fix a potential OOB read at the reporting handler" into sc-v2-dev

diff --git a/stack/avdt/avdt_scb_act.cc b/stack/avdt/avdt_scb_act.cc
index 8129344..fdf5570 100644 (file)
--- a/stack/avdt/avdt_scb_act.cc
+++ b/stack/avdt/avdt_scb_act.cc
@@ -387,7 +387,8 @@ uint8_t* avdt_scb_hdl_report(AvdtpScb* p_scb, uint8_t* p, uint16_t len) {
             goto avdt_scb_hdl_report_exit;
           }
           BE_STREAM_TO_UINT8(name_length, p);
-          if (name_length > len - 2 || name_length > AVDT_MAX_CNAME_SIZE) {
+          if (name_length > len - min_len ||
+              name_length > AVDT_MAX_CNAME_SIZE) {
             result = AVDT_BAD_PARAMS;
           } else {
             BE_STREAM_TO_ARRAY(p, &(report.cname[0]), name_length);
@@ -400,8 +401,8 @@ uint8_t* avdt_scb_hdl_report(AvdtpScb* p_scb, uint8_t* p, uint16_t len) {
                 __func__, len, min_len);
             goto avdt_scb_hdl_report_exit;
           }
-          AVDT_TRACE_WARNING(" - SDES SSRC=0x%08x sc=%d %d len=%d %s", ssrc,
-                             o_cc, *p, *(p + 1), p + 2);
+          AVDT_TRACE_WARNING("%s: SDES SSRC=0x%08x sc=%d %d len=%d", __func__,
+                             ssrc, o_cc, sdes_type, *p);
           result = AVDT_BUSY;
         }
         break;

diff --git a/stack/test/stack_avdtp_test.cc b/stack/test/stack_avdtp_test.cc
index e6621be..df889c0 100644 (file)
--- a/stack/test/stack_avdtp_test.cc
+++ b/stack/test/stack_avdtp_test.cc
@@ -17,6 +17,7 @@
 //#include <dlfcn.h>
 #include <gtest/gtest.h>
 
+#include "osi/include/allocator.h"
 #include "stack/include/avdt_api.h"
 #include "stack/avdt/avdt_int.h"
 #include "stack/test/common/mock_stack_avdt_msg.h"
@@ -39,61 +40,73 @@ class StackAvdtpTest : public ::testing::Test {
 
   virtual ~StackAvdtpTest() = default;
  protected:
-  static AvdtpRcb _reg;
-  static uint8_t _expected_stream_event;
-
-  uint8_t scb_handle_;
+  static AvdtpRcb reg_ctrl_block_;
+  static uint8_t callback_event_;
+  static uint8_t scb_handle_;
 
  protected:
-  static void _avdtcallback(UNUSED_ATTR uint8_t handle, const RawAddress& bd_addr,
-   uint8_t event, tAVDT_CTRL* p_data, uint8_t scb_index) {
+  static void AvdtConnCallback(uint8_t handle, const RawAddress& bd_addr,
+                               uint8_t event, tAVDT_CTRL* p_data,
+                               uint8_t scb_index) {
     mock_function_count_map[__func__]++;
+    callback_event_ = event;
   }
 
-  static void _streamcallback(uint8_t handle, const RawAddress& bd_addr,
-   uint8_t event, tAVDT_CTRL* p_data,
-   uint8_t scb_index) {
+  static void StreamCtrlCallback(uint8_t handle, const RawAddress& bd_addr,
+                                 uint8_t event, tAVDT_CTRL* p_data,
+                                 uint8_t scb_index) {
     mock_function_count_map[__func__]++;
-    ASSERT_EQ(event, _expected_stream_event);
-  };
-
-  static void SetUpTestCase() {
-    _reg.ctrl_mtu = 672;
-    _reg.ret_tout = 4;
-    _reg.sig_tout = 4;
-    _reg.idle_tout = 10;
-    _reg.scb_index = 0;
-    AVDT_Register(&_reg, _avdtcallback);
+    callback_event_ = event;
   }
 
-  static void TearDownTestCase() {
-    AVDT_Deregister();
+  static void AvdtReportCallback(uint8_t handle, AVDT_REPORT_TYPE type,
+                                 tAVDT_REPORT_DATA* p_data) {
+    mock_function_count_map[__func__]++;
   }
 
-  void SetUp() override {
+  static void SetUpTestCase() {
+    reg_ctrl_block_.ctrl_mtu = 672;
+    reg_ctrl_block_.ret_tout = 4;
+    reg_ctrl_block_.sig_tout = 4;
+    reg_ctrl_block_.idle_tout = 10;
+    reg_ctrl_block_.scb_index = 0;
+    AVDT_Register(&reg_ctrl_block_, AvdtConnCallback);
+
     uint8_t peer_id = 1;
     scb_handle_ = 0;
-    _expected_stream_event = 0;
-    AvdtpStreamConfig avdtp_stream_config;
+    AvdtpStreamConfig avdtp_stream_config{};
     avdtp_stream_config.cfg.psc_mask = AVDT_PSC_DELAY_RPT;
+    avdtp_stream_config.p_avdt_ctrl_cback = StreamCtrlCallback;
+    avdtp_stream_config.p_report_cback = AvdtReportCallback;
     avdtp_stream_config.tsep = AVDT_TSEP_SNK;
-    avdtp_stream_config.p_avdt_ctrl_cback = _streamcallback;
-    mock_function_count_map["_streamcallback"] = 0;
-
+    // We have to reuse the stream since there is only AVDT_NUM_SEPS *
+    // AVDT_NUM_LINKS
     ASSERT_EQ(AVDT_CreateStream(peer_id, &scb_handle_, avdtp_stream_config), AVDT_SUCCESS);
   }
 
-  void TearDown() override {
-    ASSERT_EQ(AVDT_RemoveStream(scb_handle_), AVDT_SUCCESS);
+  static void TearDownTestCase() { AVDT_Deregister(); }
+
+  void SetUp() override {
+    callback_event_ = AVDT_MAX_EVT + 1;
+    mock_function_count_map.clear();
   }
 
-  void StreamCallBackExpect(uint8_t event) {
-    _expected_stream_event = event;
+  void TearDown() override {
+    auto pscb = avdt_scb_by_hdl(scb_handle_);
+    tAVDT_SCB_EVT data;
+    // clean up the SCB state
+    avdt_scb_event(pscb, AVDT_SCB_MSG_ABORT_RSP_EVT, &data);
+    avdt_scb_event(pscb, AVDT_SCB_TC_CLOSE_EVT, &data);
+    ASSERT_EQ(AVDT_RemoveStream(scb_handle_), AVDT_SUCCESS);
+    // fallback to default settings (delay report + sink)
+    pscb->stream_config.cfg.psc_mask = AVDT_PSC_DELAY_RPT;
+    pscb->stream_config.tsep = AVDT_TSEP_SNK;
   }
 };
 
-AvdtpRcb StackAvdtpTest::_reg{};
-uint8_t StackAvdtpTest::_expected_stream_event = 0;
+AvdtpRcb StackAvdtpTest::reg_ctrl_block_{};
+uint8_t StackAvdtpTest::callback_event_ = AVDT_MAX_EVT + 1;
+uint8_t StackAvdtpTest::scb_handle_ = 0;
 
 TEST_F(StackAvdtpTest, test_delay_report_as_accept) {
   // Get SCB ready to send response
@@ -107,8 +120,6 @@ TEST_F(StackAvdtpTest, test_delay_report_as_accept) {
 
   mock_avdt_msg_send_cmd_clear_history();
   mock_avdt_msg_send_rsp_clear_history();
-  mock_function_count_map["avdt_msg_send_rsp"] = 0;
-  mock_function_count_map["avdt_msg_send_cmd"] = 0;
   ASSERT_EQ(AVDT_ConfigRsp(scb_handle_, label, err_code, category), AVDT_SUCCESS);
 
   // Config response sent
@@ -121,10 +132,9 @@ TEST_F(StackAvdtpTest, test_delay_report_as_accept) {
 
   // Delay report confirmed
   tAVDT_SCB_EVT data;
-  ASSERT_EQ(mock_function_count_map["_streamcallback"], 0);
-  StreamCallBackExpect(AVDT_DELAY_REPORT_CFM_EVT);
+  ASSERT_EQ(mock_function_count_map["StreamCtrlCallback"], 0);
   avdt_scb_hdl_delay_rpt_rsp(pscb, &data);
-  ASSERT_EQ(mock_function_count_map["_streamcallback"], 1);
+  ASSERT_EQ(callback_event_, AVDT_DELAY_REPORT_CFM_EVT);
 }
 
 TEST_F(StackAvdtpTest, test_no_delay_report_if_not_sink) {
@@ -139,8 +149,6 @@ TEST_F(StackAvdtpTest, test_no_delay_report_if_not_sink) {
   uint8_t label = 0;
   uint8_t err_code = 0;
   uint8_t category = 0;
-  mock_function_count_map["avdt_msg_send_rsp"] = 0;
-  mock_function_count_map["avdt_msg_send_cmd"] = 0;
   ASSERT_EQ(AVDT_ConfigRsp(scb_handle_, label, err_code, category), AVDT_SUCCESS);
   ASSERT_EQ(mock_function_count_map["avdt_msg_send_rsp"], 1); // Config response sent
   ASSERT_EQ(mock_function_count_map["avdt_msg_send_cmd"], 0); // Delay report command not sent
@@ -158,8 +166,6 @@ TEST_F(StackAvdtpTest, test_no_delay_report_if_not_enabled) {
   uint8_t label = 0;
   uint8_t err_code = 0;
   uint8_t category = 0;
-  mock_function_count_map["avdt_msg_send_rsp"] = 0;
-  mock_function_count_map["avdt_msg_send_cmd"] = 0;
   ASSERT_EQ(AVDT_ConfigRsp(scb_handle_, label, err_code, category), AVDT_SUCCESS);
   ASSERT_EQ(mock_function_count_map["avdt_msg_send_rsp"], 1); // Config response sent
   ASSERT_EQ(mock_function_count_map["avdt_msg_send_cmd"], 0); // Delay report command not sent
@@ -167,11 +173,10 @@ TEST_F(StackAvdtpTest, test_no_delay_report_if_not_enabled) {
 
 TEST_F(StackAvdtpTest, test_delay_report_as_init) {
   auto pscb = avdt_scb_by_hdl(scb_handle_);
+  pscb->in_use = true;
 
   tAVDT_SCB_EVT data;
 
-  mock_function_count_map["avdt_msg_send_cmd"] = 0;
-
   // Delay report -> Open command
   mock_avdt_msg_send_cmd_clear_history();
   avdt_scb_event(pscb, AVDT_SCB_MSG_SETCONFIG_RSP_EVT, &data);
@@ -180,3 +185,132 @@ TEST_F(StackAvdtpTest, test_delay_report_as_init) {
   ASSERT_EQ(mock_avdt_msg_send_cmd_get_sig_id_at(1), AVDT_SIG_OPEN);
 }
 
+TEST_F(StackAvdtpTest, test_SR_reporting_handler) {
+  constexpr uint8_t sender_report_packet[] = {
+      // Header
+      0x80, 0xc8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      // Sender Info
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      // Report Block #1
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+  uint16_t packet_length = sizeof(sender_report_packet);
+  tAVDT_SCB_EVT data;
+  auto pscb = avdt_scb_by_hdl(scb_handle_);
+
+  data.p_pkt = (BT_HDR*)osi_calloc(sizeof(BT_HDR) + packet_length);
+  *data.p_pkt = {.len = packet_length, .layer_specific = AVDT_CHAN_REPORT};
+  memcpy(data.p_pkt->data, sender_report_packet, packet_length);
+  avdt_scb_hdl_pkt(pscb, &data);
+  ASSERT_EQ(mock_function_count_map["AvdtReportCallback"], 1);
+
+  // no payload
+  data.p_pkt = (BT_HDR*)osi_calloc(sizeof(BT_HDR) + packet_length);
+  *data.p_pkt = {.layer_specific = AVDT_CHAN_REPORT};
+  memcpy(data.p_pkt->data, sender_report_packet, packet_length);
+  avdt_scb_hdl_pkt(pscb, &data);
+  ASSERT_EQ(mock_function_count_map["AvdtReportCallback"], 1);
+
+  // only reporting header
+  data.p_pkt = (BT_HDR*)osi_calloc(sizeof(BT_HDR) + packet_length);
+  *data.p_pkt = {.len = 8, .layer_specific = AVDT_CHAN_REPORT};
+  memcpy(data.p_pkt->data, sender_report_packet, packet_length);
+  avdt_scb_hdl_pkt(pscb, &data);
+  ASSERT_EQ(mock_function_count_map["AvdtReportCallback"], 1);
+
+  // reporting header + sender info
+  data.p_pkt = (BT_HDR*)osi_calloc(sizeof(BT_HDR) + packet_length);
+  *data.p_pkt = {.len = 28, .layer_specific = AVDT_CHAN_REPORT};
+  memcpy(data.p_pkt->data, sender_report_packet, packet_length);
+  avdt_scb_hdl_pkt(pscb, &data);
+  ASSERT_EQ(mock_function_count_map["AvdtReportCallback"], 2);
+}
+
+TEST_F(StackAvdtpTest, test_RR_reporting_handler) {
+  constexpr uint8_t receiver_report_packet[] = {
+      // Header
+      0x80, 0xc9, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      // Report Block #1
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+  uint16_t packet_length = sizeof(receiver_report_packet);
+  tAVDT_SCB_EVT data;
+  auto pscb = avdt_scb_by_hdl(scb_handle_);
+
+  data.p_pkt = (BT_HDR*)osi_calloc(sizeof(BT_HDR) + packet_length);
+  *data.p_pkt = {.len = packet_length, .layer_specific = AVDT_CHAN_REPORT};
+  memcpy(data.p_pkt->data, receiver_report_packet, packet_length);
+  avdt_scb_hdl_pkt(pscb, &data);
+  ASSERT_EQ(mock_function_count_map["AvdtReportCallback"], 1);
+
+  // no payload
+  data.p_pkt = (BT_HDR*)osi_calloc(sizeof(BT_HDR) + packet_length);
+  *data.p_pkt = {.layer_specific = AVDT_CHAN_REPORT};
+  memcpy(data.p_pkt->data, receiver_report_packet, packet_length);
+  avdt_scb_hdl_pkt(pscb, &data);
+  ASSERT_EQ(mock_function_count_map["AvdtReportCallback"], 1);
+
+  // only reporting header
+  data.p_pkt = (BT_HDR*)osi_calloc(sizeof(BT_HDR) + packet_length);
+  *data.p_pkt = {.len = 8, .layer_specific = AVDT_CHAN_REPORT};
+  memcpy(data.p_pkt->data, receiver_report_packet, packet_length);
+  avdt_scb_hdl_pkt(pscb, &data);
+  ASSERT_EQ(mock_function_count_map["AvdtReportCallback"], 1);
+
+  // reporting header + report block
+  data.p_pkt = (BT_HDR*)osi_calloc(sizeof(BT_HDR) + packet_length);
+  *data.p_pkt = {.len = 32, .layer_specific = AVDT_CHAN_REPORT};
+  memcpy(data.p_pkt->data, receiver_report_packet, packet_length);
+  avdt_scb_hdl_pkt(pscb, &data);
+  ASSERT_EQ(mock_function_count_map["AvdtReportCallback"], 2);
+}
+
+TEST_F(StackAvdtpTest, test_SDES_reporting_handler) {
+  constexpr uint8_t source_description_packet[] = {// Header
+                                                   0x80, 0xca, 0x00, 0x00,
+                                                   // Chunk #1
+                                                   0x00, 0x00, 0x00, 0x00,
+                                                   // SDES Item (CNAME=1)
+                                                   0x01, 0x05, 0x00, 0x00, 0x00,
+                                                   0x00, 0x00, 0x00};
+  uint16_t packet_length = sizeof(source_description_packet);
+  tAVDT_SCB_EVT data;
+  auto pscb = avdt_scb_by_hdl(scb_handle_);
+
+  data.p_pkt = (BT_HDR*)osi_calloc(sizeof(BT_HDR) + packet_length);
+  *data.p_pkt = {.len = packet_length, .layer_specific = AVDT_CHAN_REPORT};
+  memcpy(data.p_pkt->data, source_description_packet, packet_length);
+  avdt_scb_hdl_pkt(pscb, &data);
+  ASSERT_EQ(mock_function_count_map["AvdtReportCallback"], 1);
+
+  // no payload
+  data.p_pkt = (BT_HDR*)osi_calloc(sizeof(BT_HDR) + packet_length);
+  *data.p_pkt = {.layer_specific = AVDT_CHAN_REPORT};
+  memcpy(data.p_pkt->data, source_description_packet, packet_length);
+  avdt_scb_hdl_pkt(pscb, &data);
+  ASSERT_EQ(mock_function_count_map["AvdtReportCallback"], 1);
+
+  // only reporting header
+  data.p_pkt = (BT_HDR*)osi_calloc(sizeof(BT_HDR) + packet_length);
+  *data.p_pkt = {.len = 4, .layer_specific = AVDT_CHAN_REPORT};
+  memcpy(data.p_pkt->data, source_description_packet, packet_length);
+  avdt_scb_hdl_pkt(pscb, &data);
+  ASSERT_EQ(mock_function_count_map["AvdtReportCallback"], 1);
+
+  // SDES Item (CNAME) with empty value
+  data.p_pkt = (BT_HDR*)osi_calloc(sizeof(BT_HDR) + packet_length);
+  *data.p_pkt = {.len = 10, .layer_specific = AVDT_CHAN_REPORT};
+  memcpy(data.p_pkt->data, source_description_packet, packet_length);
+  avdt_scb_hdl_pkt(pscb, &data);
+  ASSERT_EQ(mock_function_count_map["AvdtReportCallback"], 1);
+
+  // SDES Item (not CNAME) which is not supported
+  data.p_pkt = (BT_HDR*)osi_calloc(sizeof(BT_HDR) + packet_length);
+  *data.p_pkt = {.len = 10, .layer_specific = AVDT_CHAN_REPORT};
+  memcpy(data.p_pkt->data, source_description_packet, packet_length);
+  *(data.p_pkt->data + 8) = 0x02;
+  *(data.p_pkt->data + 9) = 0x00;
+  avdt_scb_hdl_pkt(pscb, &data);
+  ASSERT_EQ(mock_function_count_map["AvdtReportCallback"], 1);
+}