Fix integer overflow in QPACKDecoder.cpp (T259295275)

Reviewed By: afrind

Differential Revision: D103181611

fbshipit-source-id: 999c479ac143146850da9050e554948b8495d419

Author: sandarsh

proxygen/lib/http/codec/compress/QPACKDecoder.cpp

@@ -71,13 +71,19 @@ uint32_t QPACKDecoder::decodePrefix(HPACKDecodeBuffer& dbuf) {
     err_ = HPACK::DecodeError::INVALID_INDEX;
     return 0;
   } else {
+    if (wireRIC > fullRange) {
+      LOG(ERROR) << "Decode error RIC out of range=" << wireRIC
+                 << " fullRange=" << fullRange;
+      err_ = HPACK::DecodeError::INVALID_INDEX;
+      return 0;
+    }
     uint64_t maxValue = table_.getInsertCount() + maxEntries;
     uint64_t maxWrapped = (maxValue / fullRange) * fullRange;
     requiredInsertCount = maxWrapped + wireRIC - 1;
     // If requiredInsertCount exceeds maxValue, the Encoder's value must have
     // wrapped one fewer time
     if (requiredInsertCount > maxValue) {
-      if (wireRIC > fullRange || requiredInsertCount < fullRange) {
+      if (requiredInsertCount < fullRange) {
         LOG(ERROR) << "Decode error RIC out of range=" << wireRIC;
         err_ = HPACK::DecodeError::INVALID_INDEX;
         return 0;

proxygen/lib/http/codec/compress/test/QPACKContextTests.cpp

@@ -10,7 +10,9 @@
 #include <folly/Format.h>
 #include <folly/portability/GTest.h>
 #include <glog/logging.h>
+#include <limits>
 #include <memory>
+#include <proxygen/lib/http/codec/compress/HPACKEncodeBuffer.h>
 #include <proxygen/lib/http/codec/compress/Logging.h>
 #include <proxygen/lib/http/codec/compress/QPACKDecoder.h>
 #include <proxygen/lib/http/codec/compress/QPACKEncoder.h>
@@ -858,6 +860,28 @@ void checkQError(QPACKDecoder& decoder,
   EXPECT_EQ(cb->error, err);
 }
 
+TEST(QPACKContextTests, RejectsOverflowingRequiredInsertCount) {
+  QPACKDecoder decoder;
+  HPACKEncodeBuffer insertStream(128, false);
+  const uint32_t maxEntries = HPACK::kTableSize / HPACKHeader::kMinLength;
+  // Prime insertCount so RIC reconstruction uses a non-zero maxWrapped value.
+  for (uint32_t i = 0; i < maxEntries; i++) {
+    insertStream.encodeLiteral(HPACK::Q_INSERT_NO_NAME_REF.code,
+                               HPACK::Q_INSERT_NO_NAME_REF.prefixLength,
+                               folly::to<string>("x-overflow-", i));
+    insertStream.encodeLiteral("v");
+  }
+  EXPECT_EQ(decoder.decodeEncoderStream(insertStream.release()),
+            HPACK::DecodeError::NONE);
+
+  HPACKEncodeBuffer headerBlock(128, false);
+  // This value used to overflow maxWrapped + wireRIC - 1 before validation.
+  headerBlock.encodeInteger(std::numeric_limits<uint64_t>::max() - 154);
+  headerBlock.encodeInteger(0, HPACK::Q_DELTA_BASE);
+  checkQError(
+      decoder, headerBlock.release(), HPACK::DecodeError::INVALID_INDEX);
+}
+
 TEST(QPACKContextTests, DecodeErrors) {
   QPACKDecoder decoder(128);
   unique_ptr<IOBuf> buf = IOBuf::create(128);