tls.Client: reject empty TLS 1.3 inner plaintext and short records

After decryption, TLS 1.3 plaintext is trimmed of zero padding, then
the last byte is read as the content type.

But when the plaintext was entirely zero padding, we got a
"thread panic: integer overflow at msg.len - 1" error. That could be
triggered by any server to crash the client.
This commit is contained in:
Frank Denis
2026-04-20 12:07:51 +02:00
parent 4e2147d14e
commit ac7e895df0
2 changed files with 77 additions and 1 deletions
+1
View File
@@ -367,6 +367,7 @@ test {
_ = ff;
_ = errors;
_ = tls;
_ = tls.Client;
_ = Certificate;
_ = codecs;
}
+76 -1
View File
@@ -382,7 +382,9 @@ pub fn init(input: *Reader, output: *Writer, options: Options) InitError!Client
P.AEAD.decrypt(cleartext, ciphertext, auth_tag, record_header, nonce, pv.server_handshake_key) catch
return error.TlsBadRecordMac;
// TODO use scalar, non-slice version
cleartext_fragment_end += mem.trimEnd(u8, cleartext, "\x00").len;
const trimmed_len = mem.trimEnd(u8, cleartext, "\x00").len;
if (trimmed_len == 0) return error.TlsDecodeError;
cleartext_fragment_end += trimmed_len;
},
}
read_seq += 1;
@@ -1176,6 +1178,7 @@ fn readIndirect(c: *Client) Reader.Error!usize {
.tls_1_3 => {
const pv = &p.tls_1_3;
const P = @TypeOf(p.*);
if (record_len < P.AEAD.tag_length) return failRead(c, error.TlsRecordOverflow);
const ad = input.take(tls.record_header_len) catch unreachable; // already peeked
const ciphertext_len = record_len - P.AEAD.tag_length;
const ciphertext = input.take(ciphertext_len) catch unreachable; // already peeked
@@ -1192,6 +1195,7 @@ fn readIndirect(c: *Client) Reader.Error!usize {
return failRead(c, error.TlsBadRecordMac);
// TODO use scalar, non-slice version
const msg = mem.trimEnd(u8, cleartext, "\x00");
if (msg.len == 0) return failRead(c, error.TlsDecodeError);
break :cleartext .{ msg.len - 1, @enumFromInt(msg[msg.len - 1]) };
},
.tls_1_2 => {
@@ -1668,3 +1672,74 @@ else
.AES_256_GCM_SHA384,
.ECDHE_RSA_WITH_AES_256_GCM_SHA384,
});
fn testReadError(input_buf: []const u8, cipher: tls.ApplicationCipher) ReadError {
var input_reader: Reader = .fixed(input_buf);
var read_buf: [tls.max_ciphertext_record_len]u8 = undefined;
var c: Client = .{
.input = &input_reader,
.reader = .{
.buffer = &read_buf,
.vtable = &.{ .stream = stream, .readVec = readVec },
.seek = 0,
.end = 0,
},
.output = undefined,
.writer = undefined,
.tls_version = .tls_1_3,
.read_seq = 0,
.write_seq = 0,
.received_close_notify = false,
.allow_truncation_attacks = false,
.application_cipher = cipher,
.ssl_key_log = null,
};
var w: Writer = .failing;
std.testing.expectError(error.ReadFailed, c.reader.stream(&w, .unlimited)) catch
@panic("expected ReadFailed");
return c.read_err.?;
}
test "empty inner plaintext" {
const AEAD = crypto.aead.chacha_poly.ChaCha20Poly1305;
const key: [AEAD.key_length]u8 = @splat(0);
const iv: [AEAD.nonce_length]u8 = @splat(0);
const plaintext = [1]u8{0x00};
var ciphertext: [plaintext.len]u8 = undefined;
var tag: [AEAD.tag_length]u8 = undefined;
const content_len: u16 = plaintext.len + AEAD.tag_length;
const record_header = [_]u8{ 0x17, 0x03, 0x03 } ++ mem.toBytes(big(content_len));
AEAD.encrypt(&ciphertext, &tag, &plaintext, &record_header, iv, key);
try std.testing.expectEqual(error.TlsDecodeError, testReadError(
&record_header ++ ciphertext ++ tag,
.{ .CHACHA20_POLY1305_SHA256 = .{ .tls_1_3 = .{
.server_key = key,
.server_iv = iv,
.client_secret = undefined,
.server_secret = undefined,
.client_key = undefined,
.client_iv = undefined,
} } },
));
}
test "record shorter than tag" {
const AEAD = crypto.aead.chacha_poly.ChaCha20Poly1305;
const record_len: u16 = AEAD.tag_length - 1;
const header = [_]u8{ 0x17, 0x03, 0x03 } ++ mem.toBytes(big(record_len));
const wire = header ++ @as([record_len]u8, @splat(0));
try std.testing.expectEqual(error.TlsRecordOverflow, testReadError(
&wire,
.{ .CHACHA20_POLY1305_SHA256 = .{ .tls_1_3 = .{
.server_key = undefined,
.server_iv = undefined,
.client_secret = undefined,
.server_secret = undefined,
.client_key = undefined,
.client_iv = undefined,
} } },
));
}