fix: validate salt length before reading password

Author: joelsa

README.md

@@ -26,6 +26,7 @@ echo -n "password" | argon2
 argon2 [-h] [salt] [-i|-d|-id] [-t iterations] [-m log2(memory in KiB) | -k memory in KiB] [-p parallelism] [-l hash length] [-e|-r] [-v (10|13)]
 ```
 
+- `[salt]` Optional explicit salt; must be at least 8 characters long
 - `-i` Use Argon2i (default)
 - `-d` Use Argon2d
 - `-id` Use Argon2id

src/main.rs

@@ -1,15 +1,19 @@
+use argon2::password_hash::SaltString;
 use clap::{ArgGroup, Parser};
 use std::io::{self, BufRead, IsTerminal, Write};
-use argon2::password_hash::SaltString;
 
 // Usage:  argon2 [-h] salt [-i|-d|-id] [-t iterations] [-m log2(memory in KiB) | -k memory in KiB] [-p parallelism] [-l hash length] [-e|-r] [-v (10|13)]
 #[derive(Parser, Debug)]
-#[command(name = "argon2", about = "(Rust implementation)", disable_help_flag = false)]
+#[command(
+    name = "argon2",
+    about = "(Rust implementation)",
+    disable_help_flag = false
+)]
 #[command(group(ArgGroup::new("variant").args(&["i", "d", "id"])))]
 #[command(group(ArgGroup::new("memory").args(&["m", "k"])))]
 #[command(group(ArgGroup::new("output_format").args(&["e", "r"])))]
 struct Args {
-    /// The salt to use (optional, a random salt is generated if omitted)
+    /// The salt to use (optional, a random salt is generated if omitted; minimum 8 characters)
     salt: Option<String>,
 
     /// Use Argon2i (this is the default)
@@ -73,30 +77,42 @@ fn get_input() -> io::Result<String> {
     }
 }
 
+fn validate_salt(salt: &str) -> Result<(), String> {
+    if salt.len() < 8 {
+        return Err("Invalid salt: minimum length is 8 characters".to_string());
+    }
+
+    Ok(())
+}
+
 fn main() -> Result<(), Box<dyn std::error::Error>> {
     // Handle the non-standard `-id` flag which conflicts with clap's short flag clustering
     let args_env = std::env::args();
-    let new_args: Vec<String> = args_env.map(|arg| {
-        if arg == "-id" {
-            "--id".to_string()
-        } else {
-            arg
-        }
-    }).collect();
+    let new_args: Vec<String> = args_env
+        .map(|arg| {
+            if arg == "-id" {
+                "--id".to_string()
+            } else {
+                arg
+            }
+        })
+        .collect();
 
     let args = Args::parse_from(new_args);
 
     let salt_string = match &args.salt {
-        Some(salt) => SaltString::encode_b64(salt.as_bytes())
-            .map_err(|e| format!("Invalid salt: {}", e))?,
+        Some(salt) => {
+            validate_salt(salt)?;
+            SaltString::encode_b64(salt.as_bytes()).map_err(|e| format!("Invalid salt: {}", e))?
+        }
         None => SaltString::generate(&mut rand_core::OsRng),
     };
 
     let password = get_input().unwrap_or_else(|e| {
         eprintln!("Error reading input: {}", e);
         std::process::exit(1);
     });
-    
+
     // Select algorithm variant
     let algorithm = if args.d {
         argon2::Algorithm::Argon2d
@@ -107,59 +123,48 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
     };
 
     // Calculate memory cost
-    let memory_kib = if let Some(k) = args.k {
-        k
-    } else {
-        1 << args.m
-    };
+    let memory_kib = if let Some(k) = args.k { k } else { 1 << args.m };
 
-    let params = argon2::Params::new(
-        memory_kib,
-        args.t,
-        args.p,
-        Some(args.l as usize),
-    ).map_err(|e| format!("Invalid parameters: {}", e))?;
+    let params = argon2::Params::new(memory_kib, args.t, args.p, Some(args.l as usize))
+        .map_err(|e| format!("Invalid parameters: {}", e))?;
 
     let version = match args.v {
         10 => argon2::Version::V0x10,
         13 => argon2::Version::V0x13,
         _ => return Err(format!("Invalid version: {} (expected 10 or 13)", args.v).into()),
     };
 
-    let argon2 = argon2::Argon2::new(
-        algorithm,
-        version,
-        params,
-    );
+    let argon2 = argon2::Argon2::new(algorithm, version, params);
 
     let start = std::time::Instant::now();
-    
+
     let password_bytes = password.as_bytes();
-    
+
     use argon2::PasswordHasher;
-    let password_hash = argon2.hash_password(password_bytes, salt_string.as_salt())
+    let password_hash = argon2
+        .hash_password(password_bytes, salt_string.as_salt())
         .map_err(|e| format!("Hashing failed: {}", e))?;
-    
+
     let duration = start.elapsed();
 
     // Generate output based on flags
     if args.e {
         println!("{}", password_hash);
     } else if args.r {
         if let Some(hash) = password_hash.hash {
-             io::stdout().write_all(hash.as_bytes())?;
+            io::stdout().write_all(hash.as_bytes())?;
         }
     } else {
         println!("Type:           {:?}", algorithm);
         println!("Iterations:     {}", args.t);
         println!("Memory:         {} KiB", memory_kib);
         println!("Parallelism:    {}", args.p);
-        
+
         if let Some(hash) = password_hash.hash {
             println!("Hash:           {}", hex::encode(hash.as_bytes()));
         }
         println!("Encoded:        {}", password_hash);
-        
+
         println!("{:.3} seconds", duration.as_secs_f64());
         println!("Verification ok");
     }

tests/verify.rs

@@ -1,7 +1,7 @@
-use std::process::Command;
-use std::io::Write;
 use rand::Rng;
 use std::collections::HashMap;
+use std::io::Write;
+use std::process::Command;
 
 const REF_BINARY: &str = "argon2";
 // We assume the binary is at the standard release location relative to the test runner.
@@ -24,19 +24,25 @@ fn run_argon2(binary: &str, salt: &str, password: &str, args: &[String]) -> Resu
     for arg in args {
         cmd.arg(arg);
     }
-    
+
     // Setup stdin
     cmd.stdin(std::process::Stdio::piped())
-       .stdout(std::process::Stdio::piped())
-       .stderr(std::process::Stdio::piped());
+        .stdout(std::process::Stdio::piped())
+        .stderr(std::process::Stdio::piped());
 
-    let mut child = cmd.spawn().map_err(|e| format!("Failed to spawn {}: {}", binary, e))?;
+    let mut child = cmd
+        .spawn()
+        .map_err(|e| format!("Failed to spawn {}: {}", binary, e))?;
 
     if let Some(mut stdin) = child.stdin.take() {
-        stdin.write_all(password.as_bytes()).map_err(|e| format!("Failed to write to stdin: {}", e))?;
+        stdin
+            .write_all(password.as_bytes())
+            .map_err(|e| format!("Failed to write to stdin: {}", e))?;
     }
 
-    let output = child.wait_with_output().map_err(|e| format!("Failed to wait: {}", e))?;
+    let output = child
+        .wait_with_output()
+        .map_err(|e| format!("Failed to wait: {}", e))?;
 
     if !output.status.success() {
         let stderr = String::from_utf8_lossy(&output.stderr);
@@ -91,26 +97,31 @@ fn verify(i: u32, memory_exp: u32, parallelism: u32, variant: &str) -> bool {
     let rust_data = parse_output(&rust_out);
 
     let keys_to_check = ["Iterations", "Memory", "Parallelism", "Hash", "Encoded"];
-    
+
     let mut success = true;
     for key in keys_to_check {
         let ref_val = ref_data.get(key);
         let rust_val = rust_data.get(key);
-        
-        if ref_val.is_none() { continue; } // e/-r flags not processed here yet
-        
+
+        if ref_val.is_none() {
+            continue;
+        } // e/-r flags not processed here yet
+
         if rust_val.is_none() {
             eprintln!("Mismatch: key '{}' missing in Rust output", key);
             success = false;
             continue;
         }
 
         if ref_val != rust_val {
-            eprintln!("Mismatch for '{}':\n  Ref:  {:?}\n  Rust: {:?}", key, ref_val, rust_val);
+            eprintln!(
+                "Mismatch for '{}':\n  Ref:  {:?}\n  Rust: {:?}",
+                key, ref_val, rust_val
+            );
             success = false;
         }
     }
-    
+
     if success {
         println!("OK");
     }
@@ -141,7 +152,30 @@ fn test_argon2_compatibility() {
         let variant = variants[var_idx];
 
         if !verify(iterations, memory_exp, parallelism, variant) {
-            panic!("Random test failed with: t={}, m={}, p={}, var={}", iterations, memory_exp, parallelism, variant);
+            panic!(
+                "Random test failed with: t={}, m={}, p={}, var={}",
+                iterations, memory_exp, parallelism, variant
+            );
         }
     }
 }
+
+#[test]
+fn test_short_salt_fails_before_hashing() {
+    let output = Command::new("./target/debug/argon2-cli")
+        .arg("1234567")
+        .output()
+        .expect("Failed to run debug binary");
+
+    assert!(!output.status.success(), "Short salt should fail");
+
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        stderr.contains("Invalid salt: minimum length is 8 characters"),
+        "unexpected stderr: {stderr}"
+    );
+    assert!(
+        !stderr.contains("Hashing failed"),
+        "salt validation should happen before hashing: {stderr}"
+    );
+}