Fix from_str_radix misuse in user input parsing

Co-authored-by: jgarzik <494411+jgarzik@users.noreply.github.com>

Author: Copilot

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

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");
+}

file/find.rs

@@ -476,12 +476,14 @@ fn parse_perm_mode(mode_str: &str) -> Result<PermMode, String> {
 
 /// Parse a mode value (octal or symbolic)
 fn parse_mode_value(mode_str: &str) -> Result<u32, String> {
-    // 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

file/od.rs

@@ -219,9 +219,19 @@ fn parse_count<T: FromStr<Err = ParseIntError> + FromStrRadix>(
     count: &str,
 ) -> Result<T, Box<dyn std::error::Error>> {
     if count.starts_with("0x") || count.starts_with("0X") {
-        T::from_str_radix(&count[2..], 16).map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
+        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<dyn std::error::Error>)
     } else if count.starts_with('0') && count.len() > 1 {
-        T::from_str_radix(&count[1..], 8).map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
+        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<dyn std::error::Error>)
     } else {
         count
             .parse::<T>()
@@ -279,6 +289,11 @@ fn parse_offset(offset: &str) -> Result<u64, ParseIntError> {
         offset
     };
 
+    // Reject if offset contains a sign (offsets should be unsigned)
+    if offset.starts_with('+') || offset.starts_with('-') {
+        return Err("invalid offset".parse::<u64>().unwrap_err());
+    }
+
     let parsed_offset = u64::from_str_radix(offset, base)?;
 
     Ok(parsed_offset * multiplier)

plib/src/modestr.rs

@@ -72,8 +72,11 @@ enum ParseState {
 }
 
 pub fn parse(mode: &str) -> Result<ChmodMode, String> {
-    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;

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 {

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<i32, String> {
     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))

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