From 50f07bf61967eeb3ba2799280a3a48eb7daae7ec Mon Sep 17 00:00:00 2001
From: Max Lv <max.c.lv@gmail.com>
Date: Sun, 14 Jun 2026 10:34:44 +0800
Subject: [PATCH] fix(tunnel): validate socks5 auth field lengths

---
 src/config.rs | 36 +++++++++++++++++++++++++++++++
 src/tunnel.rs | 59 ++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 75 insertions(+), 20 deletions(-)

diff --git a/src/config.rs b/src/config.rs
index 2245f92..cc7cfca 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -122,6 +122,8 @@ impl std::str::FromStr for UpstreamProxy {
                 let (user, pass) = userinfo.split_once(':').ok_or_else(|| {
                     format!("invalid socks5 userinfo '{}': expected user:pass", userinfo)
                 })?;
+                validate_socks5_credential("username", user)?;
+                validate_socks5_credential("password", pass)?;
                 (
                     ProxyAuth::UsernamePassword {
                         username: user.to_string(),
@@ -153,6 +155,19 @@ impl std::str::FromStr for UpstreamProxy {
     }
 }
 
+fn validate_socks5_credential(label: &str, value: &str) -> Result<(), String> {
+    let len = value.len();
+    if len == 0 {
+        return Err(format!("invalid socks5 {label}: cannot be empty"));
+    }
+    if len > u8::MAX as usize {
+        return Err(format!(
+            "invalid socks5 {label}: {len} bytes exceeds 255-byte limit"
+        ));
+    }
+    Ok(())
+}
+
 /// Comma-separated list of TCP ports for firewall redirection.
 ///
 /// Used with the `--ports` flag to restrict which ports are redirected.
@@ -465,6 +480,27 @@ mod tests {
         assert_eq!(proxy.addr.to_string(), "127.0.0.1:1080");
     }
 
+    #[test]
+    fn test_upstream_proxy_rejects_empty_socks5_auth_fields() {
+        let empty_user: Result<UpstreamProxy, _> = "socks5://:pass@127.0.0.1:1080".parse();
+        assert!(empty_user.is_err());
+
+        let empty_pass: Result<UpstreamProxy, _> = "socks5://user:@127.0.0.1:1080".parse();
+        assert!(empty_pass.is_err());
+    }
+
+    #[test]
+    fn test_upstream_proxy_rejects_oversized_socks5_auth_fields() {
+        let oversized = "u".repeat(256);
+        let oversized_user: Result<UpstreamProxy, _> =
+            format!("socks5://{oversized}:pass@127.0.0.1:1080").parse();
+        assert!(oversized_user.is_err());
+
+        let oversized_pass: Result<UpstreamProxy, _> =
+            format!("socks5://user:{oversized}@127.0.0.1:1080").parse();
+        assert!(oversized_pass.is_err());
+    }
+
     #[test]
     fn test_upstream_proxy_parse_invalid() {
         let result: Result<UpstreamProxy, _> = "not-valid".parse();
diff --git a/src/tunnel.rs b/src/tunnel.rs
index 8889cec..459d3c7 100644
--- a/src/tunnel.rs
+++ b/src/tunnel.rs
@@ -317,26 +317,7 @@ async fn socks5_username_auth(
     username: &str,
     password: &str,
 ) -> Result<()> {
-    if username.len() > MAX_SOCKS5_FIELD_LEN {
-        bail!(
-            "SOCKS5: username is {} bytes, maximum is {}",
-            username.len(),
-            MAX_SOCKS5_FIELD_LEN
-        );
-    }
-    if password.len() > MAX_SOCKS5_FIELD_LEN {
-        bail!(
-            "SOCKS5: password is {} bytes, maximum is {}",
-            password.len(),
-            MAX_SOCKS5_FIELD_LEN
-        );
-    }
-
-    let mut auth_req = vec![0x01]; // sub-negotiation version
-    auth_req.push(username.len() as u8);
-    auth_req.extend_from_slice(username.as_bytes());
-    auth_req.push(password.len() as u8);
-    auth_req.extend_from_slice(password.as_bytes());
+    let auth_req = build_socks5_auth_request(username, password)?;
 
     timeout(CONNECT_TIMEOUT, stream.write_all(&auth_req))
         .await
@@ -384,6 +365,29 @@ fn is_valid_proxy_hostname(hostname: &str) -> bool {
     })
 }
 
+fn build_socks5_auth_request(username: &str, password: &str) -> Result<Vec<u8>> {
+    validate_socks5_auth_field("username", username)?;
+    validate_socks5_auth_field("password", password)?;
+
+    let mut auth_req = vec![0x01]; // sub-negotiation version
+    auth_req.push(username.len() as u8);
+    auth_req.extend_from_slice(username.as_bytes());
+    auth_req.push(password.len() as u8);
+    auth_req.extend_from_slice(password.as_bytes());
+    Ok(auth_req)
+}
+
+fn validate_socks5_auth_field(label: &str, value: &str) -> Result<()> {
+    let len = value.len();
+    if len == 0 {
+        bail!("SOCKS5: {label} cannot be empty");
+    }
+    if len > MAX_SOCKS5_FIELD_LEN {
+        bail!("SOCKS5: {label} is {len} bytes, exceeds {MAX_SOCKS5_FIELD_LEN}-byte limit");
+    }
+    Ok(())
+}
+
 /// Map a SOCKS5 reply status byte to a human-readable error message.
 fn socks5_error_message(code: u8) -> &'static str {
     match code {
@@ -532,6 +536,21 @@ mod tests {
         )));
     }
 
+    #[test]
+    fn test_socks5_auth_request_encodes_field_lengths() {
+        let req = build_socks5_auth_request("user", "pass").unwrap();
+
+        assert_eq!(req, b"\x01\x04user\x04pass");
+    }
+
+    #[test]
+    fn test_socks5_auth_request_rejects_invalid_field_lengths() {
+        assert!(build_socks5_auth_request("", "pass").is_err());
+        assert!(build_socks5_auth_request("user", "").is_err());
+        assert!(build_socks5_auth_request(&"u".repeat(256), "pass").is_err());
+        assert!(build_socks5_auth_request("user", &"p".repeat(256)).is_err());
+    }
+
     /// Run handshake_http_connect against a fake proxy that replies with
     /// `response` in a single write, returning the leftover bytes.
     async fn run_connect_handshake(response: &'static [u8]) -> Result<Vec<u8>> {