From 101f658e503eb040ea8c343db3e0c729d0c84fde Mon Sep 17 00:00:00 2001 From: Luigi Colluto Date: Sun, 5 Jul 2026 19:23:00 +0200 Subject: [PATCH 1/2] fix: bound Kerberos TCP response allocation and add read/write timeouts The Kerberos TCP transport read a 4-byte length prefix from the KDC and then did `vec![0; resp_len as usize]`, so a malicious or compromised KDC (or a MITM) could request a ~4 GiB allocation and abort the process (panic = "abort"). It also set only connect_timeout, so a KDC that stalled mid-response hung the operator's thread indefinitely on read_exact. Cap the response at 16 MiB (far above any real KRB message) and set read/write timeouts on the stream. --- src/plugins/kerberos/transport.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/plugins/kerberos/transport.rs b/src/plugins/kerberos/transport.rs index be675d1..7f78c34 100644 --- a/src/plugins/kerberos/transport.rs +++ b/src/plugins/kerberos/transport.rs @@ -2,6 +2,11 @@ use std::io::{self, Read, Write}; use std::net::{SocketAddr, TcpStream, UdpSocket}; use std::time::Duration; +/// Upper bound on a Kerberos TCP response length prefix. Real KRB-AS/TGS-REP messages are at most +/// a few hundred KB (even with a large PAC); this cap prevents a malicious server's 4-GiB length +/// field from driving an unbounded allocation. +const MAX_KRB_RESPONSE: usize = 16 * 1024 * 1024; // 16 MiB + use clap::ValueEnum; use serde::{Deserialize, Serialize}; @@ -71,6 +76,10 @@ impl TCP { impl Transport for TCP { fn request(&self, timeout: Duration, raw: &[u8]) -> io::Result> { let mut tcp = TcpStream::connect_timeout(&self.server, timeout)?; + // connect_timeout only bounds the connect; without these a KDC that stalls mid-response + // would hang the operator's thread indefinitely on read_exact. + tcp.set_read_timeout(Some(timeout))?; + tcp.set_write_timeout(Some(timeout))?; let req_size = raw.len() as u32; let mut req: Vec = req_size.to_be_bytes().to_vec(); @@ -80,9 +89,18 @@ impl Transport for TCP { let mut resp_len_raw = [0_u8; 4]; tcp.read_exact(&mut resp_len_raw)?; - let resp_len = u32::from_be_bytes(resp_len_raw); + let resp_len = u32::from_be_bytes(resp_len_raw) as usize; + + // The KDC controls this length prefix; cap it so a malicious/compromised server (or a + // MITM) cannot drive a multi-GB allocation. + if resp_len > MAX_KRB_RESPONSE { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("Kerberos response length {resp_len} exceeds the {MAX_KRB_RESPONSE}-byte limit"), + )); + } - let mut resp: Vec = vec![0; resp_len as usize]; + let mut resp: Vec = vec![0; resp_len]; tcp.read_exact(&mut resp)?; Ok(resp) From 72ef45ea77a6d365fde0310c8bd5d79f44ccbd74 Mon Sep 17 00:00:00 2001 From: Luigi Colluto Date: Sun, 5 Jul 2026 19:23:01 +0200 Subject: [PATCH 2/2] fix: bound AMQP connection.start allocation from a server-controlled size The AMQP plugin read the connection.start frame size from the server and did `vec![0_u8; (u32::from_be_bytes(size) + 1) as usize]`, so a malicious server could request a ~4 GiB allocation and abort the process (panic = "abort"); the `+ 1` also overflowed u32 in debug builds. Cap the frame at 1 MiB (the AMQP 0-9-1 default frame-max is 128 KiB) and compute the length in usize to avoid the overflow. --- src/plugins/amqp/mod.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/plugins/amqp/mod.rs b/src/plugins/amqp/mod.rs index 0eab589..0880aff 100644 --- a/src/plugins/amqp/mod.rs +++ b/src/plugins/amqp/mod.rs @@ -14,6 +14,11 @@ pub(crate) mod options; const PROTOCOL_HEADER_091: &[u8] = &[b'A', b'M', b'Q', b'P', 0, 0, 9, 1]; +/// Upper bound on the AMQP connection.start frame the server sends before negotiation. The AMQP +/// 0-9-1 default frame-max is 128 KiB; 1 MiB is generous and prevents a malicious server's 4-GiB +/// size field from driving an unbounded allocation. +const MAX_CONN_START_FRAME: usize = 1024 * 1024; // 1 MiB + super::manager::register_plugin! { "amqp" => AMQP::new() } @@ -62,9 +67,17 @@ impl Plugin for AMQP { .await .map_err(|e| e.to_string())?; let size_raw: [u8; 4] = conn_start_header[3..].try_into().unwrap(); - let payload_size = u32::from_be_bytes(size_raw) + 1; - // read connection.start body - let mut conn_start_body = vec![0_u8; payload_size as usize]; + let frame_size = u32::from_be_bytes(size_raw) as usize; + // The server controls this frame-size field; cap it so it cannot drive a ~4 GiB + // allocation, which would abort the process under panic = "abort". Computing in usize + // also avoids the u32 overflow of the trailing frame-end byte (the + 1). + if frame_size > MAX_CONN_START_FRAME { + return Err(format!( + "AMQP connection.start frame size {frame_size} exceeds the {MAX_CONN_START_FRAME}-byte limit" + )); + } + // read connection.start body (frame payload + the trailing frame-end byte) + let mut conn_start_body = vec![0_u8; frame_size + 1]; stream .read_exact(&mut conn_start_body) .await