Make sure that socks4 auth doesn't have any 0 bytes.

Try to do it in constant time, to avoid even the smell of
side-channel attacks.

Cargo.lock

@@ -3934,6 +3934,7 @@ dependencies = [
  "arbitrary",
  "caret",
  "hex-literal",
+ "subtle",
  "thiserror",
  "tor-bytes",
  "tor-error",

crates/tor-socksproto/Cargo.toml

@@ -19,6 +19,7 @@ proxy-handshake = []
 [dependencies]
 arbitrary = { version = "1.0.1", optional = true, features = ["derive"] }
 caret = { path = "../caret", version = "0.2.0" }
+subtle = "2"
 thiserror = "1"
 tor-bytes = { path = "../tor-bytes", version = "0.5.1" }
 tor-error = { path = "../tor-error", version = "0.3.2" }

crates/tor-socksproto/src/msg.rs

@@ -233,6 +233,43 @@ impl AsRef<str> for SocksHostname {
     }
 }
 
+impl SocksAuth {
+    /// Check whether this authentication is well-formed and compatible with the
+    /// provided SOCKS version.
+    ///
+    /// Return an error if not.
+    fn validate(&self, version: SocksVersion) -> Result<()> {
+        match self {
+            SocksAuth::NoAuth => {}
+            SocksAuth::Socks4(data) => {
+                if version != SocksVersion::V4 || contains_zeros(data) {
+                    return Err(Error::Syntax);
+                }
+            }
+            SocksAuth::Username(user, pass) => {
+                if version != SocksVersion::V5
+                    || user.len() > u8::MAX as usize
+                    || pass.len() > u8::MAX as usize
+                {
+                    return Err(Error::Syntax);
+                }
+            }
+        }
+        Ok(())
+    }
+}
+
+/// Return true if b contains at least one zero.
+///
+/// Try to run in constant time.
+fn contains_zeros(b: &[u8]) -> bool {
+    use subtle::{Choice, ConstantTimeEq};
+    let c: Choice = b
+        .iter()
+        .fold(Choice::from(0), |seen_any, byte| seen_any | byte.ct_eq(&0));
+    c.unwrap_u8() != 0
+}
+
 impl SocksRequest {
     /// Create a SocksRequest with a given set of fields.
     ///
@@ -252,22 +289,7 @@ impl SocksRequest {
         if port == 0 && cmd.requires_port() {
             return Err(Error::Syntax);
         }
-        match &auth {
+        auth.validate(version)?;
-            SocksAuth::NoAuth => {}
-            SocksAuth::Socks4(_) => {
-                if version != SocksVersion::V4 {
-                    return Err(Error::Syntax);
-                }
-            }
-            SocksAuth::Username(user, pass) => {
-                if version != SocksVersion::V5
-                    || user.len() > u8::MAX as usize
-                    || pass.len() > u8::MAX as usize
-                {
-                    return Err(Error::Syntax);
-                }
-            }
-        }
 
         Ok(SocksRequest {
             version,
@@ -371,4 +393,10 @@ mod test {
         );
         assert!(matches!(e, Err(Error::Syntax)));
     }
+
+    #[test]
+    fn test_contains_zeros() {
+        assert!(contains_zeros(b"Hello\0world"));
+        assert!(!contains_zeros(b"Hello world"));
+    }
 }