QPACK: use || in the varint decode range guards

src/proxy/http3/QPACK.cc

@@ -284,7 +284,7 @@ QPACK::decode(uint64_t stream_id, const uint8_t *header_block, size_t header_blo
 
   uint64_t tmp = 0;
   int64_t  ret = xpack_decode_integer(tmp, header_block, header_block + header_block_len, 8);
-  if (ret < 0 && tmp > 0xFFFF) {
+  if (ret < 0 || tmp > 0xFFFF) {
     return -1;
   }
   uint16_t largest_reference = tmp;
@@ -922,7 +922,7 @@ QPACK::_decode_header(const uint8_t *header_block, size_t header_block_len, HTTP
 
   // Decode Header Data Prefix
   uint64_t tmp;
-  if ((ret = xpack_decode_integer(tmp, pos, pos + remain_len, 8)) < 0 && tmp > 0xFFFF) {
+  if ((ret = xpack_decode_integer(tmp, pos, pos + remain_len, 8)) < 0 || tmp > 0xFFFF) {
     return -1;
   }
   pos                        += ret;
@@ -931,7 +931,7 @@ QPACK::_decode_header(const uint8_t *header_block, size_t header_block_len, HTTP
 
   uint64_t delta_base_index;
   uint16_t base_index;
-  if ((ret = xpack_decode_integer(delta_base_index, pos, pos + remain_len, 7)) < 0 && delta_base_index < 0xFFFF) {
+  if ((ret = xpack_decode_integer(delta_base_index, pos, pos + remain_len, 7)) < 0 || delta_base_index > 0xFFFF) {
     return -2;
   }
 
@@ -1514,15 +1514,15 @@ QPACK::_read_insert_with_name_ref(IOBufferReader &reader, bool &is_static, uint1
 
   // Name Index
   uint64_t tmp;
-  if ((ret = xpack_decode_integer(tmp, input, input + input_len, 6)) < 0 && tmp > 0xFFFF) {
+  if ((ret = xpack_decode_integer(tmp, input, input + input_len, 6)) < 0 || tmp > 0xFFFF) {
     return -1;
   }
   index     = tmp;
   read_len += ret;
 
   // Value
-  if ((ret = xpack_decode_string(arena, value, tmp, input + read_len, input + input_len, _header_field_max_size, 7)) < 0 &&
-      tmp > 0xFF) {
+  if ((ret = xpack_decode_string(arena, value, tmp, input + read_len, input + input_len, _header_field_max_size, 7)) < 0 ||
+      tmp > 0xFFFF) {
     return -1;
   }
   value_len  = tmp;
@@ -1545,14 +1545,14 @@ QPACK::_read_insert_without_name_ref(IOBufferReader &reader, Arena &arena, char
 
   // Name
   uint64_t tmp;
-  if ((ret = xpack_decode_string(arena, name, tmp, input, input + input_len, _header_field_max_size, 5)) < 0 && tmp > 0xFFFF) {
+  if ((ret = xpack_decode_string(arena, name, tmp, input, input + input_len, _header_field_max_size, 5)) < 0 || tmp > 0xFFFF) {
     return -1;
   }
   name_len  = tmp;
   read_len += ret;
 
   // Value
-  if ((ret = xpack_decode_string(arena, value, tmp, input + read_len, input + input_len, _header_field_max_size, 7)) < 0 &&
+  if ((ret = xpack_decode_string(arena, value, tmp, input + read_len, input + input_len, _header_field_max_size, 7)) < 0 ||
       tmp > 0xFFFF) {
     return -1;
   }
@@ -1575,7 +1575,7 @@ QPACK::_read_duplicate(IOBufferReader &reader, uint16_t &index)
 
   // Index
   uint64_t tmp;
-  if ((ret = xpack_decode_integer(tmp, input, input + input_len, 5)) < 0 && tmp > 0xFFFF) {
+  if ((ret = xpack_decode_integer(tmp, input, input + input_len, 5)) < 0 || tmp > 0xFFFF) {
     return -1;
   }
   index     = tmp;
@@ -1597,7 +1597,7 @@ QPACK::_read_dynamic_table_size_update(IOBufferReader &reader, uint16_t &max_siz
   uint64_t tmp;
 
   // Max Size
-  if ((ret = xpack_decode_integer(tmp, input, input + input_len, 5)) < 0 && tmp > 0xFFFF) {
+  if ((ret = xpack_decode_integer(tmp, input, input + input_len, 5)) < 0 || tmp > 0xFFFF) {
     return -1;
   }
   max_size  = tmp;
@@ -1619,7 +1619,7 @@ QPACK::_read_table_state_synchronize(IOBufferReader &reader, uint16_t &insert_co
   uint64_t tmp;
 
   // Insert Count
-  if ((ret = xpack_decode_integer(tmp, input, input + input_len, 6)) < 0 && tmp > 0xFFFF) {
+  if ((ret = xpack_decode_integer(tmp, input, input + input_len, 6)) < 0 || tmp > 0xFFFF) {
     return -1;
   }
   insert_count  = tmp;

src/proxy/http3/test/test_QPACK.cc

@@ -439,6 +439,46 @@ TEST_CASE("Encoding", "[qpack-encode]")
   }
 }
 
+// QPACK::decode() parses the Required Insert Count varint from the header
+// block prefix and stores the decoded value in a uint16_t local; the call site
+// must reject a varint whose value exceeds the uint16_t range, otherwise the
+// value silently truncates and the decoder proceeds with corrupted state.
+// Drive QPACK::decode() with a 4-byte 8-bit-prefix varint encoding 0x10000
+// and assert that the decoder reports failure rather than accepting it.
+TEST_CASE("decode() rejects oversized Required Insert Count at entry", "[qpack-decode-entry-bounds]")
+{
+  QUICApplicationDriver driver;
+  auto                  qpack   = std::make_unique<QPACK>(driver.get_connection(), UINT32_MAX, 4096, 100, MAX_FIELD_SIZE);
+  auto                  handler = std::make_unique<TestQPACKEventHandler>();
+
+  HTTPHdr hdr;
+  hdr.create(HTTPType::REQUEST);
+
+  uint8_t  block[16] = {0};
+  uint8_t *p         = block;
+  int      enc_len   = xpack_encode_integer(p, p + sizeof(block), 0x10000, 8);
+  REQUIRE(enc_len > 0);
+  p                += enc_len;
+  *p++              = 0x00;
+  size_t block_len  = static_cast<size_t>(p - block);
+
+  int sync_ret = qpack->decode(1, block, block_len, hdr, handler.get(), eventProcessor.all_ethreads[0]);
+
+  // decode() schedules its result asynchronously when it returns >= 0; only
+  // wait in that case. A synchronous failure (sync_ret < 0) means no event
+  // will be delivered and there is nothing to wait for.
+  if (sync_ret >= 0) {
+    sleep(1);
+  }
+
+  CAPTURE(sync_ret);
+  CAPTURE(handler->last_event());
+  CHECK_FALSE((sync_ret == 0 && handler->last_event() == QPACK_EVENT_DECODE_COMPLETE));
+  CHECK((sync_ret < 0 || handler->last_event() == QPACK_EVENT_DECODE_FAILED));
+
+  hdr.destroy();
+}
+
 TEST_CASE("Decoding", "[qpack-decode]")
 {
   char app_dir[PATH_MAX + 1] = "";