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

Commit 03a1907b authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Automerger Merge Worker
Browse files

Merge "Fix potential interger overflow when parsing vendor response" into tm-dev am: 4f88fd62 

Original change: https://googleplex-android-review.googlesource.com/c/platform/packages/modules/Bluetooth/+/17530926



Change-Id: Iede5d5ab7194840c2400d048f497fa8ccd1a6a44
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents 83365df4 4f88fd62

system/stack/avrc/avrc_pars_ct.cc

+22 −5
Original line number Original line Diff line number Diff line
@@ -237,7 +237,7 @@ static tAVRC_STS avrc_pars_browse_rsp(tAVRC_MSG_BROWSE* p_msg, 
  }

  }

  BE_STREAM_TO_UINT8(pdu, p);

  BE_STREAM_TO_UINT8(pdu, p);

  uint16_t pkt_len;

  uint16_t pkt_len;

  int min_len = 0;

  uint16_t min_len = 0;

  /* read the entire packet len */

  /* read the entire packet len */

  BE_STREAM_TO_UINT16(pkt_len, p);

  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 */

              /* Parse the name now */

              BE_STREAM_TO_UINT16(attr_entry->name.charset_id, p);

              BE_STREAM_TO_UINT16(attr_entry->name.charset_id, p);

              BE_STREAM_TO_UINT16(attr_entry->name.str_len, 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;

              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.p_str = (uint8_t*)osi_malloc(

                  attr_entry->name.str_len * sizeof(uint8_t));

                  attr_entry->name.str_len * sizeof(uint8_t));

              BE_STREAM_TO_ARRAY(p, attr_entry->name.p_str,

              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_UINT32(attr_entry->attr_id, p);

        BE_STREAM_TO_UINT16(attr_entry->name.charset_id, p);

        BE_STREAM_TO_UINT16(attr_entry->name.charset_id, p);

        BE_STREAM_TO_UINT16(attr_entry->name.str_len, 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;

        min_len += attr_entry->name.str_len;

        if (pkt_len < min_len) goto browse_length_error;

        attr_entry->name.p_str =

        attr_entry->name.p_str =

            (uint8_t*)osi_malloc(attr_entry->name.str_len * sizeof(uint8_t));

            (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);

        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_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.charset_id, p);

          BE_STREAM_TO_UINT16(p_attrs[i].name.str_len, p);

          BE_STREAM_TO_UINT16(p_attrs[i].name.str_len, p);

          min_len += p_attrs[i].name.str_len;

          if (static_cast<uint16_t>(min_len + p_attrs[i].name.str_len) <

          if (len < min_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++) {

            for (int j = 0; j < i; j++) {

              osi_free(p_attrs[j].name.p_str);

              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;

            p_result->get_attrs.num_attrs = 0;

            goto length_error;

            goto length_error;

          }

          }

          min_len += p_attrs[i].name.str_len;

          if (p_attrs[i].name.str_len > 0) {

          if (p_attrs[i].name.str_len > 0) {

            p_attrs[i].name.p_str =

            p_attrs[i].name.p_str =

                (uint8_t*)osi_calloc(p_attrs[i].name.str_len);

                (uint8_t*)osi_calloc(p_attrs[i].name.str_len);

system/stack/test/stack_avrcp_test.cc

+50 −0
Original line number Original line Diff line number Diff line
@@ -27,6 +27,56 @@ class StackAvrcpTest : public ::testing::Test { 
  virtual ~StackAvrcpTest() = default;

  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) {

TEST_F(StackAvrcpTest, test_avrcp_parse_browse_cmd) {

  uint8_t scratch_buf[512]{};

  uint8_t scratch_buf[512]{};

  tAVRC_MSG msg{};

  tAVRC_MSG msg{};