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

Commit 833147ce authored by Alain Michaud's avatar Alain Michaud
Browse files

avrc: Validating msg size before accessing fields 

This change adds buffer length validation during the parsing of AVRCP
browse commands.

Bug: 79945152
Test: net_test_stack
Change-Id: Icfc44f9a91fe004932e15182b1ca3ad5bdac6370
(cherry picked from commit 0fa0bb9b)
parent 1ef9ad9d

system/stack/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -200,6 +200,7 @@ cc_test { 
    ],

    srcs: [

        "test/stack_a2dp_test.cc",

        "test/stack_avrcp_test.cc",

    ],

    shared_libs: [

        "libcrypto",

system/stack/BUILD.gn

+1 −0
Original line number Diff line number Diff line
@@ -202,6 +202,7 @@ executable("stack_unittests") { 
  testonly = true

  sources = [

    "test/stack_a2dp_test.cc",

    "test/stack_avrcp_test.cc",

  ]



  include_dirs = [

system/stack/avrc/avrc_pars_tg.cc

+51 −1
Original line number Diff line number Diff line
@@ -363,7 +363,7 @@ static tAVRC_STS avrc_pars_vendor_cmd(tAVRC_MSG_VENDOR* p_msg, 
 *

 * Description      This function is used to parse cmds received for CTRL

 *                  Currently it is for SetAbsVolume and Volume Change

 *                  Notification..

 *                  Notification.

 *

 * Returns          AVRC_STS_NO_ERROR, if the message in p_data is parsed

 *                  successfully.
@@ -390,6 +390,12 @@ tAVRC_STS AVRC_Ctrl_ParsCommand(tAVRC_MSG* p_msg, tAVRC_COMMAND* p_result) { 
  return status;

}



#define RETURN_STATUS_IF_FALSE(_status_, _b_, _msg_, ...) \

  if (!(_b_)) {                                           \

    AVRC_TRACE_DEBUG(_msg_, ##__VA_ARGS__);               \

    return _status_;                                      \

  }



/*******************************************************************************

 *

 * Function         avrc_pars_browsing_cmd
@@ -409,6 +415,10 @@ 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;

  RETURN_STATUS_IF_FALSE(AVRC_STS_BAD_CMD, (p_msg->browse_len >= min_len),

                         "msg too short");



  p_result->pdu = *p++;

  AVRC_TRACE_DEBUG("avrc_pars_browsing_cmd() pdu:0x%x", p_result->pdu);

  /* skip over len */
@@ -416,11 +426,20 @@ static tAVRC_STS avrc_pars_browsing_cmd(tAVRC_MSG_BROWSE* p_msg, 


  switch (p_result->pdu) {

    case AVRC_PDU_SET_BROWSED_PLAYER: /* 0x70 */

      min_len += 2;

      RETURN_STATUS_IF_FALSE(AVRC_STS_BAD_CMD, (p_msg->browse_len >= min_len),

                             "msg too short");



      // For current implementation all players are browsable.

      BE_STREAM_TO_UINT16(p_result->br_player.player_id, p);

      break;



    case AVRC_PDU_GET_FOLDER_ITEMS: /* 0x71 */



      min_len += 10;

      RETURN_STATUS_IF_FALSE(AVRC_STS_BAD_CMD, (p_msg->browse_len >= min_len),

                             "msg too short");



      STREAM_TO_UINT8(p_result->get_items.scope, p);

      // To be modified later here (Scope) when all browsing commands are

      // supported
@@ -441,12 +460,21 @@ static tAVRC_STS avrc_pars_browsing_cmd(tAVRC_MSG_BROWSE* p_msg, 
        if (buf_len < (count << 2))

          p_result->get_items.attr_count = count = (buf_len >> 2);

        for (int idx = 0; idx < count; idx++) {

          min_len += 4;

          RETURN_STATUS_IF_FALSE(AVRC_STS_BAD_CMD,

                                 (p_msg->browse_len >= min_len),

                                 "msg too short");



          BE_STREAM_TO_UINT32(p_result->get_items.p_attr_list[idx], p);

        }

      }

      break;



    case AVRC_PDU_CHANGE_PATH: /* 0x72 */

      min_len += 11;

      RETURN_STATUS_IF_FALSE(AVRC_STS_BAD_CMD, (p_msg->browse_len >= min_len),

                             "msg too short");



      BE_STREAM_TO_UINT16(p_result->chg_path.uid_counter, p);

      BE_STREAM_TO_UINT8(p_result->chg_path.direction, p);

      if (p_result->chg_path.direction != AVRC_DIR_UP &&
@@ -457,7 +485,12 @@ static tAVRC_STS avrc_pars_browsing_cmd(tAVRC_MSG_BROWSE* p_msg, 
      break;



    case AVRC_PDU_GET_ITEM_ATTRIBUTES: /* 0x73 */

      min_len += 12;

      RETURN_STATUS_IF_FALSE(AVRC_STS_BAD_CMD, (p_msg->browse_len >= min_len),

                             "msg too short");



      BE_STREAM_TO_UINT8(p_result->get_attrs.scope, p);



      if (p_result->get_attrs.scope > AVRC_SCOPE_NOW_PLAYING) {

        status = AVRC_STS_BAD_SCOPE;

        break;
@@ -473,6 +506,11 @@ static tAVRC_STS avrc_pars_browsing_cmd(tAVRC_MSG_BROWSE* p_msg, 
          p_result->get_attrs.attr_count = count = (buf_len >> 2);

        for (int idx = 0, count = 0; idx < p_result->get_attrs.attr_count;

             idx++) {

          min_len += 4;

          RETURN_STATUS_IF_FALSE(AVRC_STS_BAD_CMD,

                                 (p_msg->browse_len >= min_len),

                                 "msg too short");



          BE_STREAM_TO_UINT32(p_result->get_attrs.p_attr_list[count], p);

          if (AVRC_IS_VALID_MEDIA_ATTRIBUTE(

                  p_result->get_attrs.p_attr_list[count])) {
@@ -488,6 +526,10 @@ static tAVRC_STS avrc_pars_browsing_cmd(tAVRC_MSG_BROWSE* p_msg, 
      break;



    case AVRC_PDU_GET_TOTAL_NUM_OF_ITEMS: /* 0x75 */

      ++min_len;

      RETURN_STATUS_IF_FALSE(AVRC_STS_BAD_CMD, (p_msg->browse_len >= min_len),

                             "msg too short");



      BE_STREAM_TO_UINT8(p_result->get_num_of_items.scope, p);

      if (p_result->get_num_of_items.scope > AVRC_SCOPE_NOW_PLAYING) {

        status = AVRC_STS_BAD_SCOPE;
@@ -495,6 +537,10 @@ static tAVRC_STS avrc_pars_browsing_cmd(tAVRC_MSG_BROWSE* p_msg, 
      break;



    case AVRC_PDU_SEARCH: /* 0x80 */

      min_len += 4;

      RETURN_STATUS_IF_FALSE(AVRC_STS_BAD_CMD, (p_msg->browse_len >= min_len),

                             "msg too short");



      BE_STREAM_TO_UINT16(p_result->search.string.charset_id, p);

      BE_STREAM_TO_UINT16(p_result->search.string.str_len, p);

      p_result->search.string.p_str = p_buf;
@@ -504,6 +550,10 @@ static tAVRC_STS avrc_pars_browsing_cmd(tAVRC_MSG_BROWSE* p_msg, 
        } else {

          android_errorWriteLog(0x534e4554, "63146237");

        }

        min_len += p_result->search.string.str_len;

        RETURN_STATUS_IF_FALSE(AVRC_STS_BAD_CMD, (p_msg->browse_len >= min_len),

                               "msg too short");



        BE_STREAM_TO_ARRAY(p, p_buf, p_result->search.string.str_len);

      } else {

        status = AVRC_STS_INTERNAL_ERR;

system/stack/test/stack_avrcp_test.cc

0 → 100644
+112 −0
Original line number Diff line number Diff line 
/*

 * Copyright 2020 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



#include <dlfcn.h>

#include <gtest/gtest.h>



#include "stack/include/avrc_api.h"



class StackAvrcpTest : public ::testing::Test {

 protected:

  StackAvrcpTest() = default;



  virtual ~StackAvrcpTest() = default;

};



TEST_F(StackAvrcpTest, test_avrcp_parse_browse_cmd) {

  uint8_t scratch_buf[512]{};

  tAVRC_MSG msg{};

  tAVRC_COMMAND result{};

  uint8_t browse_cmd_buf[512]{};



  msg.hdr.opcode = AVRC_OP_BROWSE;

  msg.browse.p_browse_data = browse_cmd_buf;

  msg.browse.browse_len = 2;

  EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)),

            AVRC_STS_BAD_CMD);



  memset(browse_cmd_buf, 0, sizeof(browse_cmd_buf));

  browse_cmd_buf[0] = AVRC_PDU_SET_BROWSED_PLAYER;

  msg.browse.browse_len = 3;

  EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)),

            AVRC_STS_BAD_CMD);



  msg.browse.browse_len = 5;

  EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)),

            AVRC_STS_NO_ERROR);



  memset(browse_cmd_buf, 0, sizeof(browse_cmd_buf));

  browse_cmd_buf[0] = AVRC_PDU_GET_FOLDER_ITEMS;

  msg.browse.browse_len = 3;

  EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)),

            AVRC_STS_BAD_CMD);



  msg.browse.browse_len = 13;

  uint8_t* p = &browse_cmd_buf[3];

  UINT8_TO_STREAM(p, AVRC_SCOPE_NOW_PLAYING);  // scope

  UINT32_TO_STREAM(p, 0x00000001);             // start_item

  UINT32_TO_STREAM(p, 0x00000002);             // end_item

  browse_cmd_buf[12] = 0;                      // attr_count

  EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)),

            AVRC_STS_NO_ERROR);



  memset(browse_cmd_buf, 0, sizeof(browse_cmd_buf));

  browse_cmd_buf[0] = AVRC_PDU_CHANGE_PATH;

  msg.browse.browse_len = 3;

  EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)),

            AVRC_STS_BAD_CMD);



  msg.browse.browse_len = 14;

  p = &browse_cmd_buf[3];

  UINT16_TO_STREAM(p, 0x1234);      // uid_counter

  UINT8_TO_STREAM(p, AVRC_DIR_UP);  // direction

  UINT8_TO_STREAM(p, 0);            // attr_count

  EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)),

            AVRC_STS_NO_ERROR);



  memset(browse_cmd_buf, 0, sizeof(browse_cmd_buf));

  browse_cmd_buf[0] = AVRC_PDU_GET_ITEM_ATTRIBUTES;

  msg.browse.browse_len = 3;

  EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)),

            AVRC_STS_BAD_CMD);



  msg.browse.browse_len = 15;

  EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)),

            AVRC_STS_NO_ERROR);



  memset(browse_cmd_buf, 0, sizeof(browse_cmd_buf));

  browse_cmd_buf[0] = AVRC_PDU_GET_TOTAL_NUM_OF_ITEMS;

  msg.browse.browse_len = 3;

  EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)),

            AVRC_STS_BAD_CMD);



  msg.browse.browse_len = 4;

  EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)),

            AVRC_STS_NO_ERROR);



  memset(browse_cmd_buf, 0, sizeof(browse_cmd_buf));

  browse_cmd_buf[0] = AVRC_PDU_SEARCH;

  msg.browse.browse_len = 3;

  EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)),

            AVRC_STS_BAD_CMD);



  p = &browse_cmd_buf[3];

  UINT16_TO_STREAM(p, 0x0000);  // charset_id

  UINT16_TO_STREAM(p, 0x0000);  // str_len

  msg.browse.browse_len = 7;

  EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)),

            AVRC_STS_NO_ERROR);

}