[bt][l2cap] Check size of command reject payload
Check size of command reject payload before parsing.
Create InvalidCIDPayload struct for CommandRejectPayload payload.
Bug: 50625
Test: in bt-host-unittests
L2CAP_CommandHandlerTest.OutboundDisconReqRej
L2CAP_CommandHandlerTest.OutboundDisconReqRejNotEnoughBytes
L2CAP_CommandHandlerTest.OutboundDisconReqRejInvalidCIDNotEnoughBytes
Change-Id: I12a5e78174ea31abc2794e1c8f8f0a86bf23bd6d
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/383404
Reviewed-by: Xo Wang <xow@google.com>
Testability-Review: Xo Wang <xow@google.com>
Commit-Queue: Ben Lawson <benlawson@google.com>
diff --git a/src/connectivity/bluetooth/core/bt-host/l2cap/command_handler.cc b/src/connectivity/bluetooth/core/bt-host/l2cap/command_handler.cc
index ea665fb..f02cf1d 100644
--- a/src/connectivity/bluetooth/core/bt-host/l2cap/command_handler.cc
+++ b/src/connectivity/bluetooth/core/bt-host/l2cap/command_handler.cc
@@ -3,19 +3,27 @@
namespace bt::l2cap::internal {
bool CommandHandler::Response::ParseReject(const ByteBuffer& rej_payload_buf) {
+ if (rej_payload_buf.size() < sizeof(CommandRejectPayload)) {
+ bt_log(TRACE, "l2cap", "cmd: ignoring malformed Command Reject, size %zu (expected >= %zu)",
+ rej_payload_buf.size(), sizeof(CommandRejectPayload));
+ return false;
+ }
auto& rej_payload = rej_payload_buf.As<CommandRejectPayload>();
reject_reason_ = static_cast<RejectReason>(letoh16(rej_payload.reason));
+
if (reject_reason() == RejectReason::kInvalidCID) {
- if (rej_payload_buf.size() - sizeof(CommandRejectPayload) < 4) {
+ if (rej_payload_buf.size() - sizeof(CommandRejectPayload) < sizeof(InvalidCIDPayload)) {
bt_log(TRACE, "l2cap",
"cmd: ignoring malformed Command Reject Invalid Channel ID, size %zu (expected %zu)",
- rej_payload_buf.size(), sizeof(CommandRejectPayload) + 4);
+ rej_payload_buf.size(), sizeof(CommandRejectPayload) + sizeof(InvalidCIDPayload));
return false;
}
-
- remote_cid_ = (rej_payload.data[1] << 8) + rej_payload.data[0];
- local_cid_ = (rej_payload.data[3] << 8) + rej_payload.data[2];
+ auto& invalid_cid_payload =
+ rej_payload_buf.view(sizeof(CommandRejectPayload)).As<InvalidCIDPayload>();
+ remote_cid_ = letoh16(invalid_cid_payload.src_cid);
+ local_cid_ = letoh16(invalid_cid_payload.dst_cid);
}
+
return true;
}
diff --git a/src/connectivity/bluetooth/core/bt-host/l2cap/command_handler_unittest.cc b/src/connectivity/bluetooth/core/bt-host/l2cap/command_handler_unittest.cc
index 611ffcb..37fa00c 100644
--- a/src/connectivity/bluetooth/core/bt-host/l2cap/command_handler_unittest.cc
+++ b/src/connectivity/bluetooth/core/bt-host/l2cap/command_handler_unittest.cc
@@ -158,7 +158,35 @@
EXPECT_TRUE(cb_called);
}
-TEST_F(L2CAP_CommandHandlerTest, OutboundDisconReqRejNotEnoughBytesInRejection) {
+TEST_F(L2CAP_CommandHandlerTest, OutboundDisconReqRejNotEnoughBytes) {
+ constexpr ChannelId kBadLocalCId = 0x0005; // Not a dynamic channel
+
+ // Disconnect Request payload
+ auto expected_discon_req = StaticByteBuffer(
+ // Destination CID
+ LowerBits(kRemoteCId), UpperBits(kRemoteCId),
+
+ // Source CID
+ LowerBits(kBadLocalCId), UpperBits(kBadLocalCId));
+
+ // Invalid Command Reject payload (size is too small)
+ auto rej_rsp = StaticByteBuffer(0x01);
+
+ EXPECT_OUTBOUND_REQ(*fake_sig(), kDisconnectionRequest, expected_discon_req.view(),
+ {SignalingChannel::Status::kReject, rej_rsp.view()});
+
+ bool cb_called = false;
+ auto on_discon_rsp = [&cb_called](const CommandHandler::DisconnectionResponse& rsp) {
+ cb_called = true;
+ };
+
+ EXPECT_TRUE(
+ cmd_handler()->SendDisconnectionRequest(kRemoteCId, kBadLocalCId, std::move(on_discon_rsp)));
+ RunLoopUntilIdle();
+ EXPECT_FALSE(cb_called);
+}
+
+TEST_F(L2CAP_CommandHandlerTest, OutboundDisconReqRejInvalidCIDNotEnoughBytes) {
constexpr ChannelId kBadLocalCId = 0x0005; // Not a dynamic channel
// Disconnect Request payload
diff --git a/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap.h b/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap.h
index e8ea53e..93c8fbd 100644
--- a/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap.h
+++ b/src/connectivity/bluetooth/core/bt-host/l2cap/l2cap.h
@@ -279,6 +279,15 @@
uint8_t data[];
} __PACKED;
+// Payload of Command Reject (see Vol 3, Part A, Section 4.1).
+struct InvalidCIDPayload {
+ // Source CID (relative to rejecter)
+ ChannelId src_cid;
+
+ // Destination CID (relative to rejecter)
+ ChannelId dst_cid;
+} __PACKED;
+
// ACL-U
constexpr CommandCode kConnectionRequest = 0x02;
struct ConnectionRequestPayload {