From 9c6d9ce144e55b936be364a4a4c5abd25070224c Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Thu, 3 Sep 2026 12:09:50 +0200 Subject: [PATCH 1/3] Preserve full width of SETTINGS frame identifiers Http3SettingsFrame::_parse() decoded each SETTINGS identifier with read_QUICVariableInt(), which returns uint64_t, but stored the result in a uint16_t. An identifier outside the known set whose low 16 bits happen to match a known ID was therefore misread as that setting. Widen the local to uint64_t so the comparison against the known-ID set happens at full variable-length integer width. --- src/proxy/http3/Http3Frame.cc | 2 +- src/proxy/http3/test/test_Http3Frame.cc | 37 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/proxy/http3/Http3Frame.cc b/src/proxy/http3/Http3Frame.cc index 86e94d517ae..a237106cf71 100644 --- a/src/proxy/http3/Http3Frame.cc +++ b/src/proxy/http3/Http3Frame.cc @@ -456,7 +456,7 @@ Http3SettingsFrame::_parse() this->_error_reason = reinterpret_cast("invalid SETTINGS frame"); break; } - uint16_t id = QUICIntUtil::read_QUICVariableInt(buf + len, this->_length - len); + uint64_t id = QUICIntUtil::read_QUICVariableInt(buf + len, this->_length - len); len += id_len; size_t value_len = QUICVariableInt::size(buf + len); diff --git a/src/proxy/http3/test/test_Http3Frame.cc b/src/proxy/http3/test/test_Http3Frame.cc index 80461da31ee..9f4527f67af 100644 --- a/src/proxy/http3/test/test_Http3Frame.cc +++ b/src/proxy/http3/test/test_Http3Frame.cc @@ -166,6 +166,43 @@ TEST_CASE("Load SETTINGS Frame", "[http3]") } } +// A SETTINGS identifier is a full QUIC variable-length integer (up to 62 bits). +// Parsing must compare the decoded identifier against the known-ID set at full +// width; narrowing the decoded value into a smaller integer type can cause an +// identifier outside the known set to be misinterpreted as a known one. +// Here, encoding 0x10001 as a 4-byte QUIC varint (0x80 0x01 0x00 0x01) shares +// its low 16 bits with HEADER_TABLE_SIZE (0x01). The parser must treat the +// identifier as unknown and skip the setting rather than store it under +// HEADER_TABLE_SIZE. +TEST_CASE("Load SETTINGS Frame ignores wide unknown identifier", "[http3][http3-settings-id-width]") +{ + uint8_t buf[] = { + 0x04, // Type + 0x05, // Length + 0x80, 0x01, 0x00, 0x01, // Identifier: QUIC varint encoding of 0x10001 + 0x2a, // Value (1-byte QUIC varint = 42) + }; + MIOBuffer *input = new_MIOBuffer(BUFFER_SIZE_INDEX_128); + input->write(buf, sizeof(buf)); + IOBufferReader *input_reader = input->alloc_reader(); + + std::shared_ptr frame = Http3FrameFactory::create(*input_reader); + frame->update(); + REQUIRE(frame->type() == Http3FrameType::SETTINGS); + + std::shared_ptr settings_frame = std::dynamic_pointer_cast(frame); + REQUIRE(settings_frame); + REQUIRE(settings_frame->is_valid()); + + CHECK_FALSE(settings_frame->contains(Http3SettingsId::HEADER_TABLE_SIZE)); + + // ~Http3Frame deallocates the reader it holds, so drop the frames before the + // MIOBuffer that reader belongs to. + settings_frame.reset(); + frame.reset(); + free_MIOBuffer(input); +} + TEST_CASE("Store SETTINGS Frame", "[http3]") { SECTION("Normal") From b1342b11fed651b8fe981b31dc427d45983114f1 Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Tue, 8 Sep 2026 18:29:28 +0200 Subject: [PATCH 2/3] Use a reserved SETTINGS identifier in the regression test RFC 9114, Section 7.2.4.1 reserves setting identifiers of the form 0x1f * N + 0x21 to exercise the requirement that unknown identifiers be ignored, and says endpoints should include at least one such setting in their SETTINGS frame. Use one of those instead of an arbitrary value, so the test covers an identifier that is actually sent on the wire: 0x1f * 33824 + 0x21 == 0x100001 shares its low 16 bits with HEADER_TABLE_SIZE (0x01). The varint encoding, the frame length and the assertions are unchanged. Also drop the extra Catch2 tag, so the case is tagged [http3] like the rest of the file. Release the frames before the MIOBuffer they read from in "Load DATA Frame" and "Load SETTINGS Frame". ~Http3Frame calls IOBufferReader:: dealloc(), which goes through the reader's MIOBuffer, and free_MIOBuffer() returns that buffer to the ioAllocator freelist, so the existing order touches the buffer after it is released. --- src/proxy/http3/test/test_Http3Frame.cc | 34 +++++++++++++++---------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/proxy/http3/test/test_Http3Frame.cc b/src/proxy/http3/test/test_Http3Frame.cc index 9f4527f67af..0f5b335d3af 100644 --- a/src/proxy/http3/test/test_Http3Frame.cc +++ b/src/proxy/http3/test/test_Http3Frame.cc @@ -59,6 +59,10 @@ TEST_CASE("Load DATA Frame", "[http3]") CHECK(data_reader->read_avail() == 4); CHECK(memcmp(data_reader->start(), "\x11\x22\x33\x44", 4) == 0); + // ~Http3Frame deallocates the reader through its MIOBuffer, so release the + // frames before freeing that buffer. + data_frame.reset(); + frame1.reset(); free_MIOBuffer(input); } } @@ -162,25 +166,29 @@ TEST_CASE("Load SETTINGS Frame", "[http3]") CHECK(settings_frame->get(Http3SettingsId::MAX_FIELD_SECTION_SIZE) == 0x0400); CHECK(settings_frame->get(Http3SettingsId::NUM_PLACEHOLDERS) == 0x0f); + settings_frame.reset(); + frame.reset(); free_MIOBuffer(input); } } -// A SETTINGS identifier is a full QUIC variable-length integer (up to 62 bits). -// Parsing must compare the decoded identifier against the known-ID set at full -// width; narrowing the decoded value into a smaller integer type can cause an -// identifier outside the known set to be misinterpreted as a known one. -// Here, encoding 0x10001 as a 4-byte QUIC varint (0x80 0x01 0x00 0x01) shares -// its low 16 bits with HEADER_TABLE_SIZE (0x01). The parser must treat the -// identifier as unknown and skip the setting rather than store it under -// HEADER_TABLE_SIZE. -TEST_CASE("Load SETTINGS Frame ignores wide unknown identifier", "[http3][http3-settings-id-width]") +// A SETTINGS identifier is a full QUIC variable-length integer, and an +// identifier the implementation does not understand must be ignored +// (RFC 9114, Section 7.2.4). Comparing a narrowed copy of the decoded +// identifier against the known-ID set lets an unknown identifier alias a +// known one. +// +// Identifiers of the form 0x1f * N + 0x21 are reserved to exercise that +// requirement, and endpoints are expected to send one (RFC 9114, Section +// 7.2.4.1). 0x1f * 33824 + 0x21 == 0x100001, which shares its low 16 bits +// with HEADER_TABLE_SIZE (0x01). +TEST_CASE("Load SETTINGS Frame ignores reserved identifier", "[http3]") { uint8_t buf[] = { 0x04, // Type 0x05, // Length - 0x80, 0x01, 0x00, 0x01, // Identifier: QUIC varint encoding of 0x10001 - 0x2a, // Value (1-byte QUIC varint = 42) + 0x80, 0x10, 0x00, 0x01, // Identifier: QUIC varint encoding of 0x100001 + 0x2a, // Value }; MIOBuffer *input = new_MIOBuffer(BUFFER_SIZE_INDEX_128); input->write(buf, sizeof(buf)); @@ -196,8 +204,8 @@ TEST_CASE("Load SETTINGS Frame ignores wide unknown identifier", "[http3][http3- CHECK_FALSE(settings_frame->contains(Http3SettingsId::HEADER_TABLE_SIZE)); - // ~Http3Frame deallocates the reader it holds, so drop the frames before the - // MIOBuffer that reader belongs to. + // ~Http3Frame deallocates the reader through its MIOBuffer, so release the + // frames before freeing that buffer. settings_frame.reset(); frame.reset(); free_MIOBuffer(input); From d1b88d56f0d48707de7c652a17bda682773e08fc Mon Sep 17 00:00:00 2001 From: Damian Meden Date: Wed, 9 Sep 2026 10:05:57 +0200 Subject: [PATCH 3/3] Note the teardown order in Load SETTINGS Frame The other two load-path teardowns carry a note explaining why the frames are released before free_MIOBuffer(); add the same one here so all three read alike. --- src/proxy/http3/test/test_Http3Frame.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/proxy/http3/test/test_Http3Frame.cc b/src/proxy/http3/test/test_Http3Frame.cc index 0f5b335d3af..819624f1968 100644 --- a/src/proxy/http3/test/test_Http3Frame.cc +++ b/src/proxy/http3/test/test_Http3Frame.cc @@ -166,6 +166,8 @@ TEST_CASE("Load SETTINGS Frame", "[http3]") CHECK(settings_frame->get(Http3SettingsId::MAX_FIELD_SECTION_SIZE) == 0x0400); CHECK(settings_frame->get(Http3SettingsId::NUM_PLACEHOLDERS) == 0x0f); + // ~Http3Frame deallocates the reader through its MIOBuffer, so release the + // frames before freeing that buffer. settings_frame.reset(); frame.reset(); free_MIOBuffer(input);