From 683ba7b2cc9afa6037d277597e9efa21417f16ab Mon Sep 17 00:00:00 2001 From: Kanishk Sachan Date: Thu, 25 Jun 2026 17:34:30 +0100 Subject: [PATCH 1/2] fix(cksum): reject absurdly large SHAKE --length instead of aborting (fixes #12869) cksum -a shake128/shake256 --length accepted any usize value in bits with no upper bound. A value like 10011111117721172727 overflows the digest buffer allocation in Digest::result(), causing a Rust memory allocation failure and process abort (exit 134) instead of a clean error. Cap --length for SHAKE algorithms at 8192 bits (1 KiB of output), comfortably above any real use case, and report a GNU-style error when exceeded. Add a regression test. --- src/uu/cksum/src/cksum.rs | 28 +++++++++++++-------- src/uucore/src/lib/features/checksum/mod.rs | 7 ++++++ tests/by-util/test_cksum.rs | 18 +++++++++++++ 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/uu/cksum/src/cksum.rs b/src/uu/cksum/src/cksum.rs index 9af2f6f00ff..4aff7bdac87 100644 --- a/src/uu/cksum/src/cksum.rs +++ b/src/uu/cksum/src/cksum.rs @@ -12,12 +12,12 @@ use uu_checksum_common::{ChecksumCommand, checksum_main, default_checksum_app, o use uucore::checksum::compute::OutputFormat; use uucore::checksum::{ - AlgoKind, BlakeLength, ChecksumError, HashLength, parse_blake_length, + AlgoKind, BlakeLength, ChecksumError, HashLength, MAX_SHAKE_LENGTH_BITS, parse_blake_length, sanitize_sha2_sha3_length_str, }; use uucore::error::UResult; use uucore::hardware::{HasHardwareFeatures as _, SimdPolicy}; -use uucore::translate; +use uucore::{show_error, translate}; /// Print CPU hardware capability detection information to stderr /// 2>/dev/full does not abort @@ -58,15 +58,21 @@ fn maybe_sanitize_length( sanitize_sha2_sha3_length_str(algo, s_len).map(Some) } - // SHAKE128 and SHAKE256 algorithms optionally take a bit length. No - // validation is performed on this length, any value is valid. If the - // given length is not a multiple of 8, the last byte of the output - // will have its extra bits set to zero. - (Some(AlgoKind::Shake128 | AlgoKind::Shake256), Some(len)) => match len.parse::() { - Ok(0) => Ok(None), - Ok(l) => Ok(Some(HashLength::from_bits(l))), - Err(_) => Err(ChecksumError::InvalidLength(len.into()).into()), - }, + // SHAKE128 and SHAKE256 algorithms optionally take a bit length. If + // the given length is not a multiple of 8, the last byte of the + // output will have its extra bits set to zero. The length is capped + // to avoid an out-of-memory abort when allocating the digest buffer. + (Some(algo @ (AlgoKind::Shake128 | AlgoKind::Shake256)), Some(len)) => { + match len.parse::() { + Ok(0) => Ok(None), + Ok(l) if l > MAX_SHAKE_LENGTH_BITS => { + show_error!("{}", ChecksumError::InvalidLength(len.to_string())); + Err(ChecksumError::LengthTooBigForShake(algo.to_uppercase().into()).into()) + } + Ok(l) => Ok(Some(HashLength::from_bits(l))), + Err(_) => Err(ChecksumError::InvalidLength(len.into()).into()), + } + } // For BLAKE, if a length is provided, validate it. (Some(algo @ (AlgoKind::Blake2b | AlgoKind::Blake3)), Some(len)) => { diff --git a/src/uucore/src/lib/features/checksum/mod.rs b/src/uucore/src/lib/features/checksum/mod.rs index 45d4c8b5918..c74e73eae14 100644 --- a/src/uucore/src/lib/features/checksum/mod.rs +++ b/src/uucore/src/lib/features/checksum/mod.rs @@ -41,6 +41,11 @@ pub const ALGORITHM_OPTIONS_SM3: &str = "sm3"; pub const ALGORITHM_OPTIONS_SHAKE128: &str = "shake128"; pub const ALGORITHM_OPTIONS_SHAKE256: &str = "shake256"; +/// SHAKE has no inherent maximum output length, but an unbounded `--length` +/// causes the digest buffer allocation to abort the process (#12869). 8192 +/// bits (1 KiB of output) comfortably covers real use cases. +pub const MAX_SHAKE_LENGTH_BITS: usize = 8192; + pub const SUPPORTED_ALGORITHMS: [&str; 17] = [ ALGORITHM_OPTIONS_SYSV, ALGORITHM_OPTIONS_BSD, @@ -455,6 +460,8 @@ pub enum ChecksumError { InvalidLength(String), #[error("maximum digest length for {} is 512 bits", .0.quote())] LengthTooBigForBlake(String), + #[error("maximum digest length for {} is {MAX_SHAKE_LENGTH_BITS} bits", .0.quote())] + LengthTooBigForShake(String), #[error("length is not a multiple of 8")] LengthNotMultipleOf8, #[error("digest length for {} must be 224, 256, 384, or 512", .0.quote())] diff --git a/tests/by-util/test_cksum.rs b/tests/by-util/test_cksum.rs index 8f81552b3bb..467e024352d 100644 --- a/tests/by-util/test_cksum.rs +++ b/tests/by-util/test_cksum.rs @@ -3290,6 +3290,24 @@ fn test_check_shake128_no_length() { .stderr_only("cksum: 'standard input': no properly formatted checksum lines found\n"); } +#[test] +fn test_shake_length_too_large_does_not_panic() { + for algo in ["shake128", "shake256"] { + new_ucmd!() + .arg("--algorithm") + .arg(algo) + .arg("--length=10011111117721172727") + .pipe_in("xxx") + .fails_with_code(1) + .no_stdout() + .stderr_contains("invalid length: '10011111117721172727'") + .stderr_contains(format!( + "maximum digest length for '{}' is 8192 bits", + algo.to_uppercase() + )); + } +} + #[test] fn test_check_shake256_no_length() { const INPUT_SHAKE256_CORRECT_LEN: &str = "SHAKE256 (bar) = 2fa631503c3ea5fe85131dbfa24805185474740e6dcb5f2a64f69d932bcb55f7b24958f3e3c4cc0e71f1fe6f054cd3fb28b9efb62b4f8f3fbe6d50d90f5c6eba"; From c3bb6a7a713045370ec48e4b602dc499ad5b866b Mon Sep 17 00:00:00 2001 From: Kanishk Sachan Date: Sat, 27 Jun 2026 00:16:53 +0100 Subject: [PATCH 2/2] fix(cksum): catch OOM via try_reserve instead of capping SHAKE length Per review feedback, replace the fixed upper bound on --length with a recoverable error from Vec::try_reserve_exact. GNU utils don't impose arbitrary "realistic use case" limits, so Digest::result() now returns io::Result and reports an OutOfMemory error instead of letting the allocator abort the process when the requested length is absurdly large. Fixes #12869 --- src/uu/cksum/src/cksum.rs | 28 ++++++--------- src/uucore/src/lib/features/checksum/mod.rs | 9 +---- src/uucore/src/lib/features/sum.rs | 40 +++++++++++++-------- tests/by-util/test_cksum.rs | 34 +++++++++--------- 4 files changed, 54 insertions(+), 57 deletions(-) diff --git a/src/uu/cksum/src/cksum.rs b/src/uu/cksum/src/cksum.rs index 4aff7bdac87..9af2f6f00ff 100644 --- a/src/uu/cksum/src/cksum.rs +++ b/src/uu/cksum/src/cksum.rs @@ -12,12 +12,12 @@ use uu_checksum_common::{ChecksumCommand, checksum_main, default_checksum_app, o use uucore::checksum::compute::OutputFormat; use uucore::checksum::{ - AlgoKind, BlakeLength, ChecksumError, HashLength, MAX_SHAKE_LENGTH_BITS, parse_blake_length, + AlgoKind, BlakeLength, ChecksumError, HashLength, parse_blake_length, sanitize_sha2_sha3_length_str, }; use uucore::error::UResult; use uucore::hardware::{HasHardwareFeatures as _, SimdPolicy}; -use uucore::{show_error, translate}; +use uucore::translate; /// Print CPU hardware capability detection information to stderr /// 2>/dev/full does not abort @@ -58,21 +58,15 @@ fn maybe_sanitize_length( sanitize_sha2_sha3_length_str(algo, s_len).map(Some) } - // SHAKE128 and SHAKE256 algorithms optionally take a bit length. If - // the given length is not a multiple of 8, the last byte of the - // output will have its extra bits set to zero. The length is capped - // to avoid an out-of-memory abort when allocating the digest buffer. - (Some(algo @ (AlgoKind::Shake128 | AlgoKind::Shake256)), Some(len)) => { - match len.parse::() { - Ok(0) => Ok(None), - Ok(l) if l > MAX_SHAKE_LENGTH_BITS => { - show_error!("{}", ChecksumError::InvalidLength(len.to_string())); - Err(ChecksumError::LengthTooBigForShake(algo.to_uppercase().into()).into()) - } - Ok(l) => Ok(Some(HashLength::from_bits(l))), - Err(_) => Err(ChecksumError::InvalidLength(len.into()).into()), - } - } + // SHAKE128 and SHAKE256 algorithms optionally take a bit length. No + // validation is performed on this length, any value is valid. If the + // given length is not a multiple of 8, the last byte of the output + // will have its extra bits set to zero. + (Some(AlgoKind::Shake128 | AlgoKind::Shake256), Some(len)) => match len.parse::() { + Ok(0) => Ok(None), + Ok(l) => Ok(Some(HashLength::from_bits(l))), + Err(_) => Err(ChecksumError::InvalidLength(len.into()).into()), + }, // For BLAKE, if a length is provided, validate it. (Some(algo @ (AlgoKind::Blake2b | AlgoKind::Blake3)), Some(len)) => { diff --git a/src/uucore/src/lib/features/checksum/mod.rs b/src/uucore/src/lib/features/checksum/mod.rs index c74e73eae14..25f93bb1865 100644 --- a/src/uucore/src/lib/features/checksum/mod.rs +++ b/src/uucore/src/lib/features/checksum/mod.rs @@ -41,11 +41,6 @@ pub const ALGORITHM_OPTIONS_SM3: &str = "sm3"; pub const ALGORITHM_OPTIONS_SHAKE128: &str = "shake128"; pub const ALGORITHM_OPTIONS_SHAKE256: &str = "shake256"; -/// SHAKE has no inherent maximum output length, but an unbounded `--length` -/// causes the digest buffer allocation to abort the process (#12869). 8192 -/// bits (1 KiB of output) comfortably covers real use cases. -pub const MAX_SHAKE_LENGTH_BITS: usize = 8192; - pub const SUPPORTED_ALGORITHMS: [&str; 17] = [ ALGORITHM_OPTIONS_SYSV, ALGORITHM_OPTIONS_BSD, @@ -460,8 +455,6 @@ pub enum ChecksumError { InvalidLength(String), #[error("maximum digest length for {} is 512 bits", .0.quote())] LengthTooBigForBlake(String), - #[error("maximum digest length for {} is {MAX_SHAKE_LENGTH_BITS} bits", .0.quote())] - LengthTooBigForShake(String), #[error("length is not a multiple of 8")] LengthNotMultipleOf8, #[error("digest length for {} must be 224, 256, 384, or 512", .0.quote())] @@ -547,7 +540,7 @@ pub fn digest_reader( let output_size = io::copy(reader, &mut digest_writer)? as usize; digest_writer.finalize(); - Ok((digest.result(), output_size)) + Ok((digest.result()?, output_size)) } pub enum BlakeLength<'s> { diff --git a/src/uucore/src/lib/features/sum.rs b/src/uucore/src/lib/features/sum.rs index 85a3d9f58c0..c9cdaa33333 100644 --- a/src/uucore/src/lib/features/sum.rs +++ b/src/uucore/src/lib/features/sum.rs @@ -67,13 +67,24 @@ pub trait Digest { self.output_bits().div_ceil(8) } - fn result(&mut self) -> DigestOutput { - let mut buf: Vec = vec![0; self.output_bytes()]; + fn result(&mut self) -> io::Result { + let mut buf: Vec = Vec::new(); + try_reserve_zeroed(&mut buf, self.output_bytes())?; self.hash_finalize(&mut buf); - DigestOutput::Vec(buf) + Ok(DigestOutput::Vec(buf)) } } +/// Grows `buf` to `len` zero bytes, returning an [`io::Error`] with +/// [`io::ErrorKind::OutOfMemory`] instead of aborting the process if the +/// allocation can't be satisfied (e.g. an absurdly large `--length`, #12869). +fn try_reserve_zeroed(buf: &mut Vec, len: usize) -> io::Result<()> { + buf.try_reserve_exact(len) + .map_err(|e| io::Error::new(io::ErrorKind::OutOfMemory, e))?; + buf.resize(len, 0); + Ok(()) +} + /// first element of the tuple is the blake2b state /// second is the number of output bits pub struct Blake2b { @@ -246,12 +257,12 @@ impl Digest for Crc { out.copy_from_slice(&self.digest.finalize().to_ne_bytes()); } - fn result(&mut self) -> DigestOutput { + fn result(&mut self) -> io::Result { let mut out: [u8; 8] = [0; 8]; self.hash_finalize(&mut out); let x = u64::from_ne_bytes(out); - DigestOutput::Crc((x & (u32::MAX as u64)) as u32) + Ok(DigestOutput::Crc((x & (u32::MAX as u64)) as u32)) } fn reset(&mut self) { @@ -297,10 +308,10 @@ impl Digest for CRC32B { 32 } - fn result(&mut self) -> DigestOutput { + fn result(&mut self) -> io::Result { let mut out = [0; 4]; self.hash_finalize(&mut out); - DigestOutput::Crc(u32::from_be_bytes(out)) + Ok(DigestOutput::Crc(u32::from_be_bytes(out))) } } @@ -321,10 +332,10 @@ impl Digest for Bsd { out.copy_from_slice(&self.state.to_ne_bytes()); } - fn result(&mut self) -> DigestOutput { + fn result(&mut self) -> io::Result { let mut out = [0; 2]; self.hash_finalize(&mut out); - DigestOutput::U16(self.state) + Ok(DigestOutput::U16(self.state)) } fn reset(&mut self) { @@ -354,10 +365,10 @@ impl Digest for SysV { out.copy_from_slice(&(self.state as u16).to_ne_bytes()); } - fn result(&mut self) -> DigestOutput { + fn result(&mut self) -> io::Result { let mut out = [0; 2]; self.hash_finalize(&mut out); - DigestOutput::U16((self.state & (u16::MAX as u32)) as u16) + Ok(DigestOutput::U16((self.state & (u16::MAX as u32)) as u16)) } fn reset(&mut self) { @@ -442,10 +453,11 @@ macro_rules! impl_digest_shake { self.bit_size } - fn result(&mut self) -> DigestOutput { - let mut bytes = vec![0; self.output_bits().div_ceil(8)]; + fn result(&mut self) -> io::Result { + let mut bytes = Vec::new(); + try_reserve_zeroed(&mut bytes, self.output_bits().div_ceil(8))?; self.hash_finalize(&mut bytes); - DigestOutput::Vec(bytes) + Ok(DigestOutput::Vec(bytes)) } } }; diff --git a/tests/by-util/test_cksum.rs b/tests/by-util/test_cksum.rs index 467e024352d..3fc19dcd9aa 100644 --- a/tests/by-util/test_cksum.rs +++ b/tests/by-util/test_cksum.rs @@ -3290,24 +3290,6 @@ fn test_check_shake128_no_length() { .stderr_only("cksum: 'standard input': no properly formatted checksum lines found\n"); } -#[test] -fn test_shake_length_too_large_does_not_panic() { - for algo in ["shake128", "shake256"] { - new_ucmd!() - .arg("--algorithm") - .arg(algo) - .arg("--length=10011111117721172727") - .pipe_in("xxx") - .fails_with_code(1) - .no_stdout() - .stderr_contains("invalid length: '10011111117721172727'") - .stderr_contains(format!( - "maximum digest length for '{}' is 8192 bits", - algo.to_uppercase() - )); - } -} - #[test] fn test_check_shake256_no_length() { const INPUT_SHAKE256_CORRECT_LEN: &str = "SHAKE256 (bar) = 2fa631503c3ea5fe85131dbfa24805185474740e6dcb5f2a64f69d932bcb55f7b24958f3e3c4cc0e71f1fe6f054cd3fb28b9efb62b4f8f3fbe6d50d90f5c6eba"; @@ -3333,6 +3315,22 @@ fn test_check_shake256_no_length() { .stderr_only("cksum: 'standard input': no properly formatted checksum lines found\n"); } +#[test] +fn test_shake_extremely_large_length_does_not_abort() { + // Regression test for #12869: an absurdly large `--length` used to + // trigger an unguarded allocation that aborts the process instead of + // returning a normal error. + new_ucmd!() + .args(&[ + "--algorithm", + "shake128", + "--length", + "10011111117721172727", + ]) + .pipe_in("") + .fails(); +} + #[template] #[rstest] #[case::no_length(