From 2c8eee9ef1106ca188976912431337b878940281 Mon Sep 17 00:00:00 2001 From: Alan Hoffmeister Date: Tue, 17 Mar 2026 18:00:03 -0300 Subject: [PATCH 1/4] fix(quic): bound decrypt header associated data Reject oversized QUIC headers before copying them into the fixed-size associated-data buffer used during packet decryption, and add regression tests covering the oversize and unprotected-first-byte cases. Co-authored-by: Codex --- src/quic/packet.zig | 51 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/src/quic/packet.zig b/src/quic/packet.zig index 2db6153..f014061 100644 --- a/src/quic/packet.zig +++ b/src/quic/packet.zig @@ -38,6 +38,24 @@ const RETRY_INTEGRITY_NONCE_V1: [crypto.nonce_len]u8 = .{ 0x46, 0x15, 0x99, 0xd3 const RETRY_INTEGRITY_KEY_V2: [crypto.key_len]u8 = .{ 0x8f, 0xb4, 0xb0, 0x1b, 0x56, 0xac, 0x48, 0xe2, 0x60, 0xfb, 0xcb, 0xce, 0xad, 0x7c, 0xcc, 0x92 }; const RETRY_INTEGRITY_NONCE_V2: [crypto.nonce_len]u8 = .{ 0xd8, 0x69, 0x69, 0xbc, 0x2d, 0x7c, 0x6d, 0x99, 0x90, 0xef, 0xb0, 0x4a }; +fn buildHeaderBytes( + header_bytes_buf: *[512]u8, + first_byte: u8, + packet: []u8, + pkt_start: usize, + header_end: usize, +) ![]const u8 { + if (header_end <= pkt_start) return error.InvalidPacket; + const header_len = header_end - pkt_start; + if (header_len > header_bytes_buf.len) return error.InvalidPacket; + + header_bytes_buf[0] = first_byte; + if (header_len > 1) { + @memcpy(header_bytes_buf[1..][0..(header_len - 1)], packet[(pkt_start + 1)..header_end]); + } + return header_bytes_buf[0..header_len]; +} + pub const PacketType = enum(u8) { /// Initial packet initial = LONG_HEADER_BIT | FIXED_BIT | 0x00, @@ -380,13 +398,8 @@ pub fn decrypt(header: *Header, fbs: anytype, space: PacketNumSpace) ![]u8 { // RFC 9001 Section 5.2: AD includes the unprotected first byte and everything up to and including packet number // For coalesced packets, use packet_start to get the correct offset within the buffer const pkt_start = header.packet_start; - const header_len = fbs.pos - pkt_start; // total header length including packet number var header_bytes_buf: [512]u8 = undefined; - // Copy first byte as unprotected - header_bytes_buf[0] = first_byte; - // Copy the rest of the header and packet number (from byte after first to current position) - @memcpy(header_bytes_buf[1..][0..(header_len - 1)], fbs.buffer[(pkt_start + 1)..fbs.pos]); - const header_bytes = header_bytes_buf[0..header_len]; + const header_bytes = try buildHeaderBytes(&header_bytes_buf, first_byte, fbs.buffer, pkt_start, fbs.pos); const encrypted_payload = fbs.buffer[(fbs.pos)..(fbs.pos + payload_len)]; // Decode packet number @@ -461,11 +474,8 @@ pub fn decryptWithKeyUpdate(header: *Header, fbs: anytype, space: *PacketNumSpac // Build associated data const pkt_start = header.packet_start; - const header_len = fbs.pos - pkt_start; var header_bytes_buf: [512]u8 = undefined; - header_bytes_buf[0] = first_byte; - @memcpy(header_bytes_buf[1..][0..(header_len - 1)], fbs.buffer[(pkt_start + 1)..fbs.pos]); - const header_bytes = header_bytes_buf[0..header_len]; + const header_bytes = try buildHeaderBytes(&header_bytes_buf, first_byte, fbs.buffer, pkt_start, fbs.pos); const encrypted_payload = fbs.buffer[(fbs.pos)..(fbs.pos + payload_len)]; // Decode packet number @@ -1035,6 +1045,27 @@ test "QUIC: Variable-Length Integer Decoding" { try std.testing.expect(37 == try readVarInt(fbs.reader())); } +test "QUIC: buildHeaderBytes rejects oversized associated data headers" { + var packet_buf: [513]u8 = .{0x42} ** 513; + var header_bytes_buf: [512]u8 = undefined; + + try std.testing.expectError( + error.InvalidPacket, + buildHeaderBytes(&header_bytes_buf, 0xc0, &packet_buf, 0, packet_buf.len), + ); +} + +test "QUIC: buildHeaderBytes keeps the unprotected first byte" { + var packet_buf = [_]u8{ 0xaa, 0x01, 0x02, 0x03, 0x04 }; + var header_bytes_buf: [512]u8 = undefined; + + const header_bytes = try buildHeaderBytes(&header_bytes_buf, 0xc1, &packet_buf, 0, packet_buf.len); + + try std.testing.expectEqual(@as(usize, packet_buf.len), header_bytes.len); + try std.testing.expectEqual(@as(u8, 0xc1), header_bytes[0]); + try std.testing.expectEqualSlices(u8, packet_buf[1..], header_bytes[1..]); +} + // // Recover a packet number from a truncated packet number. // From 51538e17e93740e33140536804a9ef7670c53d32 Mon Sep 17 00:00:00 2001 From: Alan Hoffmeister Date: Tue, 17 Mar 2026 18:18:08 -0300 Subject: [PATCH 2/4] fix(packet): validate decrypt payload bounds Co-authored-by: Codex --- src/quic/packet.zig | 67 ++++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 43 deletions(-) diff --git a/src/quic/packet.zig b/src/quic/packet.zig index f014061..1948e18 100644 --- a/src/quic/packet.zig +++ b/src/quic/packet.zig @@ -38,24 +38,6 @@ const RETRY_INTEGRITY_NONCE_V1: [crypto.nonce_len]u8 = .{ 0x46, 0x15, 0x99, 0xd3 const RETRY_INTEGRITY_KEY_V2: [crypto.key_len]u8 = .{ 0x8f, 0xb4, 0xb0, 0x1b, 0x56, 0xac, 0x48, 0xe2, 0x60, 0xfb, 0xcb, 0xce, 0xad, 0x7c, 0xcc, 0x92 }; const RETRY_INTEGRITY_NONCE_V2: [crypto.nonce_len]u8 = .{ 0xd8, 0x69, 0x69, 0xbc, 0x2d, 0x7c, 0x6d, 0x99, 0x90, 0xef, 0xb0, 0x4a }; -fn buildHeaderBytes( - header_bytes_buf: *[512]u8, - first_byte: u8, - packet: []u8, - pkt_start: usize, - header_end: usize, -) ![]const u8 { - if (header_end <= pkt_start) return error.InvalidPacket; - const header_len = header_end - pkt_start; - if (header_len > header_bytes_buf.len) return error.InvalidPacket; - - header_bytes_buf[0] = first_byte; - if (header_len > 1) { - @memcpy(header_bytes_buf[1..][0..(header_len - 1)], packet[(pkt_start + 1)..header_end]); - } - return header_bytes_buf[0..header_len]; -} - pub const PacketType = enum(u8) { /// Initial packet initial = LONG_HEADER_BIT | FIXED_BIT | 0x00, @@ -393,14 +375,25 @@ pub fn decrypt(header: *Header, fbs: anytype, space: PacketNumSpace) ![]u8 { // Skip the packet number bytes in the buffer try fbs.seekBy(@intCast(header.packet_number_len)); + if (header.remainder_len < header.packet_number_len) { + return error.InvalidPacket; + } const payload_len = header.remainder_len - header.packet_number_len; + if (payload_len > (fbs.buffer.len - fbs.pos)) { + return error.InvalidPacket; + } // RFC 9001 Section 5.2: AD includes the unprotected first byte and everything up to and including packet number // For coalesced packets, use packet_start to get the correct offset within the buffer const pkt_start = header.packet_start; + const header_len = fbs.pos - pkt_start; // total header length including packet number var header_bytes_buf: [512]u8 = undefined; - const header_bytes = try buildHeaderBytes(&header_bytes_buf, first_byte, fbs.buffer, pkt_start, fbs.pos); - const encrypted_payload = fbs.buffer[(fbs.pos)..(fbs.pos + payload_len)]; + // Copy first byte as unprotected + header_bytes_buf[0] = first_byte; + // Copy the rest of the header and packet number (from byte after first to current position) + @memcpy(header_bytes_buf[1..][0..(header_len - 1)], fbs.buffer[(pkt_start + 1)..fbs.pos]); + const header_bytes = header_bytes_buf[0..header_len]; + const encrypted_payload = fbs.buffer[fbs.pos..(fbs.pos + payload_len)]; // Decode packet number header.packet_number = decodePacketNumber(space.next_packet_number, truncated_packet_number, header.packet_number_len * 8); @@ -470,13 +463,22 @@ pub fn decryptWithKeyUpdate(header: *Header, fbs: anytype, space: *PacketNumSpac try fbs.seekBy(@intCast(header.packet_number_len)); + if (header.remainder_len < header.packet_number_len) { + return error.InvalidPacket; + } const payload_len = header.remainder_len - header.packet_number_len; + if (payload_len > (fbs.buffer.len - fbs.pos)) { + return error.InvalidPacket; + } // Build associated data const pkt_start = header.packet_start; + const header_len = fbs.pos - pkt_start; var header_bytes_buf: [512]u8 = undefined; - const header_bytes = try buildHeaderBytes(&header_bytes_buf, first_byte, fbs.buffer, pkt_start, fbs.pos); - const encrypted_payload = fbs.buffer[(fbs.pos)..(fbs.pos + payload_len)]; + header_bytes_buf[0] = first_byte; + @memcpy(header_bytes_buf[1..][0..(header_len - 1)], fbs.buffer[(pkt_start + 1)..fbs.pos]); + const header_bytes = header_bytes_buf[0..header_len]; + const encrypted_payload = fbs.buffer[fbs.pos..(fbs.pos + payload_len)]; // Decode packet number header.packet_number = decodePacketNumber(space.next_packet_number, truncated_packet_number, header.packet_number_len * 8); @@ -1045,27 +1047,6 @@ test "QUIC: Variable-Length Integer Decoding" { try std.testing.expect(37 == try readVarInt(fbs.reader())); } -test "QUIC: buildHeaderBytes rejects oversized associated data headers" { - var packet_buf: [513]u8 = .{0x42} ** 513; - var header_bytes_buf: [512]u8 = undefined; - - try std.testing.expectError( - error.InvalidPacket, - buildHeaderBytes(&header_bytes_buf, 0xc0, &packet_buf, 0, packet_buf.len), - ); -} - -test "QUIC: buildHeaderBytes keeps the unprotected first byte" { - var packet_buf = [_]u8{ 0xaa, 0x01, 0x02, 0x03, 0x04 }; - var header_bytes_buf: [512]u8 = undefined; - - const header_bytes = try buildHeaderBytes(&header_bytes_buf, 0xc1, &packet_buf, 0, packet_buf.len); - - try std.testing.expectEqual(@as(usize, packet_buf.len), header_bytes.len); - try std.testing.expectEqual(@as(u8, 0xc1), header_bytes[0]); - try std.testing.expectEqualSlices(u8, packet_buf[1..], header_bytes[1..]); -} - // // Recover a packet number from a truncated packet number. // From 2f13b45d5561c77ed73f210a41b359326a7915b3 Mon Sep 17 00:00:00 2001 From: Alan Hoffmeister Date: Tue, 17 Mar 2026 18:32:11 -0300 Subject: [PATCH 3/4] fix(packet): bound decrypt associated data rebuild Add shared header rebuilding bounds checks and regression tests for malformed payload lengths in both decrypt paths. Co-authored-by: Codex --- src/quic/packet.zig | 129 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 119 insertions(+), 10 deletions(-) diff --git a/src/quic/packet.zig b/src/quic/packet.zig index 1948e18..3d171b3 100644 --- a/src/quic/packet.zig +++ b/src/quic/packet.zig @@ -308,6 +308,24 @@ pub fn parseIncoming(bytes: []const u8) void { _ = bytes; } +fn buildHeaderBytes(header_bytes_buf: []u8, first_byte: u8, packet_buf: []const u8, packet_start: usize, header_end: usize) PacketError![]const u8 { + if (header_end <= packet_start or header_end > packet_buf.len) { + return error.InvalidPacket; + } + + const header_len = header_end - packet_start; + if (header_len > header_bytes_buf.len) { + return error.InvalidPacket; + } + + header_bytes_buf[0] = first_byte; + if (header_len > 1) { + @memcpy(header_bytes_buf[1..header_len], packet_buf[(packet_start + 1)..header_end]); + } + + return header_bytes_buf[0..header_len]; +} + pub fn decrypt(header: *Header, fbs: anytype, space: PacketNumSpace) ![]u8 { // We need at least 4 bytes for packet number + 16 for sample if (fbs.pos + 4 + crypto.SAMPLE_LEN > fbs.buffer.len) { @@ -386,13 +404,8 @@ pub fn decrypt(header: *Header, fbs: anytype, space: PacketNumSpace) ![]u8 { // RFC 9001 Section 5.2: AD includes the unprotected first byte and everything up to and including packet number // For coalesced packets, use packet_start to get the correct offset within the buffer const pkt_start = header.packet_start; - const header_len = fbs.pos - pkt_start; // total header length including packet number var header_bytes_buf: [512]u8 = undefined; - // Copy first byte as unprotected - header_bytes_buf[0] = first_byte; - // Copy the rest of the header and packet number (from byte after first to current position) - @memcpy(header_bytes_buf[1..][0..(header_len - 1)], fbs.buffer[(pkt_start + 1)..fbs.pos]); - const header_bytes = header_bytes_buf[0..header_len]; + const header_bytes = try buildHeaderBytes(&header_bytes_buf, first_byte, fbs.buffer, pkt_start, fbs.pos); const encrypted_payload = fbs.buffer[fbs.pos..(fbs.pos + payload_len)]; // Decode packet number @@ -473,11 +486,8 @@ pub fn decryptWithKeyUpdate(header: *Header, fbs: anytype, space: *PacketNumSpac // Build associated data const pkt_start = header.packet_start; - const header_len = fbs.pos - pkt_start; var header_bytes_buf: [512]u8 = undefined; - header_bytes_buf[0] = first_byte; - @memcpy(header_bytes_buf[1..][0..(header_len - 1)], fbs.buffer[(pkt_start + 1)..fbs.pos]); - const header_bytes = header_bytes_buf[0..header_len]; + const header_bytes = try buildHeaderBytes(&header_bytes_buf, first_byte, fbs.buffer, pkt_start, fbs.pos); const encrypted_payload = fbs.buffer[fbs.pos..(fbs.pos + payload_len)]; // Decode packet number @@ -1073,6 +1083,105 @@ fn decodePacketNumber(expected_pkt_num: u64, truncated_pkt_num: u64, num_bits: u return candidate; } +fn testOpen() crypto.Open { + return .{ + .key = .{0} ** crypto.max_key_len, + .hp_key = .{0} ** crypto.max_key_len, + .nonce = .{0} ** crypto.nonce_len, + }; +} + +fn testKeyUpdateManager() crypto.KeyUpdateManager { + return crypto.KeyUpdateManager.init( + .{0} ** 32, + .{0} ** 32, + .{0} ** crypto.max_key_len, + .{0} ** crypto.max_key_len, + ); +} + +fn initProtectedShortHeaderPacket(buffer: []u8, hp_key: [crypto.max_key_len]u8, unprotected_first_byte: u8, truncated_packet_number: u8) void { + @memset(buffer, 0); + + const pn_offset = 1; + const sample: *const [crypto.SAMPLE_LEN]u8 = buffer[(pn_offset + 4)..][0..crypto.SAMPLE_LEN]; + const mask = crypto.computeHpMask(sample, hp_key, .aes_128_gcm_sha256); + + buffer[0] = unprotected_first_byte ^ (mask[0] & 0x1f); + buffer[pn_offset] = truncated_packet_number ^ mask[1]; +} + +test "QUIC: buildHeaderBytes rejects oversized associated data headers" { + var header_bytes_buf: [512]u8 = undefined; + var packet_buf: [513]u8 = .{0} ** 513; + + try std.testing.expectError( + error.InvalidPacket, + buildHeaderBytes(&header_bytes_buf, FIXED_BIT, &packet_buf, 0, packet_buf.len), + ); +} + +test "QUIC: buildHeaderBytes keeps the unprotected first byte" { + var header_bytes_buf: [8]u8 = undefined; + const packet_buf = [_]u8{ 0xff, 0xaa, 0xbb, 0xcc }; + + const header_bytes = try buildHeaderBytes(&header_bytes_buf, FIXED_BIT, &packet_buf, 0, packet_buf.len); + + try std.testing.expectEqualSlices(u8, &[_]u8{ FIXED_BIT, 0xaa, 0xbb, 0xcc }, header_bytes); +} + +test "QUIC: decrypt rejects malformed payload lengths" { + const open = testOpen(); + const space = PacketNumSpace{ .crypto_open = open }; + const unprotected_first_byte = FIXED_BIT; + + { + var buf: [32]u8 = undefined; + initProtectedShortHeaderPacket(&buf, open.hp_key, unprotected_first_byte, 0); + var fbs = io.fixedBufferStream(&buf); + fbs.pos = 1; + var header = Header{ .packet_start = 0, .remainder_len = 0 }; + + try std.testing.expectError(error.InvalidPacket, decrypt(&header, &fbs, space)); + } + + { + var buf: [32]u8 = undefined; + initProtectedShortHeaderPacket(&buf, open.hp_key, unprotected_first_byte, 0); + var fbs = io.fixedBufferStream(&buf); + fbs.pos = 1; + var header = Header{ .packet_start = 0, .remainder_len = buf.len }; + + try std.testing.expectError(error.InvalidPacket, decrypt(&header, &fbs, space)); + } +} + +test "QUIC: decryptWithKeyUpdate rejects malformed payload lengths" { + var ku = testKeyUpdateManager(); + var space = PacketNumSpace{}; + const unprotected_first_byte = FIXED_BIT; + + { + var buf: [32]u8 = undefined; + initProtectedShortHeaderPacket(&buf, ku.hp_open, unprotected_first_byte, 0); + var fbs = io.fixedBufferStream(&buf); + fbs.pos = 1; + var header = Header{ .packet_start = 0, .remainder_len = 0 }; + + try std.testing.expectError(error.InvalidPacket, decryptWithKeyUpdate(&header, &fbs, &space, &ku)); + } + + { + var buf: [32]u8 = undefined; + initProtectedShortHeaderPacket(&buf, ku.hp_open, unprotected_first_byte, 0); + var fbs = io.fixedBufferStream(&buf); + fbs.pos = 1; + var header = Header{ .packet_start = 0, .remainder_len = buf.len }; + + try std.testing.expectError(error.InvalidPacket, decryptWithKeyUpdate(&header, &fbs, &space, &ku)); + } +} + // Retry token roundtrip test test "Retry token: generate and validate roundtrip" { var token_key: [crypto.key_len]u8 = undefined; From bb3bde27215caec1ac85ee9c37573d5e28c097bd Mon Sep 17 00:00:00 2001 From: Alan Hoffmeister Date: Tue, 17 Mar 2026 18:41:31 -0300 Subject: [PATCH 4/4] fix(packet): narrow helper error set Use an explicit InvalidPacket-only error set for buildHeaderBytes so decrypt APIs do not inherit a broader helper error surface. Co-authored-by: Codex --- src/quic/packet.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/quic/packet.zig b/src/quic/packet.zig index 3d171b3..4da6b68 100644 --- a/src/quic/packet.zig +++ b/src/quic/packet.zig @@ -308,7 +308,7 @@ pub fn parseIncoming(bytes: []const u8) void { _ = bytes; } -fn buildHeaderBytes(header_bytes_buf: []u8, first_byte: u8, packet_buf: []const u8, packet_start: usize, header_end: usize) PacketError![]const u8 { +fn buildHeaderBytes(header_bytes_buf: []u8, first_byte: u8, packet_buf: []const u8, packet_start: usize, header_end: usize) error{InvalidPacket}![]const u8 { if (header_end <= packet_start or header_end > packet_buf.len) { return error.InvalidPacket; }