Skip to content

Commit 576e56f

Browse files
georgeglarsonclaude
andcommitted
Fix 13 audit issues: TLS zeroization, resource caps, info leak, defensive coding
HIGH: Zeroize cloned TLS 1.3 secrets after key derivation; zeroize decrypted plaintext after copying to connection buffer. MEDIUM: Cap HTTP/2 discard_header_bufs at 64 entries (was 1000); cap HTTP continuation header values at 8KB; use saturating_add for TUI memory tracking; cap DNS detail display at 100 records/section; cap HTTP message display at 256KB; cap format_hex input at 16KB. LOW: Remove zombie connections on handshake buffer overflow; redact StreamKey from eprintln warning; add context to regex error message; replace unwrap with expect on guarded HashMap remove. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 6263a3b commit 576e56f

8 files changed

Lines changed: 88 additions & 34 deletions

File tree

src/main.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ fn main() -> Result<()> {
127127
Some(
128128
regex::bytes::RegexBuilder::new(&pat)
129129
.size_limit(10 * 1024 * 1024)
130-
.build()?,
130+
.build()
131+
.context(format!("Invalid regex pattern: {}", p))?,
131132
)
132133
}
133134
None => None,

src/output/mod.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,8 +442,13 @@ fn format_addr(ip: Option<std::net::IpAddr>, port: Option<u16>) -> String {
442442
}
443443

444444
/// Build hex + ASCII dump as a String.
445+
/// Input is capped at 16 KB to prevent unbounded output.
445446
pub fn format_hex(data: &[u8]) -> String {
446447
use std::fmt::Write;
448+
const MAX_HEX_INPUT: usize = 16 * 1024;
449+
let original_len = data.len();
450+
let truncated = original_len > MAX_HEX_INPUT;
451+
let data = &data[..data.len().min(MAX_HEX_INPUT)];
447452
let mut out = String::new();
448453
for (i, chunk) in data.chunks(16).enumerate() {
449454
write!(out, "{:08x} ", i * 16).unwrap();
@@ -473,6 +478,9 @@ pub fn format_hex(data: &[u8]) -> String {
473478
}
474479
out.push_str("|\n");
475480
}
481+
if truncated {
482+
writeln!(out, "[... truncated, {} bytes total]", original_len).unwrap();
483+
}
476484
out
477485
}
478486

src/protocol/http.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,11 @@ pub fn parse_http(data: &[u8]) -> Vec<HttpMessage> {
248248
for line in lines {
249249
if (line.starts_with(' ') || line.starts_with('\t')) && !headers.is_empty() {
250250
// Continuation line: append to previous header value
251+
// Cap individual header value size to prevent abuse via many continuation lines.
251252
if let Some(last) = headers.last_mut() {
253+
if last.1.len() + line.len() + 1 > 8192 {
254+
break;
255+
}
252256
last.1.push(' ');
253257
last.1.push_str(line.trim());
254258
}

src/protocol/http2.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ const MAX_FRAME_PAYLOAD: usize = 16_777_215; // 2^24 - 1
2525
const MAX_STREAMS_PER_CONN: usize = 1_000;
2626
const MAX_HEADER_BLOCK: usize = 65_536;
2727
const MAX_DATA_PER_STREAM: usize = 1_048_576; // 1 MB
28+
/// Cap on discard_header_bufs entries (streams with incomplete CONTINUATION
29+
/// sequences). Much smaller than MAX_STREAMS_PER_CONN because each entry
30+
/// can hold up to MAX_HEADER_BLOCK (64 KB) of buffered header data.
31+
const MAX_DISCARD_HEADER_BUFS: usize = 64;
2832
/// M1: Reduced from 10,000 to 2,000 to cap compound memory usage.
2933
/// 2,000 connections * 2 MB * 2 directions = ~8 GB worst case.
3034
const MAX_CONNECTIONS: usize = 2_000;
@@ -299,7 +303,7 @@ impl H2Tracker {
299303
H2Direction::ServerToClient => &mut conn.server_decoder,
300304
};
301305
let _ = decoder.decode(header_block);
302-
} else if conn.discard_header_bufs.len() < MAX_STREAMS_PER_CONN {
306+
} else if conn.discard_header_bufs.len() < MAX_DISCARD_HEADER_BUFS {
303307
// No END_HEADERS — accumulate fragments for HPACK
304308
// decode when the final CONTINUATION arrives.
305309
// Cap discard_header_bufs to prevent memory exhaustion.
@@ -469,7 +473,7 @@ impl H2Tracker {
469473
H2Direction::ServerToClient => &mut conn.server_decoder,
470474
};
471475
let _ = decoder.decode(header_block);
472-
} else if conn.discard_header_bufs.len() < MAX_STREAMS_PER_CONN {
476+
} else if conn.discard_header_bufs.len() < MAX_DISCARD_HEADER_BUFS {
473477
// No END_HEADERS — accumulate fragments for HPACK
474478
// decode when the final CONTINUATION arrives.
475479
// Cap discard_header_bufs to prevent memory exhaustion.

src/reassembly/mod.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,10 @@ impl DirectionBuffer {
173173
.copied();
174174
match overlapping {
175175
Some(seg_seq) => {
176-
let seg_data = self.reorder_buf.remove(&seg_seq).unwrap();
176+
let seg_data = self
177+
.reorder_buf
178+
.remove(&seg_seq)
179+
.expect("seg_seq found by preceding .find().copied()");
177180
let overlap = expected.wrapping_sub(seg_seq) as usize;
178181
if overlap < seg_data.len() {
179182
if !self.truncated {

src/tls/mod.rs

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,8 @@ impl TlsDecryptor {
166166
};
167167
if overflow {
168168
eprintln!(
169-
"Warning: TLS {} buffer overflow for {:?}, removing connection",
169+
"Warning: TLS {} buffer overflow, removing connection",
170170
if from_client { "client" } else { "server" },
171-
key
172171
);
173172
self.connections.remove(key);
174173
return;
@@ -306,15 +305,19 @@ impl TlsDecryptor {
306305
}
307306
// After CCS, handshake records (Finished) are encrypted — decrypt to advance seq
308307
TlsRecordType::Handshake | TlsRecordType::ApplicationData => {
309-
if let Some(plaintext) = self.decrypt_record(key, &record, src_ip, src_port)
310-
&& record.hdr.record_type == TlsRecordType::ApplicationData
311-
&& let Some(conn) = self.connections.get_mut(key)
308+
if let Some(mut plaintext) = self.decrypt_record(key, &record, src_ip, src_port)
312309
{
313-
let remaining = MAX_DECRYPTED_BYTES.saturating_sub(conn.decrypted.len());
314-
let to_copy = plaintext.len().min(remaining);
315-
if to_copy > 0 {
316-
conn.decrypted.extend_from_slice(&plaintext[..to_copy]);
310+
if record.hdr.record_type == TlsRecordType::ApplicationData {
311+
if let Some(conn) = self.connections.get_mut(key) {
312+
let remaining =
313+
MAX_DECRYPTED_BYTES.saturating_sub(conn.decrypted.len());
314+
let to_copy = plaintext.len().min(remaining);
315+
if to_copy > 0 {
316+
conn.decrypted.extend_from_slice(&plaintext[..to_copy]);
317+
}
318+
}
317319
}
320+
plaintext.zeroize();
318321
}
319322
}
320323
_ => {}
@@ -349,21 +352,32 @@ impl TlsDecryptor {
349352
// complete messages (a single TLS record can contain multiple handshake
350353
// messages). We extract one message per iteration while holding the
351354
// borrow, release it to call process_handshake, then re-acquire.
355+
// Check overflow before taking mutable borrow for extend.
356+
let overflow = match self.connections.get(key) {
357+
Some(conn) => {
358+
let buf_len = if from_client {
359+
conn.handshake_buf_client.len()
360+
} else {
361+
conn.handshake_buf_server.len()
362+
};
363+
buf_len + data.len() > MAX_HANDSHAKE_BUF
364+
}
365+
None => return,
366+
};
367+
if overflow {
368+
self.connections.remove(key);
369+
return;
370+
}
352371
{
353372
let conn = match self.connections.get_mut(key) {
354373
Some(c) => c,
355374
None => return,
356375
};
357-
let buf = if from_client {
358-
&mut conn.handshake_buf_client
376+
if from_client {
377+
conn.handshake_buf_client.extend_from_slice(data);
359378
} else {
360-
&mut conn.handshake_buf_server
361-
};
362-
if buf.len() + data.len() > MAX_HANDSHAKE_BUF {
363-
buf.clear();
364-
return;
379+
conn.handshake_buf_server.extend_from_slice(data);
365380
}
366-
buf.extend_from_slice(data);
367381
}
368382

369383
loop {
@@ -484,14 +498,17 @@ impl TlsDecryptor {
484498
hash_algo: hkdf::Algorithm,
485499
aead_algo: &'static aead::Algorithm,
486500
) {
487-
let secrets = match self.keylog.tls13_secrets.get(client_random) {
501+
let mut secrets = match self.keylog.tls13_secrets.get(client_random) {
488502
Some(s) => s.clone(),
489503
None => return,
490504
};
491505

492506
let conn = match self.connections.get_mut(key) {
493507
Some(c) => c,
494-
None => return,
508+
None => {
509+
secrets.zeroize();
510+
return;
511+
}
495512
};
496513

497514
// Derive application traffic keys
@@ -517,6 +534,7 @@ impl TlsDecryptor {
517534
{
518535
conn.server_hs_keys = Some(keys);
519536
}
537+
secrets.zeroize();
520538
}
521539

522540
fn derive_tls12_keys(

src/tui/event.rs

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -188,23 +188,25 @@ impl CaptureEvent {
188188
("DNS Q".into(), format!("{} {}", qname, qtype))
189189
};
190190

191+
// Cap record display at 100 entries per section to bound memory.
192+
const MAX_DISPLAY_RECORDS: usize = 100;
191193
let mut display = String::new();
192-
for q in &info.questions {
194+
for q in info.questions.iter().take(MAX_DISPLAY_RECORDS) {
193195
display.push_str(&format!("Q: {} {}\n", q.name, q.qtype));
194196
}
195-
for r in &info.answers {
197+
for r in info.answers.iter().take(MAX_DISPLAY_RECORDS) {
196198
display.push_str(&format!(
197199
"A: {} {} {} TTL={}\n",
198200
r.name, r.rtype, r.rdata, r.ttl
199201
));
200202
}
201-
for r in &info.authorities {
203+
for r in info.authorities.iter().take(MAX_DISPLAY_RECORDS) {
202204
display.push_str(&format!(
203205
"AUTH: {} {} {} TTL={}\n",
204206
r.name, r.rtype, r.rdata, r.ttl
205207
));
206208
}
207-
for r in &info.additionals {
209+
for r in info.additionals.iter().take(MAX_DISPLAY_RECORDS) {
208210
display.push_str(&format!(
209211
"ADD: {} {} {} TTL={}\n",
210212
r.name, r.rtype, r.rdata, r.ttl
@@ -282,12 +284,26 @@ impl CaptureEvent {
282284

283285
let header = format!("HTTP {} {}{}", stream_id, &info, count_suffix);
284286

285-
// Combine all messages into the display
286-
let display: String = messages
287-
.iter()
288-
.map(|msg| msg.display_string())
289-
.collect::<Vec<_>>()
290-
.join("\n---\n");
287+
// Combine all messages into the display, capping total size.
288+
const MAX_DISPLAY_BYTES: usize = 256 * 1024;
289+
let mut display = String::new();
290+
for (i, msg) in messages.iter().enumerate() {
291+
if i > 0 {
292+
display.push_str("\n---\n");
293+
}
294+
let s = msg.display_string();
295+
let remaining = MAX_DISPLAY_BYTES.saturating_sub(display.len());
296+
if remaining == 0 {
297+
display.push_str("\n[truncated]");
298+
break;
299+
}
300+
if s.len() > remaining {
301+
display.push_str(&s[..remaining]);
302+
display.push_str("\n[truncated]");
303+
break;
304+
}
305+
display.push_str(&s);
306+
}
291307

292308
CaptureEvent {
293309
id,

src/tui/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ pub fn run_tui(
176176
let auto_select = app.events.is_empty();
177177
while let Ok(event) = rx.try_recv() {
178178
if app.events.len() < MAX_TUI_EVENTS && app.events_bytes < MAX_TUI_BYTES {
179-
app.events_bytes += event.approx_bytes();
179+
app.events_bytes = app.events_bytes.saturating_add(event.approx_bytes());
180180
app.events.push(event);
181181
} else {
182182
// M22: Track dropped events

0 commit comments

Comments
 (0)