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

diff --git a/stack/avrc/avrc_pars_tg.cc b/stack/avrc/avrc_pars_tg.cc
index 5a81d0d..190a88d 100644 (file)
--- 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 ad1cc9e..72ec45f 100644 (file)
--- a/stack/test/stack_avrcp_test.cc
+++ b/stack/test/stack_avrcp_test.cc
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include <arpa/inet.h>  // htons
 #include <dlfcn.h>
 #include <gtest/gtest.h>
 
@@ -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);
+}