From 204780849d39ffd1d6b078b1d739b1e7281377b4 Mon Sep 17 00:00:00 2001 From: Pradip De Date: Thu, 6 Jun 2024 20:05:42 -0700 Subject: [PATCH] Close TCP connection when received message size is too large. (#33768) When the framing length value of a received message is larger than what the local node can process, abort the connection with the peer. Sending a StatusResponse message back to the peer as a notification may not be feasible in all circumstances for reasons, such as: 1) It would require a cross-layered feedback up to the Exchange layer to generate such a message in response to a failure at the transport layer. 2) A Status Response is sent in response to a message on an ExchangeContext and that may not be the case in scnearios where this message is the first unsolicited message. The receiver could drain out the bits from the offending message and move on to the next message in the stream but that may not guarantee correct behavior and would consume resources unnecessarily. Given that the peer was already aware of the max length this node was willing to receive during its TCP advertisement, it seems prudent to fail fast and close the connection. Fixes #33307. --- src/transport/raw/TCP.cpp | 5 ++++- src/transport/raw/tests/TestTCP.cpp | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/transport/raw/TCP.cpp b/src/transport/raw/TCP.cpp index b928c3e91cccae..3c3e6da15c2b58 100644 --- a/src/transport/raw/TCP.cpp +++ b/src/transport/raw/TCP.cpp @@ -331,7 +331,10 @@ CHIP_ERROR TCPBase::ProcessReceivedBuffer(Inet::TCPEndPoint * endPoint, const Pe uint32_t messageSize = LittleEndian::Get32(messageSizeBuf); if (messageSize >= kMaxTCPMessageSize) { - // This message is too long for upper layers. + // Message is too big for this node to process. Disconnect from peer. + ChipLogError(Inet, "Received TCP message of length %" PRIu32 " exceeds limit.", messageSize); + CloseConnectionInternal(state, CHIP_ERROR_MESSAGE_TOO_LONG, SuppressCallback::No); + return CHIP_ERROR_MESSAGE_TOO_LONG; } // The subtraction will not underflow because we successfully read kPacketSizeBytes. diff --git a/src/transport/raw/tests/TestTCP.cpp b/src/transport/raw/tests/TestTCP.cpp index dbce3b8ce5ef43..4f78da5dcdce86 100644 --- a/src/transport/raw/tests/TestTCP.cpp +++ b/src/transport/raw/tests/TestTCP.cpp @@ -682,7 +682,9 @@ TEST_F(TestTCP, CheckProcessReceivedBuffer) EXPECT_EQ(err, CHIP_ERROR_MESSAGE_TOO_LONG); EXPECT_EQ(gMockTransportMgrDelegate.mReceiveHandlerCallCount, 0); - gMockTransportMgrDelegate.DisconnectTest(tcp, addr); + // The receipt of a message exceeding the allowed size should have + // closed the connection. + EXPECT_EQ(TestAccess::GetEndpoint(state), nullptr); } } // namespace