From b8423e2064e3a5cbfa6cf7ec90c601800990cbe6 Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Fri, 10 Sep 2021 13:47:52 -0400
Subject: [PATCH] Ensure that stack layers above transport manager never get
 chained packet buffers.

None of our upper-layer core really deals with chained packet buffers,
starting with the way we do in-place decryption that assumes that we
have a contiguous buffer.

The changes here:

1) Flatten out possibly-chained buffers in the LwIP case in the UDP
   endpoint.
2) Ensure that we don't have a chained buffer in BTP.  There was
   already a check for this in the "in progress" case; this change just
   adds the same check in the "idle" case.
---
 src/ble/BtpEngine.cpp              |  4 ++++
 src/inet/UDPEndPoint.cpp           | 18 ++++++++++++++++++
 src/transport/TransportMgrBase.cpp |  9 +++++++++
 3 files changed, 31 insertions(+)

diff --git a/src/ble/BtpEngine.cpp b/src/ble/BtpEngine.cpp
index 5c36bc354177ec..6c5e561b96ad3b 100644
--- a/src/ble/BtpEngine.cpp
+++ b/src/ble/BtpEngine.cpp
@@ -321,6 +321,10 @@ CHIP_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle &&
 
         mRxBuf->AddToEnd(std::move(data));
         mRxBuf->CompactHead(); // will free 'data' and adjust rx buf's end/length
+
+        // For now, limit BtpEngine message size to max length of 1 pbuf, as we do for chip messages sent via IP.
+        // TODO add support for BtpEngine messages longer than 1 pbuf
+        VerifyOrExit(!mRxBuf->HasChainedBuffer(), err = CHIP_ERROR_INBOUND_MESSAGE_TOO_BIG);
     }
     else if (mRxState == kState_InProgress)
     {
diff --git a/src/inet/UDPEndPoint.cpp b/src/inet/UDPEndPoint.cpp
index 4f114b8e5db68c..60bd3f841083a8 100644
--- a/src/inet/UDPEndPoint.cpp
+++ b/src/inet/UDPEndPoint.cpp
@@ -864,6 +864,24 @@ void UDPEndPoint::LwIPReceiveUDPMessage(void * arg, struct udp_pcb * pcb, struct
     System::LayerLwIP * lSystemLayer = static_cast<System::LayerLwIP *>(ep->Layer().SystemLayer());
     IPPacketInfo * pktInfo           = NULL;
     System::PacketBufferHandle buf   = System::PacketBufferHandle::Adopt(p);
+    if (buf->HasChainedBuffer())
+    {
+        // Try the simple expedient of flattening in-place.
+        buf->CompactHead();
+    }
+
+    if (buf->HasChainedBuffer())
+    {
+        // Have to allocate a new big-enough buffer and copy.
+        uint16_t messageSize            = buf->TotalLength();
+        System::PacketBufferHandle copy = System::PacketBufferHandle::New(messageSize, 0);
+        if (copy.IsNull() || buf->Read(copy->Start(), messageSize) != CHIP_NO_ERROR)
+        {
+            ChipLogError(Inet, "No memory to flatten incoming packet buffer chain of size %" PRIu16, buf->TotalLength());
+            return;
+        }
+        buf = std::move(copy);
+    }
 
     pktInfo = GetPacketInfo(buf);
     if (pktInfo != NULL)
diff --git a/src/transport/TransportMgrBase.cpp b/src/transport/TransportMgrBase.cpp
index 868d54732405b2..a88e116377bedf 100644
--- a/src/transport/TransportMgrBase.cpp
+++ b/src/transport/TransportMgrBase.cpp
@@ -52,6 +52,15 @@ void TransportMgrBase::Close()
 
 void TransportMgrBase::HandleMessageReceived(const Transport::PeerAddress & peerAddress, System::PacketBufferHandle && msg)
 {
+    if (msg->HasChainedBuffer())
+    {
+        // Something in the lower levels messed up.
+        char addrBuffer[Transport::PeerAddress::kMaxToStringSize];
+        peerAddress.ToString(addrBuffer);
+        ChipLogError(Inet, "message from %s dropped due to lower layers not ensuring a single packet buffer.", addrBuffer);
+        return;
+    }
+
     if (mSecureSessionMgr != nullptr)
     {
         mSecureSessionMgr->OnMessageReceived(peerAddress, std::move(msg));