From bf9310d47472ac58cba37ceccc9f1ed836a662d3 Mon Sep 17 00:00:00 2001 From: Chris Manton Date: Sun, 21 Mar 2021 15:51:18 -0700 Subject: [PATCH] RESTRICT AUTOMERGE Contain avrc_ctrl_pars_vendor_cmd OOB write Bug: 181860042 Test: net_test_stack Tag: #security Ignore-AOSP-First: Security Change-Id: I5d8d4051a1439ee9f1f04af3dfe6da6d8016e546 --- stack/avrc/avrc_pars_tg.cc | 6 ++++++ stack/test/stack_avrcp_test.cc | 47 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/stack/avrc/avrc_pars_tg.cc b/stack/avrc/avrc_pars_tg.cc index 5a81d0d8a..190a88d75 100644 --- a/stack/avrc/avrc_pars_tg.cc +++ b/stack/avrc/avrc_pars_tg.cc @@ -75,6 +75,12 @@ static tAVRC_STS avrc_ctrl_pars_vendor_cmd(tAVRC_MSG_VENDOR* p_msg, BE_STREAM_TO_UINT8(p_result->reg_notif.event_id, p); BE_STREAM_TO_UINT32(p_result->reg_notif.param, p); + + if (p_result->reg_notif.event_id == 0 || + p_result->reg_notif.event_id > AVRC_NUM_NOTIF_EVENTS) { + android_errorWriteLog(0x534e4554, "181860042"); + status = AVRC_STS_BAD_PARAM; + } break; default: status = AVRC_STS_BAD_CMD; diff --git a/stack/test/stack_avrcp_test.cc b/stack/test/stack_avrcp_test.cc index ad1cc9e72..72ec45f29 100644 --- a/stack/test/stack_avrcp_test.cc +++ b/stack/test/stack_avrcp_test.cc @@ -14,6 +14,7 @@ * limitations under the License. */ +#include // htons #include #include @@ -110,3 +111,49 @@ TEST_F(StackAvrcpTest, test_avrcp_parse_browse_cmd) { EXPECT_EQ(AVRC_ParsCommand(&msg, &result, scratch_buf, sizeof(scratch_buf)), AVRC_STS_NO_ERROR); } + +TEST_F(StackAvrcpTest, test_avrcp_pdu_register_notification) { + ASSERT_EQ(htons(0x500), 5); + + struct { + uint8_t pdu; + uint8_t reserved; + uint16_t len; + struct { + uint8_t event_id; + uint32_t param; + } payload; + } data = { + AVRC_PDU_REGISTER_NOTIFICATION, + 0, // reserved + htons(sizeof(data.payload)), + .payload = + { + .event_id = 0, + .param = 0x1234, + }, + }; + + tAVRC_MSG msg = { + .vendor = + { + .hdr = + { + .ctype = AVRC_CMD_NOTIF, + .opcode = AVRC_OP_VENDOR, + }, + .p_vendor_data = (uint8_t*)&data, + .vendor_len = sizeof(data), + }, + }; + tAVRC_COMMAND result{}; + + // Run through all possible event ids + uint8_t id = 0; + do { + data.payload.event_id = id; + ASSERT_EQ((id == 0 || id > AVRC_NUM_NOTIF_EVENTS) ? AVRC_STS_BAD_PARAM + : AVRC_STS_NO_ERROR, + AVRC_Ctrl_ParsCommand(&msg, &result)); + } while (++id != 0); +} -- 2.11.0