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');