Copilot commented on code in PR #13627:
URL: https://github.com/apache/trafficserver/pull/13627#discussion_r3932246346
##########
src/proxy/http3/test/test_Http3Frame.cc:
##########
@@ -166,6 +166,39 @@ 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<Http3Frame> frame = Http3FrameFactory::create(*input_reader);
+ frame->update();
+ REQUIRE(frame->type() == Http3FrameType::SETTINGS);
+
+ std::shared_ptr<Http3SettingsFrame> settings_frame =
std::dynamic_pointer_cast<Http3SettingsFrame>(frame);
+ REQUIRE(settings_frame);
+ REQUIRE(settings_frame->is_valid());
+
+ CHECK_FALSE(settings_frame->contains(Http3SettingsId::HEADER_TABLE_SIZE));
+
+ free_MIOBuffer(input);
+}
Review Comment:
`free_MIOBuffer(input)` is called while `frame` / `settings_frame` still own
the `IOBufferReader` allocated from that `MIOBuffer`. Since
`Http3Frame::~Http3Frame()` calls `_reader->dealloc()`, freeing the `MIOBuffer`
first can lead to a use-after-free / nullptr dereference in the frame
destructor (depending on optimization and allocator reuse). Reset the
shared_ptrs before freeing the buffer so the reader is deallocated while the
`MIOBuffer` is still alive.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
