Merge pull request #463 from rustcoreutils/copilot/fix-from-str-radix-usage

Fix from_str_radix accepting leading signs in non-decimal radixes

Author: jgarzik

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)

pax/formats/cpio.rs

@@ -272,8 +272,12 @@ fn parse_header<R: Read>(header: &[u8; HEADER_SIZE], reader: &mut R) -> PaxResul
 fn parse_octal_field(bytes: &[u8]) -> PaxResult<u64> {
     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

pax/formats/pax.rs

@@ -837,6 +837,10 @@ fn parse_octal(bytes: &[u8]) -> PaxResult<u64> {
     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)))
 }
 

pax/formats/ustar.rs

@@ -273,6 +273,10 @@ fn parse_octal(bytes: &[u8]) -> PaxResult<u64> {
     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)))
 }
 

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,

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<u64> {
     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)))
 }
 

pax/multivolume.rs

@@ -745,6 +745,10 @@ fn parse_octal(bytes: &[u8]) -> PaxResult<u64> {
     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)))
 }