From 44f265b0e260aafca73540e9dbf3dcf592ab69dc Mon Sep 17 00:00:00 2001 From: contributor Date: Sat, 8 Aug 2026 10:17:01 +0200 Subject: [PATCH] xargs: cap single-argument growth to avoid OOM on unterminated input Reading from an input that never produces a delimiter (such as `/dev/full`, which yields an endless stream of NUL bytes) made the argument readers accumulate a single argument without bound until the process was OOM-killed, because nothing bounded the growth between delimiters. Bound each accumulated argument by a multiple of the effective command-line character budget (the same budget the `-s` / system ARG_MAX limiters enforce), reporting `argument line too long` once an argument exceeds it. The cap is sized generously (4x the budget) so that no argument the size limiters would ever accept can be rejected by the reader. The byte-delimited reader previously relied on `BufReader::read_until` to loop internally; driving the buffered reader by hand (so the cap can be checked) surfaces `Interrupted` reads to the caller, so retry those explicitly to preserve the previous behaviour. --- src/xargs/mod.rs | 177 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 148 insertions(+), 29 deletions(-) diff --git a/src/xargs/mod.rs b/src/xargs/mod.rs index ff833335..18080b85 100644 --- a/src/xargs/mod.rs +++ b/src/xargs/mod.rs @@ -234,9 +234,22 @@ impl MaxCharsCommandSizeLimiter { /// kernel additionally charges the argv/envp pointers (since Linux commit /// 98da7d08850f) against the limit, which are not part of the character /// count. An explicit -s can still go beyond this, up to the system limit. - fn new_default(env: &HashMap) -> Self { + /// The effective per-command-line character budget that the size limiters + /// enforce: the smaller of the user-specified (or default) `-s` value and + /// the system `ARG_MAX`-derived limit (which is always added). Used to + /// derive a bound on a single accumulated argument so the reader cannot + /// grow it without limit on an unterminated input such as `/dev/full`. + fn effective_max_chars( + options_max_chars: Option, + env: &HashMap, + ) -> usize { const DEFAULT_MAX_CHARS: usize = 128 * 1024; - Self::new(Self::new_system(env).max_chars.min(DEFAULT_MAX_CHARS)) + let system = Self::new_system(env).max_chars; + if let Some(user) = options_max_chars { + user.min(system) + } else { + DEFAULT_MAX_CHARS.min(system) + } } #[cfg(not(any(unix, windows)))] @@ -548,22 +561,34 @@ impl CommandBuilder<'_> { } trait ArgumentReader { - fn next(&mut self) -> io::Result>; + fn next(&mut self) -> Result, XargsError>; } +/// A single accumulated argument may not grow without bound: an input that +/// never produces a delimiter (e.g. reading from `/dev/full`, which yields an +/// infinite stream of NUL bytes) would otherwise make the reader accumulate +/// forever until the process is OOM-killed. We therefore cap how many bytes a +/// single argument may accumulate before declaring it too large, mirroring +/// GNU xargs' "argument line too long" error. The cap is generously sized (a +/// multiple of the configured command-line character budget) so that no +/// argument the size limiters would accept can ever be rejected here. +const ARG_SIZE_OVERFLOW_MULTIPLIER: usize = 4; + struct WhitespaceDelimitedArgumentReader { rd: R, pending: Vec, + max_arg_size: usize, } impl WhitespaceDelimitedArgumentReader where R: Read, { - fn new(rd: R) -> Self { + fn new(rd: R, max_arg_size: usize) -> Self { Self { rd, pending: vec![], + max_arg_size, } } } @@ -572,7 +597,7 @@ impl ArgumentReader for WhitespaceDelimitedArgumentReader where R: Read, { - fn next(&mut self) -> io::Result> { + fn next(&mut self) -> Result, XargsError> { enum Escape { Slash, Quote(u8), @@ -594,16 +619,16 @@ where match self.rd.read(&mut pending[..]) { Ok(bytes_read) => break bytes_read, Err(e) if e.kind() == io::ErrorKind::Interrupted => {} - Err(e) => return Err(e), + Err(e) => return Err(XargsError::from(e)), } }; if bytes_read == 0 { if let Some(Escape::Quote(q)) = &escape { - return Err(io::Error::new( + return Err(XargsError::from(io::Error::new( io::ErrorKind::InvalidInput, format!("Unterminated quote: {q}"), - )); + ))); } if i == 0 { return Ok(None); @@ -635,6 +660,14 @@ where } i += 1; + + // Guard against an input that never produces a delimiter (such as + // `/dev/full`, which supplies an endless stream of NUL bytes): if + // a single argument grows past the configured size budget, error + // out instead of accumulating until the process is OOM-killed. + if result.len() > self.max_arg_size { + return Err(XargsError::ArgumentTooLarge); + } } if i < pending.len() { @@ -655,16 +688,18 @@ where struct ByteDelimitedArgumentReader { rd: BufReader, delimiter: u8, + max_arg_size: usize, } impl ByteDelimitedArgumentReader where R: Read, { - fn new(rd: R, delimiter: u8) -> Self { + fn new(rd: R, delimiter: u8, max_arg_size: usize) -> Self { Self { rd: BufReader::new(rd), delimiter, + max_arg_size, } } } @@ -673,10 +708,45 @@ impl ArgumentReader for ByteDelimitedArgumentReader where R: Read, { - fn next(&mut self) -> io::Result> { + fn next(&mut self) -> Result, XargsError> { Ok(loop { let mut buf = vec![]; - let bytes_read = self.rd.read_until(self.delimiter, &mut buf)?; + // Bounded replacement for `BufReader::read_until`: the library + // helper would loop forever (and grow `buf` without bound) on an + // input that never yields the delimiter, such as `/dev/full` + // (endless NUL bytes) with a non-NUL delimiter. Drive the buffered + // reader by hand so we can cap how large a single argument may + // grow before declaring it too large. + loop { + // `fill_buf` surfaces `Interrupted` to the caller (unlike + // `read_until`, which retried internally); retry here so a + // transient `Interrupted` read does not abort the argument. + let available = loop { + match self.rd.fill_buf() { + Ok(b) => break b, + Err(e) if e.kind() == io::ErrorKind::Interrupted => {} + Err(e) => return Err(XargsError::from(e)), + } + }; + if available.is_empty() { + break; + } + let pos = available.iter().position(|&b| b == self.delimiter); + let take = match pos { + Some(i) => i + 1, + None => available.len(), + }; + buf.extend_from_slice(&available[..take]); + self.rd.consume(take); + if buf.len() > self.max_arg_size { + return Err(XargsError::ArgumentTooLarge); + } + if pos.is_some() { + break; + } + } + + let bytes_read = buf.len(); if bytes_read > 0 { let need_to_trim_delimiter = buf[buf.len() - 1] == self.delimiter; let bytes = if need_to_trim_delimiter { @@ -717,7 +787,7 @@ impl EofArgumentReader { } impl ArgumentReader for EofArgumentReader { - fn next(&mut self) -> io::Result> { + fn next(&mut self) -> Result, XargsError> { Ok(if self.eof_found { None } else { @@ -744,7 +814,7 @@ enum XargsError { impl Display for XargsError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::ArgumentTooLarge => write!(f, "Argument too large"), + Self::ArgumentTooLarge => write!(f, "argument line too long"), Self::CommandExecution(e) => write!(f, "{e}"), Self::Io(e) => write!(f, "{e}"), Self::Untyped(s) => write!(f, "{s}"), @@ -1168,11 +1238,8 @@ fn do_xargs(args: &[&str]) -> Result { if let Some(max_lines) = options.max_lines { limiters.add(MaxLinesCommandSizeLimiter::new(max_lines)); } - if let Some(max_chars) = options.max_chars { - limiters.add(MaxCharsCommandSizeLimiter::new(max_chars)); - } else { - limiters.add(MaxCharsCommandSizeLimiter::new_default(&env)); - } + let effective_max_chars = MaxCharsCommandSizeLimiter::effective_max_chars(options.max_chars, &env); + limiters.add(MaxCharsCommandSizeLimiter::new(effective_max_chars)); limiters.add(MaxCharsCommandSizeLimiter::new_system(&env)); let mut builder_options = @@ -1189,10 +1256,23 @@ fn do_xargs(args: &[&str]) -> Result { Box::new(io::stdin()) }; + // Cap how large a single accumulated argument may grow before the reader + // errors out. An input that never produces a delimiter (such as + // `/dev/full`, which yields an endless stream of NUL bytes) would + // otherwise make the reader accumulate without bound until the process is + // OOM-killed. Size the cap from the same command-line budget the size + // limiters enforce, generously multiplied so that no argument the limiters + // would accept can ever be rejected by the reader. + let max_arg_size = effective_max_chars * ARG_SIZE_OVERFLOW_MULTIPLIER; + let mut args: Box = if let Some(delimiter) = options.delimiter { - Box::new(ByteDelimitedArgumentReader::new(args_file, delimiter)) + Box::new(ByteDelimitedArgumentReader::new( + args_file, + delimiter, + max_arg_size, + )) } else { - Box::new(WhitespaceDelimitedArgumentReader::new(args_file)) + Box::new(WhitespaceDelimitedArgumentReader::new(args_file, max_arg_size)) }; if let Some(eof_delimiter) = options.eof_delimiter { @@ -1356,7 +1436,9 @@ mod tests { fn test_default_chars_limiter_caps_system_limit() { let env = HashMap::new(); let system = MaxCharsCommandSizeLimiter::new_system(&env); - let default = MaxCharsCommandSizeLimiter::new_default(&env); + let default = MaxCharsCommandSizeLimiter::new( + MaxCharsCommandSizeLimiter::effective_max_chars(None, &env), + ); // The default never exceeds 128 KiB nor what the system allows. assert!(default.max_chars <= 128 * 1024); assert!(default.max_chars <= system.max_chars); @@ -1474,7 +1556,7 @@ mod tests { Chunk::Error(io::ErrorKind::Interrupted), Chunk::Data(b"\\\t\\ o 'ab"), Chunk::Data(b" \"' \"xy' z\""), - ])); + ]), usize::MAX); assert_eq!(reader.next().unwrap().unwrap(), make_arg_soft("abc")); assert_eq!(reader.next().unwrap().unwrap(), make_arg_hard("def")); @@ -1492,7 +1574,7 @@ mod tests { let reader = WhitespaceDelimitedArgumentReader::new(ChunkReader::new(vec![Chunk::Data( b"abc def ghi", - )])); + )]), usize::MAX); let mut wrapper = EofArgumentReader::new(Box::new(reader), &filter); assert_eq!(wrapper.next().unwrap().unwrap(), make_arg_soft("abc")); assert_eq!(wrapper.next().unwrap(), None); @@ -1500,7 +1582,7 @@ mod tests { let reader = WhitespaceDelimitedArgumentReader::new(ChunkReader::new(vec![Chunk::Data( b"abc define undef undefined ghi", - )])); + )]), usize::MAX); let mut wrapper = EofArgumentReader::new(Box::new(reader), &filter); assert_eq!(wrapper.next().unwrap().unwrap(), make_arg_soft("abc")); assert_eq!(wrapper.next().unwrap().unwrap(), make_arg_soft("define")); @@ -1517,14 +1599,14 @@ mod tests { Chunk::Data(b"ghi "), Chunk::Data(b"def "), Chunk::Error(io::ErrorKind::BrokenPipe), - ])); + ]), usize::MAX); let mut wrapper = EofArgumentReader::new(Box::new(reader), &filter); assert_eq!(wrapper.next().unwrap().unwrap(), make_arg_soft("abc")); assert_eq!(wrapper.next().unwrap().unwrap(), make_arg_soft("deF")); - assert_eq!( - wrapper.next().err().unwrap().kind(), - io::ErrorKind::BrokenPipe - ); + match wrapper.next().err().unwrap() { + XargsError::Io(e) => assert_eq!(e.kind(), io::ErrorKind::BrokenPipe), + other => panic!("expected Io(BrokenPipe), got {other:?}"), + } assert_eq!(wrapper.next().unwrap().unwrap(), make_arg_soft("ghi")); assert_eq!(wrapper.next().unwrap(), None); assert_eq!(wrapper.next().unwrap(), None); @@ -1541,6 +1623,7 @@ mod tests { Chunk::Data(b"!ij"), ]), b'!', + usize::MAX, ); assert_eq!(reader.next().unwrap().unwrap(), make_arg_hard("abc")); @@ -1551,6 +1634,42 @@ mod tests { assert_eq!(reader.next().unwrap(), None); } + /// An input that never produces a delimiter (such as `/dev/full`, which + /// yields an endless stream of NUL bytes, or any finite stream that simply + /// lacks the delimiter byte) must not make the reader accumulate a single + /// argument without bound until the process is OOM-killed. Once the + /// accumulated argument exceeds the configured size budget, the reader + /// reports `ArgumentTooLarge` instead of looping forever. + #[test] + fn test_whitespace_reader_caps_unbounded_argument() { + let reader = WhitespaceDelimitedArgumentReader::new( + // A long run of non-whitespace bytes with no terminator at all. + ChunkReader::new(vec![Chunk::Data(b"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")]), + 8, + ); + let mut reader = reader; + assert!(matches!( + reader.next(), + Err(XargsError::ArgumentTooLarge), + )); + } + + #[test] + fn test_byte_reader_caps_unbounded_argument() { + // The delimiter never appears, so the reader would otherwise read the + // whole (terminator-less) stream into one argument. + let reader = ByteDelimitedArgumentReader::new( + ChunkReader::new(vec![Chunk::Data(b"yyyyyyyyyyyyyyyyyyyyyyyy")]), + b'!', + 8, + ); + let mut reader = reader; + assert!(matches!( + reader.next(), + Err(XargsError::ArgumentTooLarge), + )); + } + #[test] fn test_delimiter_parsing() { assert_eq!(parse_delimiter("a").unwrap(), b'a');