[WiFi-PAF][Bugfix] heap-buffer overflow in GetPktSn (project-chip#71945) (project-chip#72333)

* [WiFi-PAF]: validate Read8 status and SnOffset bounds in GetPktSn

* Porting fix to a debug method: DebugPktAckSn

* copilot comment

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------


(cherry picked from commit d2f6c22)

Co-authored-by: Amine Alami <43780877+Alami-Amine@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: 3 people

src/wifipaf/WiFiPAFEndPoint.cpp

@@ -812,11 +812,10 @@ SequenceNumber_t WiFiPAFEndPoint::AdjustRemoteReceiveWindow(SequenceNumber_t las
 
 CHIP_ERROR WiFiPAFEndPoint::GetPktSn(Encoding::LittleEndian::Reader & reader, uint8_t * pHead, SequenceNumber_t & seqNum)
 {
-    CHIP_ERROR err;
     BitFlags<WiFiPAFTP::HeaderFlags> rx_flags;
     size_t SnOffset = 0;
     SequenceNumber_t * pSn;
-    err = reader.Read8(rx_flags.RawStorage()).StatusCode();
+    ReturnErrorOnFailure(reader.Read8(rx_flags.RawStorage()).StatusCode());
     if (rx_flags.Has(WiFiPAFTP::HeaderFlags::kHankshake))
     {
         // Handkshake message => No ack/sn
@@ -832,6 +831,7 @@ CHIP_ERROR WiFiPAFEndPoint::GetPktSn(Encoding::LittleEndian::Reader & reader, ui
     {
         SnOffset += kTransferProtocolAckSize;
     }
+    VerifyOrReturnError(SnOffset + sizeof(seqNum) <= reader.OctetsRead() + reader.Remaining(), CHIP_ERROR_MESSAGE_INCOMPLETE);
     pSn    = pHead + SnOffset;
     seqNum = *pSn;
 
@@ -866,6 +866,7 @@ CHIP_ERROR WiFiPAFEndPoint::DebugPktAckSn(const PktDirect_t PktDirect, Encoding:
         pAct = pHead + kTransferProtocolHeaderFlagsSize;
         SnOffset += kTransferProtocolAckSize;
     }
+    VerifyOrExit(SnOffset + sizeof(*pSn) <= reader.OctetsRead() + reader.Remaining(), err = CHIP_ERROR_MESSAGE_INCOMPLETE);
     pSn = pHead + SnOffset;
     if (pAct == nullptr)
     {

src/wifipaf/tests/TestWiFiPAFLayer.cpp

@@ -183,6 +183,37 @@ TEST_F(TestWiFiPAFLayer, CheckPafSession)
     EXPECT_EQ(RmPafSession(PafInfoAccess::kAccSessionId, sessionInfo), CHIP_ERROR_NOT_FOUND);
 }
 
+// Run under ASan to catch regressions of the heap-buffer-overflow read.
+TEST_F(TestWiFiPAFLayer, GetPktSnRejectsShortFragment)
+{
+    WiFiPAFSession sessionInfo = {
+        .role          = kWiFiPafRole_Publisher,
+        .id            = 1,
+        .peer_id       = 1,
+        .peer_addr     = { 0xd0, 0x17, 0x69, 0xee, 0x7f, 0x3c },
+        .nodeId        = 1,
+        .discriminator = 0xF00,
+    };
+
+    WiFiPAFEndPoint * newEndPoint = nullptr;
+    ASSERT_EQ(NewEndPoint(&newEndPoint, sessionInfo, sessionInfo.role), CHIP_NO_ERROR);
+    ASSERT_NE(newEndPoint, nullptr);
+    SetEndPoint(newEndPoint);
+    EXPECT_EQ(AddPafSession(PafInfoAccess::kAccSessionId, sessionInfo), CHIP_NO_ERROR);
+    newEndPoint->mState = WiFiPAFEndPoint::kState_Ready;
+
+    // Flag byte 0x2B sets kFragmentAck and kManagementOpcode, pushing SnOffset
+    // to 3 (header + mgmt-op + ack). With a 3-byte buffer, dereferencing
+    // pHead + 3 is a heap-buffer-overflow read.
+    constexpr uint8_t kShortFragmentWithMaxFlags[] = { 0x2B, 0x8F, 0x2B };
+    auto packet = System::PacketBufferHandle::NewWithData(kShortFragmentWithMaxFlags, sizeof(kShortFragmentWithMaxFlags));
+    ASSERT_FALSE(packet.IsNull());
+
+    EXPECT_EQ(newEndPoint->Receive(std::move(packet)), CHIP_ERROR_MESSAGE_INCOMPLETE);
+
+    EXPECT_EQ(RmPafSession(PafInfoAccess::kAccSessionId, sessionInfo), CHIP_NO_ERROR);
+}
+
 TEST_F(TestWiFiPAFLayer, CheckRunAsCommissioner)
 {
     WiFiPAFSession sessionInfo = {