diff --git a/Cargo.lock b/Cargo.lock index a428994d4..3f416c029 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3934,6 +3934,7 @@ dependencies = [ "arbitrary", "caret", "hex-literal", + "subtle", "thiserror", "tor-bytes", "tor-error", diff --git a/crates/tor-socksproto/Cargo.toml b/crates/tor-socksproto/Cargo.toml index 513c41dba..4ac9e156e 100644 --- a/crates/tor-socksproto/Cargo.toml +++ b/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" } diff --git a/crates/tor-socksproto/src/msg.rs b/crates/tor-socksproto/src/msg.rs index 20b9f954b..88b0a9a93 100644 --- a/crates/tor-socksproto/src/msg.rs +++ b/crates/tor-socksproto/src/msg.rs @@ -233,6 +233,43 @@ impl AsRef 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 { - 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); - } - } - } + auth.validate(version)?; 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")); + } }