From 51ec405a2491b86e353d95033578de5073963547 Mon Sep 17 00:00:00 2001
From: Mate Barany <mate.barany@qt.io>
Date: Mon, 17 Aug 2026 16:01:28 +0200
Subject: [PATCH] Add bound checks to the CoAP option parser

The option-parsing loop in createFromFrame lacks bound checks.
- The loop condition used "i != reply.size()". Because the index can
  advance by more than one per iteration (extended delta/length bytes
  and the option value length), it could step over reply.size() without
  ever equalling it, leaving the condition true and reading pduData[i]
  out of bounds.
- The extended delta/length branches did "++i" and then read pduData[i]
  without checking that the extra byte was actually present.
- The option value length was not validated against the remaining
  buffer, so an option claiming more bytes than were sent was accepted
  with a silently truncated value. QByteArray::mid() clamps rather than
  reading out of bounds, so this is an integrity problem rather than a
  crash, but the resulting reply is delivered as if it were complete.

Change the loop condition to "i < reply.size()", reject the frame
(return nullptr) when an extended length byte or an option value would
run past the end of the buffer, and make internalReply a std::unique_ptr
so these mid-loop rejections do not leak the partially built reply.

Task-number: QTBUG-149345
Pick-to: 6.8 6.5 5.15
Change-Id: I609145015127ac9fcf87880fcfbeee74b4c22556
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
(cherry picked from commit 73020a686d9a8d92ac8cd8914f2530a70390dd3a)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 44451a90e87d3cd2865e7d5e2e38f6d097cfda9a)
---

diff --git a/src/coap/qcoapinternalreply.cpp b/src/coap/qcoapinternalreply.cpp
index a1d67af..3ecd16b 100644
--- a/src/coap/qcoapinternalreply.cpp
+++ b/src/coap/qcoapinternalreply.cpp
@@ -6,6 +6,8 @@
 #include "qcoapinternalreply_p.h"
 #include <QtCore/qmath.h>
 
+#include <memory>
+
 QT_BEGIN_NAMESPACE
 
 /*!
@@ -55,7 +57,7 @@
     if (reply.size() < 4)
         return nullptr;
 
-    QCoapInternalReply *internalReply = new QCoapInternalReply(parent);
+    auto internalReply = std::make_unique<QCoapInternalReply>(parent);
     QCoapInternalReplyPrivate *d = internalReply->d_func();
 
     const quint8 *pduData = reinterpret_cast<const quint8 *>(reply.data());
@@ -70,30 +72,39 @@
     d->message.setToken(reply.mid(4, tokenLength));
 
     // Parse Options
-    int i = 4 + tokenLength;
+    qsizetype i = 4 + tokenLength;
     quint16 lastOptionNumber = 0;
-    while (i != reply.size() && pduData[i] != 0xFF) {
+    while (i < reply.size() && pduData[i] != 0xFF) {
         quint16 optionDelta = ((pduData[i] >> 4) & 0x0F);
         quint16 optionLength = (pduData[i] & 0x0F);
 
         // Delta value > 12 : special values
         if (optionDelta == 13) {
-            ++i;
+            if (++i >= reply.size())
+                return nullptr;
             optionDelta = pduData[i] + 13;
         } else if (optionDelta == 14) {
-            ++i;
+            if (++i >= reply.size())
+                return nullptr;
             optionDelta = pduData[i] + 269;
         }
 
         // Delta length > 12 : special values
         if (optionLength == 13) {
-            ++i;
+            if (++i >= reply.size())
+                return nullptr;
             optionLength = pduData[i] + 13;
         } else if (optionLength == 14) {
-            ++i;
+            if (++i >= reply.size())
+                return nullptr;
             optionLength = pduData[i] + 269;
         }
 
+        // The option value is optionLength bytes following the current byte.
+        // Reject the frame if it would run past the end of the buffer.
+        if (i >= reply.size() - optionLength)
+            return nullptr;
+
         quint16 optionNumber = lastOptionNumber + optionDelta;
         internalReply->addOption(QCoapOption::OptionName(optionNumber),
                                  reply.mid(i + 1, optionLength));
@@ -108,7 +119,7 @@
         d->message.setPayload(d->message.payload().append(currentPayload));
     }
 
-    return internalReply;
+    return internalReply.release();
 }
 
 /*!
diff --git a/tests/auto/qcoapinternalreply/tst_qcoapinternalreply.cpp b/tests/auto/qcoapinternalreply/tst_qcoapinternalreply.cpp
index cab3402..bf32363 100644
--- a/tests/auto/qcoapinternalreply/tst_qcoapinternalreply.cpp
+++ b/tests/auto/qcoapinternalreply/tst_qcoapinternalreply.cpp
@@ -150,6 +150,20 @@
     QTest::newRow("one_byte")    << QByteArray::fromHex("50");
     QTest::newRow("two_bytes")   << QByteArray::fromHex("5045");
     QTest::newRow("three_bytes") << QByteArray::fromHex("5045fb");
+
+    // Option-parser cases: each frame has a valid 4-byte header (no token)
+    // followed by a single option byte whose encoding demands bytes that are
+    // not present. createFromFrame() must reject the frame rather than read
+    // past the buffer or deliver a silently truncated option.
+
+    // 0x09: option value length 9, but no value bytes follow it.
+    QTest::newRow("option_value_overruns_buffer")   << QByteArray::fromHex("50450001" "09");
+    // 0xd0: delta nibble 13 promises one extended delta byte that is missing.
+    QTest::newRow("missing_option_delta_ext_byte")  << QByteArray::fromHex("50450001" "d0");
+    // 0x0d: length nibble 13 promises one extended length byte that is missing.
+    QTest::newRow("missing_option_length_ext_byte") << QByteArray::fromHex("50450001" "0d");
+    // 0x0d + 0xff: extended length yields optionLength 268, far past the buffer.
+    QTest::newRow("extended_option_length_overruns") << QByteArray::fromHex("50450001" "0d" "ff");
 }
 
 void tst_QCoapInternalReply::parseInvalidReplyPdu()