Validate all AVRCP packets before use to prevent out of bounds reads.

If a packet arrives that is too short to read all the fields from, then
an out of bounds read occurs. This patch validates all recieved packets
before use.

Bug: 119562595
Test: run net_test_avrcp
Change-Id: Ifc1c0def1b411664facbbeb7e4d6e9c56ecc31c1

diff --git a/packet/avrcp/general_reject_packet.cc b/packet/avrcp/general_reject_packet.cc
index cde0031..db3d31a 100644 (file)
--- a/packet/avrcp/general_reject_packet.cc
+++ b/packet/avrcp/general_reject_packet.cc
@@ -19,11 +19,9 @@
 namespace bluetooth {
 namespace avrcp {
 
-std::unique_ptr<GeneralRejectBuilder> GeneralRejectBuilder::MakeBuilder(
-    BrowsePdu pdu, Status reason) {
+std::unique_ptr<GeneralRejectBuilder> GeneralRejectBuilder::MakeBuilder(Status reason) {
   std::unique_ptr<GeneralRejectBuilder> builder =
-      std::unique_ptr<GeneralRejectBuilder>(
-          new GeneralRejectBuilder(pdu, reason));
+      std::unique_ptr<GeneralRejectBuilder>(new GeneralRejectBuilder(reason));
 
   return builder;
 }

diff --git a/packet/avrcp/general_reject_packet.h b/packet/avrcp/general_reject_packet.h
index 4058fd4..b3fb2fe 100644 (file)
--- a/packet/avrcp/general_reject_packet.h
+++ b/packet/avrcp/general_reject_packet.h
@@ -25,8 +25,7 @@ class GeneralRejectBuilder : public BrowsePacketBuilder {
  public:
   virtual ~GeneralRejectBuilder() = default;
 
-  static std::unique_ptr<GeneralRejectBuilder> MakeBuilder(BrowsePdu pdu,
-                                                           Status reason);
+  static std::unique_ptr<GeneralRejectBuilder> MakeBuilder(Status reason);
 
   virtual size_t size() const override;
   virtual bool Serialize(
@@ -35,8 +34,7 @@ class GeneralRejectBuilder : public BrowsePacketBuilder {
  protected:
   Status reason_;
 
-  GeneralRejectBuilder(BrowsePdu pdu, Status reason)
-      : BrowsePacketBuilder(pdu), reason_(reason){};
+  GeneralRejectBuilder(Status reason) : BrowsePacketBuilder(BrowsePdu::GENERAL_REJECT), reason_(reason){};
 };
 
 }  // namespace avrcp

diff --git a/packet/avrcp/register_notification_packet.cc b/packet/avrcp/register_notification_packet.cc
index adb5e59..6bbd16a 100644 (file)
--- a/packet/avrcp/register_notification_packet.cc
+++ b/packet/avrcp/register_notification_packet.cc
@@ -39,8 +39,9 @@ uint8_t RegisterNotificationResponse::GetVolume() const {
 bool RegisterNotificationResponse::IsValid() const {
   if (!VendorPacket::IsValid()) return false;
   if (size() < kMinSize()) return false;
-  if (GetCType() != CType::INTERIM && GetCType() != CType::CHANGED)
+  if (GetCType() != CType::INTERIM && GetCType() != CType::CHANGED && GetCType() != CType::REJECTED) {
     return false;
+  }
 
   switch (GetEvent()) {
     case Event::VOLUME_CHANGED:

diff --git a/packet/tests/avrcp/avrcp_test_packets.h b/packet/tests/avrcp/avrcp_test_packets.h
index 13994d0..14e30e7 100644 (file)
--- a/packet/tests/avrcp/avrcp_test_packets.h
+++ b/packet/tests/avrcp/avrcp_test_packets.h
@@ -106,8 +106,8 @@ std::vector<uint8_t> register_volume_changed_notification = {
     0x00, 0x05, 0x0d, 0x00, 0x00, 0x00, 0x00};
 
 // AVRCP Register Notification without any parameter
-std::vector<uint8_t> register_notification_invalid = {
-    0x03, 0x48, 0x00, 0x00, 0x19, 0x58, 0x31, 0x00, 0x00, 0x05};
+std::vector<uint8_t> register_notification_invalid = {0x03, 0x48, 0x00, 0x00, 0x19, 0x58, 0x31,
+                                                      0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00};
 
 // AVRCP Interim Playback Status Notification
 std::vector<uint8_t> interim_play_status_notification = {
@@ -208,9 +208,8 @@ std::vector<uint8_t> get_folder_items_request_vfs = {
 //    start_item = 0x00
 //    end_item = 0x05
 //    attributes_requested: All Items
-std::vector<uint8_t> get_folder_items_request_now_playing = {
-    0x71, 0x00, 0x0e, 0x03, 0x00, 0x00, 0x00,
-    0x00, 0x00, 0x00, 0x00, 0x05, 0x00};
+std::vector<uint8_t> get_folder_items_request_now_playing = {0x71, 0x00, 0x0a, 0x03, 0x00, 0x00, 0x00,
+                                                             0x00, 0x00, 0x00, 0x00, 0x05, 0x00};
 
 // AVRCP Browse Get Folder Items Response packet with range out of bounds error
 std::vector<uint8_t> get_folder_items_error_response = {0x71, 0x00, 0x01, 0x0b};
@@ -356,4 +355,36 @@ std::vector<uint8_t> set_absolute_volume_request = {
 std::vector<uint8_t> set_absolute_volume_response = {
     0x09, 0x48, 0x00, 0x00, 0x19, 0x58, 0x50, 0x00, 0x00, 0x01, 0x43};
 
+// Invalid Packets
+// Short Vendor Packet
+std::vector<uint8_t> short_vendor_packet = {0x01, 0x48, 0x00, 0x00, 0x19, 0x58, 0x10, 0x00, 0x00, 0x01};
+
+// Short Get Capabilities Request Packet
+std::vector<uint8_t> short_get_capabilities_request = {0x01, 0x48, 0x00, 0x00, 0x19, 0x58, 0x10, 0x00, 0x00, 0x00};
+
+// Short Get Element Attributes Request Packet
+std::vector<uint8_t> short_get_element_attributes_request = {0x01, 0x48, 0x00, 0x00, 0x19,
+                                                             0x58, 0x20, 0x00, 0x00, 0x00};
+
+// Short Play Item Request Packet
+std::vector<uint8_t> short_play_item_request = {0x00, 0x48, 0x00, 0x00, 0x19, 0x58, 0x74, 0x00, 0x00, 0x00};
+
+// Short Set Addressed Player Request Packet
+std::vector<uint8_t> short_set_addressed_player_request = {0x00, 0x48, 0x00, 0x00, 0x19, 0x58, 0x60, 0x00, 0x00, 0x00};
+
+// Short Browse Packet
+std::vector<uint8_t> short_browse_packet = {0x71, 0x00, 0x0a};
+
+// Short Get Folder Items Request Packet
+std::vector<uint8_t> short_get_folder_items_request = {0x71, 0x00, 0x00};
+
+// Short Get Total Number of Items Request Packet
+std::vector<uint8_t> short_get_total_number_of_items_request = {0x75, 0x00, 0x00};
+
+// Short Change Path Request Packet
+std::vector<uint8_t> short_change_path_request = {0x72, 0x00, 0x00};
+
+// Short Get Item Attributes Request Packet
+std::vector<uint8_t> short_get_item_attributes_request = {0x73, 0x00, 0x00};
+
 }  // namespace

diff --git a/packet/tests/avrcp/general_reject_packet_test.cc b/packet/tests/avrcp/general_reject_packet_test.cc
index e27d0b7..4d73dba 100644 (file)
--- a/packet/tests/avrcp/general_reject_packet_test.cc
+++ b/packet/tests/avrcp/general_reject_packet_test.cc
@@ -26,8 +26,7 @@ namespace avrcp {
 using TestGeneralRejectPacket = TestPacketType<BrowsePacket>;
 
 TEST(GeneralRejectPacketBuilderTest, buildPacketTest) {
-  auto builder = GeneralRejectBuilder::MakeBuilder(BrowsePdu::GENERAL_REJECT,
-                                                   Status::INVALID_COMMAND);
+  auto builder = GeneralRejectBuilder::MakeBuilder(Status::INVALID_COMMAND);
 
   ASSERT_EQ(builder->size(), general_reject_invalid_command_packet.size());
 

diff --git a/profile/avrcp/device.cc b/profile/avrcp/device.cc
index cf94cfc..4ca624b 100644 (file)
--- a/profile/avrcp/device.cc
+++ b/profile/avrcp/device.cc
@@ -80,6 +80,13 @@ void Device::VendorPacketHandler(uint8_t label,
   CHECK(media_interface_);
   DEVICE_VLOG(3) << __func__ << ": pdu=" << pkt->GetCommandPdu();
 
+  if (!pkt->IsValid()) {
+    DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
+    auto response = RejectBuilder::MakeBuilder(static_cast<CommandPdu>(0), Status::INVALID_COMMAND);
+    send_message(label, false, std::move(response));
+    return;
+  }
+
   // All CTypes at and above NOT_IMPLEMENTED are all response types.
   if (pkt->GetCType() == CType::NOT_IMPLEMENTED) {
     return;
@@ -125,9 +132,15 @@ void Device::VendorPacketHandler(uint8_t label,
     } break;
 
     case CommandPdu::GET_ELEMENT_ATTRIBUTES: {
-      media_interface_->GetSongInfo(base::Bind(
-          &Device::GetElementAttributesResponse, weak_ptr_factory_.GetWeakPtr(),
-          label, Packet::Specialize<GetElementAttributesRequest>(pkt)));
+      auto get_element_attributes_request_pkt = Packet::Specialize<GetElementAttributesRequest>(pkt);
+
+      if (!get_element_attributes_request_pkt->IsValid()) {
+        DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
+        auto response = RejectBuilder::MakeBuilder(pkt->GetCommandPdu(), Status::INVALID_PARAMETER);
+        send_message(label, false, std::move(response));
+      }
+      media_interface_->GetSongInfo(base::Bind(&Device::GetElementAttributesResponse, weak_ptr_factory_.GetWeakPtr(),
+                                               label, get_element_attributes_request_pkt));
     } break;
 
     case CommandPdu::GET_PLAY_STATUS: {
@@ -145,9 +158,17 @@ void Device::VendorPacketHandler(uint8_t label,
       // this currently since the current implementation only has one
       // player and the player will never change, but we need it for a
       // more complete implementation.
-      media_interface_->GetMediaPlayerList(base::Bind(
-          &Device::HandleSetAddressedPlayer, weak_ptr_factory_.GetWeakPtr(),
-          label, Packet::Specialize<SetAddressedPlayerRequest>(pkt)));
+      auto set_addressed_player_request = Packet::Specialize<SetAddressedPlayerRequest>(pkt);
+
+      if (!set_addressed_player_request->IsValid()) {
+        DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
+        auto response = RejectBuilder::MakeBuilder(pkt->GetCommandPdu(), Status::INVALID_PARAMETER);
+        send_message(label, false, std::move(response));
+        return;
+      }
+
+      media_interface_->GetMediaPlayerList(base::Bind(&Device::HandleSetAddressedPlayer, weak_ptr_factory_.GetWeakPtr(),
+                                                      label, set_addressed_player_request));
     } break;
 
     default: {
@@ -164,6 +185,13 @@ void Device::HandleGetCapabilities(
   DEVICE_VLOG(4) << __func__
                  << ": capability=" << pkt->GetCapabilityRequested();
 
+  if (!pkt->IsValid()) {
+    DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
+    auto response = RejectBuilder::MakeBuilder(pkt->GetCommandPdu(), Status::INVALID_PARAMETER);
+    send_message(label, false, std::move(response));
+    return;
+  }
+
   switch (pkt->GetCapabilityRequested()) {
     case Capability::COMPANY_ID: {
       auto response =
@@ -202,7 +230,7 @@ void Device::HandleGetCapabilities(
 void Device::HandleNotification(
     uint8_t label, const std::shared_ptr<RegisterNotificationRequest>& pkt) {
   if (!pkt->IsValid()) {
-    DEVICE_LOG(ERROR) << __func__ << ": Request packet is not valid";
+    DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
     auto response = RejectBuilder::MakeBuilder(pkt->GetCommandPdu(),
                                                Status::INVALID_PARAMETER);
     send_message(label, false, std::move(response));
@@ -307,6 +335,17 @@ void Device::RegisterVolumeChanged() {
 void Device::HandleVolumeChanged(
     uint8_t label, const std::shared_ptr<RegisterNotificationResponse>& pkt) {
   DEVICE_VLOG(1) << __func__ << ": interim=" << pkt->IsInterim();
+
+  if (!pkt->IsValid()) {
+    DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
+    auto response = RejectBuilder::MakeBuilder(pkt->GetCommandPdu(), Status::INVALID_PARAMETER);
+    send_message(label, false, std::move(response));
+    active_labels_.erase(label);
+    volume_interface_ = nullptr;
+    volume_ = VOL_REGISTRATION_FAILED;
+    return;
+  }
+
   if (volume_interface_ == nullptr) return;
 
   if (pkt->GetCType() == CType::REJECTED) {
@@ -546,6 +585,7 @@ void Device::GetElementAttributesResponse(
     uint8_t label, std::shared_ptr<GetElementAttributesRequest> pkt,
     SongInfo info) {
   DEVICE_VLOG(2) << __func__;
+
   auto get_element_attributes_pkt = pkt;
   auto attributes_requested =
       get_element_attributes_pkt->GetAttributesRequested();
@@ -570,10 +610,15 @@ void Device::GetElementAttributesResponse(
 }
 
 void Device::MessageReceived(uint8_t label, std::shared_ptr<Packet> pkt) {
-  DEVICE_VLOG(4) << __func__ << ": opcode=" << pkt->GetOpcode();
+  if (!pkt->IsValid()) {
+    DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
+    auto response = RejectBuilder::MakeBuilder(static_cast<CommandPdu>(0), Status::INVALID_COMMAND);
+    send_message(label, false, std::move(response));
+    return;
+  }
 
+  DEVICE_VLOG(4) << __func__ << ": opcode=" << pkt->GetOpcode();
   active_labels_.insert(label);
-
   switch (pkt->GetOpcode()) {
     // TODO (apanicke): Remove handling of UNIT_INFO and SUBUNIT_INFO from
     // the AVRC_API and instead handle it here to reduce fragmentation.
@@ -583,6 +628,14 @@ void Device::MessageReceived(uint8_t label, std::shared_ptr<Packet> pkt) {
     } break;
     case Opcode::PASS_THROUGH: {
       auto pass_through_packet = Packet::Specialize<PassThroughPacket>(pkt);
+
+      if (!pass_through_packet->IsValid()) {
+        DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
+        auto response = RejectBuilder::MakeBuilder(static_cast<CommandPdu>(0), Status::INVALID_COMMAND);
+        send_message(label, false, std::move(response));
+        return;
+      }
+
       auto response = PassThroughPacketBuilder::MakeBuilder(
           true, pass_through_packet->GetKeyState() == KeyState::PUSHED,
           pass_through_packet->GetOperationId());
@@ -633,6 +686,13 @@ void Device::HandlePlayItem(uint8_t label,
   DEVICE_VLOG(2) << __func__ << ": scope=" << pkt->GetScope()
                  << " uid=" << pkt->GetUid();
 
+  if (!pkt->IsValid()) {
+    DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
+    auto response = RejectBuilder::MakeBuilder(pkt->GetCommandPdu(), Status::INVALID_PARAMETER);
+    send_message(label, false, std::move(response));
+    return;
+  }
+
   std::string media_id = "";
   switch (pkt->GetScope()) {
     case Scope::NOW_PLAYING:
@@ -680,6 +740,13 @@ void Device::HandleSetAddressedPlayer(
 
 void Device::BrowseMessageReceived(uint8_t label,
                                    std::shared_ptr<BrowsePacket> pkt) {
+  if (!pkt->IsValid()) {
+    DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
+    auto response = GeneralRejectBuilder::MakeBuilder(Status::INVALID_COMMAND);
+    send_message(label, false, std::move(response));
+    return;
+  }
+
   DEVICE_VLOG(1) << __func__ << ": pdu=" << pkt->GetPdu();
 
   switch (pkt->GetPdu()) {
@@ -704,8 +771,7 @@ void Device::BrowseMessageReceived(uint8_t label,
       break;
     default:
       DEVICE_LOG(WARNING) << __func__ << ": " << pkt->GetPdu();
-      auto response = GeneralRejectBuilder::MakeBuilder(
-          BrowsePdu::GENERAL_REJECT, Status::INVALID_COMMAND);
+      auto response = GeneralRejectBuilder::MakeBuilder(Status::INVALID_COMMAND);
       send_message(label, true, std::move(response));
 
       break;
@@ -714,6 +780,15 @@ void Device::BrowseMessageReceived(uint8_t label,
 
 void Device::HandleGetFolderItems(uint8_t label,
                                   std::shared_ptr<GetFolderItemsRequest> pkt) {
+  if (!pkt->IsValid()) {
+    // The specific get folder items builder is unimportant on failure.
+    DEVICE_LOG(WARNING) << __func__ << ": Get folder items request packet is not valid";
+    auto response =
+        GetFolderItemsResponseBuilder::MakePlayerListBuilder(Status::INVALID_PARAMETER, 0x0000, browse_mtu_);
+    send_message(label, true, std::move(response));
+    return;
+  }
+
   DEVICE_VLOG(2) << __func__ << ": scope=" << pkt->GetScope();
 
   switch (pkt->GetScope()) {
@@ -735,12 +810,21 @@ void Device::HandleGetFolderItems(uint8_t label,
       break;
     default:
       DEVICE_LOG(ERROR) << __func__ << ": " << pkt->GetScope();
+      auto response = GetFolderItemsResponseBuilder::MakePlayerListBuilder(Status::INVALID_PARAMETER, 0, browse_mtu_);
+      send_message(label, true, std::move(response));
       break;
   }
 }
 
 void Device::HandleGetTotalNumberOfItems(
     uint8_t label, std::shared_ptr<GetTotalNumberOfItemsRequest> pkt) {
+  if (!pkt->IsValid()) {
+    DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
+    auto response = GetTotalNumberOfItemsResponseBuilder::MakeBuilder(Status::INVALID_PARAMETER, 0x0000, 0);
+    send_message(label, true, std::move(response));
+    return;
+  }
+
   DEVICE_VLOG(2) << __func__ << ": scope=" << pkt->GetScope();
 
   switch (pkt->GetScope()) {
@@ -796,6 +880,13 @@ void Device::GetTotalNumberOfItemsNowPlayingResponse(
 
 void Device::HandleChangePath(uint8_t label,
                               std::shared_ptr<ChangePathRequest> pkt) {
+  if (!pkt->IsValid()) {
+    DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
+    auto response = ChangePathResponseBuilder::MakeBuilder(Status::INVALID_PARAMETER, 0);
+    send_message(label, true, std::move(response));
+    return;
+  }
+
   DEVICE_VLOG(2) << __func__ << ": direction=" << pkt->GetDirection()
                  << " uid=" << loghex(pkt->GetUid());
 
@@ -846,6 +937,13 @@ void Device::ChangePathResponse(uint8_t label,
 
 void Device::HandleGetItemAttributes(
     uint8_t label, std::shared_ptr<GetItemAttributesRequest> pkt) {
+  if (!pkt->IsValid()) {
+    DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
+    auto builder = GetItemAttributesResponseBuilder::MakeBuilder(Status::INVALID_PARAMETER, browse_mtu_);
+    send_message(label, true, std::move(builder));
+    return;
+  }
+
   DEVICE_VLOG(2) << __func__ << ": scope=" << pkt->GetScope()
                  << " uid=" << loghex(pkt->GetUid())
                  << " uid counter=" << loghex(pkt->GetUidCounter());
@@ -856,6 +954,7 @@ void Device::HandleGetItemAttributes(
     send_message(label, true, std::move(builder));
     return;
   }
+
   switch (pkt->GetScope()) {
     case Scope::NOW_PLAYING: {
       media_interface_->GetNowPlayingList(
@@ -1121,6 +1220,13 @@ void Device::GetNowPlayingListResponse(
 
 void Device::HandleSetBrowsedPlayer(
     uint8_t label, std::shared_ptr<SetBrowsedPlayerRequest> pkt) {
+  if (!pkt->IsValid()) {
+    DEVICE_LOG(WARNING) << __func__ << ": Request packet is not valid";
+    auto response = SetBrowsedPlayerResponseBuilder::MakeBuilder(Status::INVALID_PARAMETER, 0x0000, 0, 0, "");
+    send_message(label, true, std::move(response));
+    return;
+  }
+
   DEVICE_VLOG(2) << __func__ << ": player_id=" << pkt->GetPlayerId();
   media_interface_->SetBrowsedPlayer(
       pkt->GetPlayerId(),

diff --git a/profile/avrcp/tests/avrcp_device_test.cc b/profile/avrcp/tests/avrcp_device_test.cc
index 83c6a13..c608da6 100644 (file)
--- a/profile/avrcp/tests/avrcp_device_test.cc
+++ b/profile/avrcp/tests/avrcp_device_test.cc
@@ -1358,6 +1358,126 @@ TEST_F(AvrcpDeviceTest, invalidRegisterNotificationTest) {
   SendMessage(1, reg_notif_request);
 }
 
+TEST_F(AvrcpDeviceTest, invalidVendorPacketTest) {
+  MockMediaInterface interface;
+  NiceMock<MockA2dpInterface> a2dp_interface;
+
+  test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);
+
+  auto rsp = RejectBuilder::MakeBuilder(static_cast<CommandPdu>(0), Status::INVALID_COMMAND);
+  EXPECT_CALL(response_cb, Call(1, false, matchPacket(std::move(rsp)))).Times(1);
+  auto short_packet = TestAvrcpPacket::Make(short_vendor_packet);
+  SendMessage(1, short_packet);
+}
+
+TEST_F(AvrcpDeviceTest, invalidCapabilitiesPacketTest) {
+  MockMediaInterface interface;
+  NiceMock<MockA2dpInterface> a2dp_interface;
+
+  test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);
+
+  auto rsp = RejectBuilder::MakeBuilder(CommandPdu::GET_CAPABILITIES, Status::INVALID_PARAMETER);
+  EXPECT_CALL(response_cb, Call(1, false, matchPacket(std::move(rsp)))).Times(1);
+  auto short_packet = TestAvrcpPacket::Make(short_get_capabilities_request);
+  SendMessage(1, short_packet);
+}
+
+TEST_F(AvrcpDeviceTest, invalidGetElementAttributesPacketTest) {
+  MockMediaInterface interface;
+  NiceMock<MockA2dpInterface> a2dp_interface;
+
+  test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);
+
+  auto rsp = RejectBuilder::MakeBuilder(CommandPdu::GET_ELEMENT_ATTRIBUTES, Status::INVALID_PARAMETER);
+  EXPECT_CALL(response_cb, Call(1, false, matchPacket(std::move(rsp)))).Times(1);
+  auto short_packet = TestAvrcpPacket::Make(short_get_element_attributes_request);
+  SendMessage(1, short_packet);
+}
+
+TEST_F(AvrcpDeviceTest, invalidPlayItemPacketTest) {
+  MockMediaInterface interface;
+  NiceMock<MockA2dpInterface> a2dp_interface;
+
+  test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);
+
+  auto rsp = RejectBuilder::MakeBuilder(CommandPdu::PLAY_ITEM, Status::INVALID_PARAMETER);
+  EXPECT_CALL(response_cb, Call(1, false, matchPacket(std::move(rsp)))).Times(1);
+  auto short_packet = TestAvrcpPacket::Make(short_play_item_request);
+  SendMessage(1, short_packet);
+}
+
+TEST_F(AvrcpDeviceTest, invalidSetAddressedPlayerPacketTest) {
+  MockMediaInterface interface;
+  NiceMock<MockA2dpInterface> a2dp_interface;
+
+  test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);
+
+  auto rsp = RejectBuilder::MakeBuilder(CommandPdu::SET_ADDRESSED_PLAYER, Status::INVALID_PARAMETER);
+  EXPECT_CALL(response_cb, Call(1, false, matchPacket(std::move(rsp)))).Times(1);
+  auto short_packet = TestAvrcpPacket::Make(short_set_addressed_player_request);
+  SendMessage(1, short_packet);
+}
+
+TEST_F(AvrcpDeviceTest, invalidBrowsePacketTest) {
+  MockMediaInterface interface;
+  NiceMock<MockA2dpInterface> a2dp_interface;
+
+  test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);
+
+  auto rsp = GeneralRejectBuilder::MakeBuilder(Status::INVALID_COMMAND);
+  EXPECT_CALL(response_cb, Call(1, false, matchPacket(std::move(rsp)))).Times(1);
+  auto short_packet = TestBrowsePacket::Make(short_browse_packet);
+  SendBrowseMessage(1, short_packet);
+}
+
+TEST_F(AvrcpDeviceTest, invalidGetFolderItemsPacketTest) {
+  MockMediaInterface interface;
+  NiceMock<MockA2dpInterface> a2dp_interface;
+
+  test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);
+
+  auto rsp = GetFolderItemsResponseBuilder::MakePlayerListBuilder(Status::INVALID_PARAMETER, 0x0000, 0xFFFF);
+  EXPECT_CALL(response_cb, Call(1, true, matchPacket(std::move(rsp)))).Times(1);
+  auto short_packet = TestBrowsePacket::Make(short_get_folder_items_request);
+  SendBrowseMessage(1, short_packet);
+}
+
+TEST_F(AvrcpDeviceTest, invalidGetTotalNumberOfItemsPacketTest) {
+  MockMediaInterface interface;
+  NiceMock<MockA2dpInterface> a2dp_interface;
+
+  test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);
+
+  auto rsp = GetTotalNumberOfItemsResponseBuilder::MakeBuilder(Status::INVALID_PARAMETER, 0x0000, 0xFFFF);
+  EXPECT_CALL(response_cb, Call(1, true, matchPacket(std::move(rsp)))).Times(1);
+  auto short_packet = TestBrowsePacket::Make(short_get_total_number_of_items_request);
+  SendBrowseMessage(1, short_packet);
+}
+
+TEST_F(AvrcpDeviceTest, invalidChangePathPacketTest) {
+  MockMediaInterface interface;
+  NiceMock<MockA2dpInterface> a2dp_interface;
+
+  test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);
+
+  auto rsp = ChangePathResponseBuilder::MakeBuilder(Status::INVALID_PARAMETER, 0);
+  EXPECT_CALL(response_cb, Call(1, true, matchPacket(std::move(rsp)))).Times(1);
+  auto short_packet = TestBrowsePacket::Make(short_change_path_request);
+  SendBrowseMessage(1, short_packet);
+}
+
+TEST_F(AvrcpDeviceTest, invalidGetItemAttributesPacketTest) {
+  MockMediaInterface interface;
+  NiceMock<MockA2dpInterface> a2dp_interface;
+
+  test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);
+
+  auto rsp = GetItemAttributesResponseBuilder::MakeBuilder(Status::INVALID_PARAMETER, 0xFFFF);
+  EXPECT_CALL(response_cb, Call(1, true, matchPacket(std::move(rsp)))).Times(1);
+  auto short_packet = TestBrowsePacket::Make(short_get_item_attributes_request);
+  SendBrowseMessage(1, short_packet);
+}
+
 }  // namespace avrcp
 }  // namespace bluetooth