From 806236cd484d7c478ad1c02d6481177146fbf9a5 Mon Sep 17 00:00:00 2001 From: itzzdev09 Date: Fri, 11 Sep 2026 21:57:28 +0530 Subject: [PATCH] tls: read the InnerPlaintext content type past the padding RFC 8446 sect. 5.2 lays a TLS 1.3 InnerPlaintext out as content || content_type || zeros(padding) TLSInnerPlaintext.pre_dissect() tried to skip that padding, but the guard could never fire: if s[-1] != b"\x00": s is bytes, so s[-1] is an int and the comparison against b"\x00" is always true. The padding branch was dead code, and msg_len was always len(s) - 1 -- the type was read from the final byte, which for a padded record is a pad byte, not the content type. The dead branch was also inverted: it started at n = 1 and advanced while the byte was non-zero, so had it run it would have stopped immediately on the first zero and stripped one byte regardless of the padding length. Scan back over the zero padding instead; the last non-zero byte is the content type. An unpadded record stops the loop immediately and keeps msg_len = len(s) - 1, exactly as before. An all-zero payload holds no valid content type (0 is not a ContentType), so it keeps the previous behaviour rather than raising, which leaves default-constructed packets dissectable. Before, on b"HELLO" + b"\x16" + b"\x00" * 3: type=0x00 pad=b'' After: type=0x16 pad=b'\x00\x00\x00' Test suites, run against this checkout and against an unmodified master worktree for comparison. cert/sslv2/tlsclientserver fail identically in both arms -- pre-existing here with a newer `cryptography` than this checkout supports -- and are listed for completeness: tls13 53 -> 56 passed, 0 failed (3 new; 1 fails on master) tls 91 passed, 0 failed unchanged cert 33 passed, 37 failed unchanged sslv2 1 passed, 27 failed unchanged tlsclientserver 9 passed, 26 failed unchanged flake8 on the touched module is identical to master. AI-Assisted: yes (Claude Code, Claude Opus 5) Co-Authored-By: Claude Opus 5 --- scapy/layers/tls/record_tls13.py | 20 +++++++++++------- test/scapy/layers/tls/tls13.uts | 36 ++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/scapy/layers/tls/record_tls13.py b/scapy/layers/tls/record_tls13.py index f6805b0dacc..09245b6674a 100644 --- a/scapy/layers/tls/record_tls13.py +++ b/scapy/layers/tls/record_tls13.py @@ -47,14 +47,18 @@ def pre_dissect(self, s): if len(s) < 1: raise Exception("Invalid InnerPlaintext (too short).") - tmp_len = len(s) - 1 - if s[-1] != b"\x00": - msg_len = tmp_len - else: - n = 1 - while s[-n] != b"\x00" and n < tmp_len: - n += 1 - msg_len = tmp_len - n + # RFC 8446 sect. 5.2: an InnerPlaintext is + # content || content_type || zeros(padding) + # so the content type is the last non-zero byte. Scan back over the + # padding to find it; with no padding the loop stops immediately and + # msg_len is len(s) - 1, as before. + n = len(s) + while n > 0 and s[n - 1] == 0: + n -= 1 + # All-zero input carries no valid content type (0 is not a ContentType). + # Keep the previous behaviour there rather than raising, so that + # default-constructed packets still round-trip. + msg_len = n - 1 if n > 0 else len(s) - 1 self.fields_desc[0].length_from = lambda pkt: msg_len self.type = struct.unpack("B", s[msg_len:msg_len + 1])[0] diff --git a/test/scapy/layers/tls/tls13.uts b/test/scapy/layers/tls/tls13.uts index 3e51200695e..1f4898811f6 100644 --- a/test/scapy/layers/tls/tls13.uts +++ b/test/scapy/layers/tls/tls13.uts @@ -1355,3 +1355,39 @@ def parses_share(value): assert not parses_share(1) assert parses_share(2) + ++ TLS 1.3 InnerPlaintext padding + += InnerPlaintext - the content type is the last non-zero byte + +% RFC 8446 sect. 5.2 lays an InnerPlaintext out as +% content || content_type || zeros(padding) +% The type was previously read from the very last byte, so any padded record +% reported content type 0 -- not a valid ContentType -- and the padding was +% swallowed into the message list instead of landing in `pad`. + +from scapy.layers.tls.record_tls13 import TLSInnerPlaintext + +for pad_len in [1, 3, 16]: + for content_type in [0x15, 0x16, 0x17]: + pkt = TLSInnerPlaintext(b"HELLO" + bytes([content_type]) + b"\x00" * pad_len) + assert pkt.type == content_type, (pad_len, content_type, pkt.type) + assert bytes(pkt.pad) == b"\x00" * pad_len, (pad_len, bytes(pkt.pad)) + += InnerPlaintext - an unpadded record is unaffected + +from scapy.layers.tls.record_tls13 import TLSInnerPlaintext + +pkt = TLSInnerPlaintext(b"HELLO" + b"\x16") +assert pkt.type == 0x16 +assert bytes(pkt.pad) == b"" + += InnerPlaintext - an all-zero payload still dissects without raising + +% Nothing in it is a valid content type, so there is nothing to find; this only +% pins that the scan does not walk off the front of the buffer. + +from scapy.layers.tls.record_tls13 import TLSInnerPlaintext + +pkt = TLSInnerPlaintext(b"\x00\x00") +assert pkt.type == 0