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 03bfb9e880764c1fbad3c7ce5159c295f1c6d551)
diff --git a/stack/Android.bp b/stack/Android.bp
index f7f5a45..f9d11e2 100644
--- a/stack/Android.bp
+++ b/stack/Android.bp
@@ -200,6 +200,7 @@
],
srcs: [
"test/stack_a2dp_test.cc",
+ "test/stack_avrcp_test.cc",
],
shared_libs: [
"libcrypto",
diff --git a/stack/BUILD.gn b/stack/BUILD.gn
index 00680c8..87dc726 100644
--- a/stack/BUILD.gn
+++ b/stack/BUILD.gn
@@ -202,6 +202,7 @@
testonly = true
sources = [
"test/stack_a2dp_test.cc",
+ "test/stack_avrcp_test.cc",
]
include_dirs = [
diff --git a/stack/avrc/avrc_pars_tg.cc b/stack/avrc/avrc_pars_tg.cc
index 22471bd..fe1db3d 100644
--- a/stack/avrc/avrc_pars_tg.cc
+++ b/stack/avrc/avrc_pars_tg.cc
@@ -363,7 +363,7 @@
*
* 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
} 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;
diff --git a/stack/test/stack_avrcp_test.cc b/stack/test/stack_avrcp_test.cc
new file mode 100644
index 0000000..ad1cc9e
--- /dev/null
+++ b/stack/test/stack_avrcp_test.cc
@@ -0,0 +1,112 @@
+/*
+ * 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);
+}
