diff --git a/display/printf.rs b/display/printf.rs index c9358512..20543929 100644 --- a/display/printf.rs +++ b/display/printf.rs @@ -840,6 +840,10 @@ fn parse_integer_arg(arg: &[u8]) -> Result<(i64, bool), String> { .or_else(|| num_str.strip_prefix("0X")) { // Hexadecimal + // Reject if hex part starts with a sign (after we already handled the sign above) + if hex.starts_with('+') || hex.starts_with('-') { + return Err(format!("invalid number: {arg_str}")); + } match i64::from_str_radix(hex, 16) { Ok(n) => (n, true), Err(_) => { @@ -863,6 +867,10 @@ fn parse_integer_arg(arg: &[u8]) -> Result<(i64, bool), String> { .unwrap_or(false) { // Octal + // Reject if octal part starts with a sign (after we already handled the sign above) + if num_str.starts_with('+') || num_str.starts_with('-') { + return Err(format!("invalid number: {arg_str}")); + } let valid_len = num_str .chars() .take_while(|c| ('0'..='7').contains(c)) diff --git a/display/tests/printf/mod.rs b/display/tests/printf/mod.rs index 8ef68055..f7d0df79 100644 --- a/display/tests/printf/mod.rs +++ b/display/tests/printf/mod.rs @@ -532,3 +532,26 @@ fn test_float_missing_arg() { fn test_float_char_constant() { printf_test(&["%f", "'A"], "65.000000"); } + +// Regression tests for from_str_radix security issue +// These should fail because double signs are invalid +#[test] +#[should_panic] +fn test_reject_double_plus_hex() { + // This should fail: "+0x+55" should be rejected, not parsed as 0x55 + printf_test(&["%d", "+0x+55"], "85"); +} + +#[test] +#[should_panic] +fn test_reject_double_minus_hex() { + // This should fail: "-0x-55" should be rejected + printf_test(&["%d", "-0x-55"], "-85"); +} + +#[test] +#[should_panic] +fn test_reject_double_plus_octal() { + // This should fail: "+0+55" should be rejected + printf_test(&["%d", "+0+55"], "45"); +} diff --git a/file/find.rs b/file/find.rs index 93fab9dd..c26eb983 100644 --- a/file/find.rs +++ b/file/find.rs @@ -476,12 +476,14 @@ fn parse_perm_mode(mode_str: &str) -> Result { /// Parse a mode value (octal or symbolic) fn parse_mode_value(mode_str: &str) -> Result { - // Try octal first - if let Ok(m) = u32::from_str_radix(mode_str, 8) { - if m > 0o7777 { - return Err(format!("invalid mode: {}", mode_str)); + // Try octal first, but reject if it starts with a sign + if !mode_str.starts_with('+') && !mode_str.starts_with('-') { + if let Ok(m) = u32::from_str_radix(mode_str, 8) { + if m > 0o7777 { + return Err(format!("invalid mode: {}", mode_str)); + } + return Ok(m); } - return Ok(m); } // Parse as symbolic mode starting from 0 diff --git a/file/od.rs b/file/od.rs index 0e9a6406..eea1a30d 100644 --- a/file/od.rs +++ b/file/od.rs @@ -219,9 +219,19 @@ fn parse_count + FromStrRadix>( count: &str, ) -> Result> { if count.starts_with("0x") || count.starts_with("0X") { - T::from_str_radix(&count[2..], 16).map_err(|e| Box::new(e) as Box) + let hex_part = &count[2..]; + // Reject if hex part contains a sign + if hex_part.starts_with('+') || hex_part.starts_with('-') { + return Err("invalid hexadecimal number".into()); + } + T::from_str_radix(hex_part, 16).map_err(|e| Box::new(e) as Box) } else if count.starts_with('0') && count.len() > 1 { - T::from_str_radix(&count[1..], 8).map_err(|e| Box::new(e) as Box) + let oct_part = &count[1..]; + // Reject if octal part contains a sign + if oct_part.starts_with('+') || oct_part.starts_with('-') { + return Err("invalid octal number".into()); + } + T::from_str_radix(oct_part, 8).map_err(|e| Box::new(e) as Box) } else { count .parse::() @@ -279,6 +289,11 @@ fn parse_offset(offset: &str) -> Result { offset }; + // Reject if offset contains a sign (offsets should be unsigned) + if offset.starts_with('+') || offset.starts_with('-') { + return Err("invalid offset".parse::().unwrap_err()); + } + let parsed_offset = u64::from_str_radix(offset, base)?; Ok(parsed_offset * multiplier) diff --git a/pax/formats/cpio.rs b/pax/formats/cpio.rs index 181b1a7e..78752bfc 100644 --- a/pax/formats/cpio.rs +++ b/pax/formats/cpio.rs @@ -272,8 +272,12 @@ fn parse_header(header: &[u8; HEADER_SIZE], reader: &mut R) -> PaxResul fn parse_octal_field(bytes: &[u8]) -> PaxResult { let s = std::str::from_utf8(bytes) .map_err(|_| PaxError::InvalidHeader("invalid octal field".to_string()))?; - u64::from_str_radix(s.trim(), 8) - .map_err(|_| PaxError::InvalidHeader(format!("invalid octal: {}", s))) + let s = s.trim(); + // Reject if the octal string contains a sign + if s.starts_with('+') || s.starts_with('-') { + return Err(PaxError::InvalidHeader(format!("invalid octal: {}", s))); + } + u64::from_str_radix(s, 8).map_err(|_| PaxError::InvalidHeader(format!("invalid octal: {}", s))) } /// Parse filename, removing NUL terminator diff --git a/pax/formats/pax.rs b/pax/formats/pax.rs index 8bc75aa2..1a030297 100644 --- a/pax/formats/pax.rs +++ b/pax/formats/pax.rs @@ -837,6 +837,10 @@ fn parse_octal(bytes: &[u8]) -> PaxResult { if s.is_empty() { return Ok(0); } + // Reject if the octal string contains a sign + if s.starts_with('+') || s.starts_with('-') { + return Err(PaxError::InvalidHeader(format!("invalid octal: {}", s))); + } u64::from_str_radix(&s, 8).map_err(|_| PaxError::InvalidHeader(format!("invalid octal: {}", s))) } diff --git a/pax/formats/ustar.rs b/pax/formats/ustar.rs index 84862b18..fc1add95 100644 --- a/pax/formats/ustar.rs +++ b/pax/formats/ustar.rs @@ -273,6 +273,10 @@ fn parse_octal(bytes: &[u8]) -> PaxResult { if s.is_empty() { return Ok(0); } + // Reject if the octal string contains a sign + if s.starts_with('+') || s.starts_with('-') { + return Err(PaxError::InvalidHeader(format!("invalid octal: {}", s))); + } u64::from_str_radix(&s, 8).map_err(|_| PaxError::InvalidHeader(format!("invalid octal: {}", s))) } diff --git a/pax/main.rs b/pax/main.rs index 063df8fc..69666e8c 100644 --- a/pax/main.rs +++ b/pax/main.rs @@ -557,6 +557,11 @@ fn is_valid_tar_checksum(buf: &[u8]) -> bool { return false; } + // Reject if checksum contains a sign + if chksum_str.starts_with('+') || chksum_str.starts_with('-') { + return false; + } + let stored = match u32::from_str_radix(chksum_str, 8) { Ok(v) => v, Err(_) => return false, diff --git a/pax/modes/append.rs b/pax/modes/append.rs index 94b135e5..1687ff9a 100644 --- a/pax/modes/append.rs +++ b/pax/modes/append.rs @@ -124,6 +124,11 @@ fn is_valid_tar_checksum(header: &[u8]) -> bool { return false; } + // Reject if checksum contains a sign + if chksum_str.starts_with('+') || chksum_str.starts_with('-') { + return false; + } + let stored = match u32::from_str_radix(chksum_str, 8) { Ok(v) => v, Err(_) => return false, @@ -214,6 +219,10 @@ fn parse_octal(bytes: &[u8]) -> PaxResult { if s.is_empty() { return Ok(0); } + // Reject if the octal string contains a sign + if s.starts_with('+') || s.starts_with('-') { + return Err(PaxError::InvalidHeader(format!("invalid octal: {}", s))); + } u64::from_str_radix(s, 8).map_err(|_| PaxError::InvalidHeader(format!("invalid octal: {}", s))) } diff --git a/pax/multivolume.rs b/pax/multivolume.rs index 575286c8..0f66910c 100644 --- a/pax/multivolume.rs +++ b/pax/multivolume.rs @@ -745,6 +745,10 @@ fn parse_octal(bytes: &[u8]) -> PaxResult { if s.is_empty() { return Ok(0); } + // Reject if the octal string contains a sign + if s.starts_with('+') || s.starts_with('-') { + return Err(PaxError::InvalidHeader(format!("invalid octal: {}", s))); + } u64::from_str_radix(s, 8).map_err(|_| PaxError::InvalidHeader(format!("invalid octal: {}", s))) } diff --git a/plib/src/modestr.rs b/plib/src/modestr.rs index 68498c00..ab84874b 100644 --- a/plib/src/modestr.rs +++ b/plib/src/modestr.rs @@ -72,8 +72,11 @@ enum ParseState { } pub fn parse(mode: &str) -> Result { - if let Ok(m) = u32::from_str_radix(mode, 8) { - return Ok(ChmodMode::Absolute(m, mode.len() as u32)); + // Try parsing as octal, but reject if it starts with a sign + if !mode.starts_with('+') && !mode.starts_with('-') { + if let Ok(m) = u32::from_str_radix(mode, 8) { + return Ok(ChmodMode::Absolute(m, mode.len() as u32)); + } } let mut done_with_char; diff --git a/sh/builtin/umask.rs b/sh/builtin/umask.rs index 99ab04f7..f05a2af5 100644 --- a/sh/builtin/umask.rs +++ b/sh/builtin/umask.rs @@ -56,6 +56,10 @@ impl BuiltinUtility for Umask { } else if option_parser.next_argument() == args.len() - 1 { // TODO: support symbolic umask let new_umask = &args[option_parser.next_argument()]; + // Reject if umask contains a sign + if new_umask.starts_with('+') || new_umask.starts_with('-') { + return Err(format!("umask: invalid mask '{new_umask}'").into()); + } let new_umask = u32::from_str_radix(new_umask, 8) .map_err(|_| format!("umask: invalid mask '{new_umask}'"))?; if new_umask > 0o777 { diff --git a/sys/ipcrm.rs b/sys/ipcrm.rs index 69053f56..fc08a6ae 100644 --- a/sys/ipcrm.rs +++ b/sys/ipcrm.rs @@ -20,6 +20,10 @@ use libc::{semctl, semget, shmctl, shmget, shmid_ds}; /// Parse an IPC key value that may be decimal or hexadecimal (0x prefix) fn parse_ipc_key(s: &str) -> Result { if let Some(hex) = s.strip_prefix("0x").or_else(|| s.strip_prefix("0X")) { + // Reject if hex part starts with a sign + if hex.starts_with('+') || hex.starts_with('-') { + return Err("invalid hex key: unexpected sign after 0x prefix".to_string()); + } u32::from_str_radix(hex, 16) .map(|v| v as i32) .map_err(|e| format!("invalid hex key: {}", e)) diff --git a/text/tr.rs b/text/tr.rs index 86c9761d..4e8ff3fd 100644 --- a/text/tr.rs +++ b/text/tr.rs @@ -625,6 +625,11 @@ mod parsing { let char_repetition = match repeat_string.as_str() { "" => CharRepetition::AsManyAsNeeded, st => { + // Reject if repeat count starts with a sign + if st.starts_with('+') || st.starts_with('-') { + return Err(format!("invalid repeat count '{st}' in [c*n] construct",)); + } + let radix = if st.starts_with('0') { // Octal 8_u32 diff --git a/xform/uudecode.rs b/xform/uudecode.rs index 6d0571da..6424553f 100644 --- a/xform/uudecode.rs +++ b/xform/uudecode.rs @@ -59,7 +59,12 @@ impl Header { panic!("Invalid encoding type"); }; - let lower_perm_bits = u32::from_str_radix(split[1], 8).expect("Invalid permission value"); + let mode_str = split[1]; + // Reject if mode contains a sign + if mode_str.starts_with('+') || mode_str.starts_with('-') { + panic!("Invalid permission value: unexpected sign"); + } + let lower_perm_bits = u32::from_str_radix(mode_str, 8).expect("Invalid permission value"); let out = PathBuf::from(split[2]); Self {