From a1d6ceef620f3603259f3b66a10dac57fe5dcadb Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Wed, 29 Nov 2017 15:39:26 -0800
Subject: [PATCH] [ncp-buffer] limit number of segments in a frame (being read)
 (#2377)

This commit adds code in `NcpFrameBuffer` to check and limit the
number of segments in a frame (being read). This is to help detect
cases where the underlying buffer may get corrupted.
---
 src/ncp/ncp_buffer.cpp | 16 ++++++++++++++++
 src/ncp/ncp_buffer.hpp |  1 +
 2 files changed, 17 insertions(+)

diff --git a/src/ncp/ncp_buffer.cpp b/src/ncp/ncp_buffer.cpp
index 3f3413fa3..6097628cb 100644
--- a/src/ncp/ncp_buffer.cpp
+++ b/src/ncp/ncp_buffer.cpp
@@ -772,6 +772,7 @@ otError NcpFrameBuffer::OutFrameRemove(void)
     uint8_t *bufPtr;
     otMessage *message;
     uint16_t header;
+    uint8_t numSegments;
     FrameTag tag;
 
     VerifyOrExit(!IsEmpty(), error = OT_ERROR_NOT_FOUND);
@@ -784,6 +785,7 @@ otError NcpFrameBuffer::OutFrameRemove(void)
     // Begin at the start of current frame and move through all segments.
 
     bufPtr = mReadFrameStart[mReadDirection];
+    numSegments = 0;
 
     while (bufPtr != mWriteFrameStart[mReadDirection])
     {
@@ -812,6 +814,12 @@ otError NcpFrameBuffer::OutFrameRemove(void)
 
         // Move the pointer to next segment.
         bufPtr = GetUpdatedBufPtr(bufPtr, kSegmentHeaderSize + (header & kSegmentHeaderLengthMask), mReadDirection);
+
+        numSegments++;
+
+        // If this assert fails, it is a likely indicator that the internal structure of the NCP buffer has been
+        // corrupted.
+        assert(numSegments <= kMaxSegments);
     }
 
     mReadFrameStart[mReadDirection] = bufPtr;
@@ -858,6 +866,7 @@ uint16_t NcpFrameBuffer::OutFrameGetLength(void)
     uint16_t frameLength = 0;
     uint16_t header;
     uint8_t *bufPtr;
+    uint8_t numSegments;
     otMessage *message = NULL;
 
     // If the frame length was calculated before, return the previously calculated length.
@@ -870,6 +879,7 @@ uint16_t NcpFrameBuffer::OutFrameGetLength(void)
     // Calculate frame length by adding length of all segments and messages within the current frame.
 
     bufPtr = mReadFrameStart[mReadDirection];
+    numSegments = 0;
 
     while (bufPtr != mWriteFrameStart[mReadDirection])
     {
@@ -904,6 +914,12 @@ uint16_t NcpFrameBuffer::OutFrameGetLength(void)
 
         // Move the pointer to next segment.
         bufPtr = GetUpdatedBufPtr(bufPtr, kSegmentHeaderSize + (header & kSegmentHeaderLengthMask), mReadDirection);
+
+        numSegments++;
+
+        // If this assert fails, it is a likely indicator that the internal structure of the NCP buffer has been
+        // corrupted.
+        assert(numSegments <= kMaxSegments);
     }
 
     // Remember the calculated frame length for current active frame.
diff --git a/src/ncp/ncp_buffer.hpp b/src/ncp/ncp_buffer.hpp
index 2fe982635..f199bb058 100644
--- a/src/ncp/ncp_buffer.hpp
+++ b/src/ncp/ncp_buffer.hpp
@@ -571,6 +571,7 @@ private:
         kUnknownFrameLength                = 0xffff,     // Value used when frame length is unknown.
         kSegmentHeaderSize                 = 2,          // Length of the segment header.
         kSegmentHeaderLengthMask           = 0x3fff,     // Bit mask to get the length from the segment header
+        kMaxSegments                       = 10,         // Max number of segments allowed in a frame
 
         kSegmentHeaderNoFlag               = 0,          // No flags are set.
         kSegmentHeaderNewFrameFlag         = (1 << 15),  // Indicates that this segment starts a new frame.