Fix from_str_radix accepting leading signs in non-decimal radixes

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,7 +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)
+    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)))
 }
 

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

plib/src/lib.rs

@@ -15,6 +15,7 @@ pub mod modestr;
 pub mod platform;
 pub mod priority;
 pub mod regex;
+pub mod safe_parse;
 pub mod sccsfile;
 pub mod testing;
 pub mod user;

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;

plib/src/safe_parse.rs

@@ -0,0 +1,167 @@
+//
+// Copyright (c) 2024 Hemi Labs, Inc.
+//
+// This file is part of the posixutils-rs project covered under
+// the MIT License.  For the full license text, please see the LICENSE
+// file in the root directory of this project.
+// SPDX-License-Identifier: MIT
+//
+
+//! Safe parsing utilities for integers with radix validation.
+//!
+//! This module provides helper functions to safely parse integers from strings
+//! with specific radixes, addressing the issue that Rust's `from_str_radix`
+//! accepts leading `+` or `-` signs even for non-decimal radixes.
+//!
+//! For example, `i64::from_str_radix("+55", 16)` succeeds, which may be
+//! undesirable when parsing user input that should strictly conform to
+//! hexadecimal or octal format without signs.
+
+use std::num::ParseIntError;
+
+/// Safely parse an unsigned integer from a string with the given radix.
+///
+/// This function validates that the input string contains only valid digits
+/// for the specified radix, rejecting any leading signs (`+` or `-`).
+///
+/// # Examples
+///
+/// ```
+/// use plib::safe_parse::parse_u32_radix;
+///
+/// assert!(parse_u32_radix("ff", 16).is_ok());
+/// assert!(parse_u32_radix("+ff", 16).is_err()); // Reject leading +
+/// assert!(parse_u32_radix("377", 8).is_ok());
+/// assert!(parse_u32_radix("+377", 8).is_err()); // Reject leading +
+/// ```
+pub fn parse_u32_radix(s: &str, radix: u32) -> Result<u32, ParseIntError> {
+    if s.is_empty() {
+        return Err("empty string".parse::<u32>().unwrap_err());
+    }
+
+    // Check for leading signs
+    if s.starts_with('+') || s.starts_with('-') {
+        return Err("leading sign not allowed".parse::<u32>().unwrap_err());
+    }
+
+    u32::from_str_radix(s, radix)
+}
+
+/// Safely parse a signed integer from a string with the given radix.
+///
+/// This function is similar to `parse_u32_radix`, but allows a leading `-`
+/// sign for negative numbers. It still rejects a leading `+` sign for
+/// non-decimal radixes to ensure strict format validation.
+///
+/// # Examples
+///
+/// ```
+/// use plib::safe_parse::parse_i64_radix;
+///
+/// assert!(parse_i64_radix("ff", 16).is_ok());
+/// assert!(parse_i64_radix("-ff", 16).is_ok()); // Allow leading -
+/// assert!(parse_i64_radix("+ff", 16).is_err()); // Reject leading +
+/// ```
+pub fn parse_i64_radix(s: &str, radix: u32) -> Result<i64, ParseIntError> {
+    if s.is_empty() {
+        return Err("empty string".parse::<i64>().unwrap_err());
+    }
+
+    // For non-decimal radixes, reject leading + sign
+    if radix != 10 && s.starts_with('+') {
+        return Err("leading + sign not allowed".parse::<i64>().unwrap_err());
+    }
+
+    i64::from_str_radix(s, radix)
+}
+
+/// Safely parse a u64 from a string with the given radix.
+pub fn parse_u64_radix(s: &str, radix: u32) -> Result<u64, ParseIntError> {
+    if s.is_empty() {
+        return Err("empty string".parse::<u64>().unwrap_err());
+    }
+
+    if s.starts_with('+') || s.starts_with('-') {
+        return Err("leading sign not allowed".parse::<u64>().unwrap_err());
+    }
+
+    u64::from_str_radix(s, radix)
+}
+
+/// Safely parse a usize from a string with the given radix.
+pub fn parse_usize_radix(s: &str, radix: u32) -> Result<usize, ParseIntError> {
+    if s.is_empty() {
+        return Err("empty string".parse::<usize>().unwrap_err());
+    }
+
+    if s.starts_with('+') || s.starts_with('-') {
+        return Err("leading sign not allowed".parse::<usize>().unwrap_err());
+    }
+
+    usize::from_str_radix(s, radix)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_parse_u32_radix_hex() {
+        // Valid hex
+        assert_eq!(parse_u32_radix("ff", 16).unwrap(), 255);
+        assert_eq!(parse_u32_radix("FF", 16).unwrap(), 255);
+        assert_eq!(parse_u32_radix("0", 16).unwrap(), 0);
+
+        // Invalid - leading signs
+        assert!(parse_u32_radix("+ff", 16).is_err());
+        assert!(parse_u32_radix("-ff", 16).is_err());
+
+        // Invalid - non-hex digits
+        assert!(parse_u32_radix("gg", 16).is_err());
+
+        // Invalid - empty
+        assert!(parse_u32_radix("", 16).is_err());
+    }
+
+    #[test]
+    fn test_parse_u32_radix_octal() {
+        // Valid octal
+        assert_eq!(parse_u32_radix("377", 8).unwrap(), 255);
+        assert_eq!(parse_u32_radix("0", 8).unwrap(), 0);
+
+        // Invalid - leading signs
+        assert!(parse_u32_radix("+377", 8).is_err());
+        assert!(parse_u32_radix("-377", 8).is_err());
+
+        // Invalid - non-octal digits
+        assert!(parse_u32_radix("389", 8).is_err());
+    }
+
+    #[test]
+    fn test_parse_i64_radix_hex() {
+        // Valid hex
+        assert_eq!(parse_i64_radix("ff", 16).unwrap(), 255);
+        assert_eq!(parse_i64_radix("-ff", 16).unwrap(), -255);
+
+        // Invalid - leading + sign
+        assert!(parse_i64_radix("+ff", 16).is_err());
+    }
+
+    #[test]
+    fn test_parse_i64_radix_octal() {
+        // Valid octal
+        assert_eq!(parse_i64_radix("377", 8).unwrap(), 255);
+        assert_eq!(parse_i64_radix("-377", 8).unwrap(), -255);
+
+        // Invalid - leading + sign
+        assert!(parse_i64_radix("+377", 8).is_err());
+    }
+
+    #[test]
+    fn test_parse_i64_radix_decimal() {
+        // Decimal allows + sign
+        assert_eq!(parse_i64_radix("123", 10).unwrap(), 123);
+        assert_eq!(parse_i64_radix("+123", 10).unwrap(), 123);
+        assert_eq!(parse_i64_radix("-123", 10).unwrap(), -123);
+    }
+}

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