From d7704356cbcd4f300b792ba3bc2c45803aa0f1c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 12:35:44 +0000 Subject: [PATCH 1/2] Security: enforce SOCKS5 auth, validate proxyList.txt entries Two issues surfaced in security review: 1. SOCKS5 auth downgrade (SocksPeerConnector.h): when credentials were configured, the client advertised both NO AUTH and USER/PASS methods in the greeting and also accepted a server choice of NO AUTH after the fact. A rogue SOCKS5 peer could therefore accept traffic without ever verifying the credentials the operator required. The greeting now advertises only USER/PASS when credentials are present, and the no-auth branch additionally refuses to proceed if hasAuth is true. 2. squid.conf injection from proxyList.txt (generate.php): socks-user and socks-pass values were written verbatim with no escaping. A tainted proxy list (e.g. one fetched from a remote URL via public_proxy_cron.sh) could inject arbitrary squid.conf directives via whitespace/newlines/quotes in credentials, or a hostile host/ port field. Each line is now validated: host must be hostname or IP-literal charset, port must be 1-65535, credentials must not contain whitespace/control chars/quotes/backslash/#, and credential length is capped at 255 bytes per RFC 1929. --- setup/generate.php | 34 ++++++++++++++++++++++++++++ squid_patch/src/SocksPeerConnector.h | 29 ++++++++++++------------ 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/setup/generate.php b/setup/generate.php index 1af1a9b..fbcec7c 100644 --- a/setup/generate.php +++ b/setup/generate.php @@ -19,8 +19,17 @@ // the connection is direct to the target (not an HTTP proxy). $squid_socks = 'cache_peer %s parent %d 0 no-digest no-netdb-exchange connect-fail-limit=2 connect-timeout=8 round-robin no-query allow-miss proxy-only originserver name=%s %s'; +// Reject any byte that could break squid.conf tokenization or inject +// a new directive (whitespace, control chars, quotes, backslash, '#'). +// Applied to host/user/pass to prevent config injection from a tainted +// proxyList.txt (e.g. one fetched from a remote URL). +$reject_unsafe = '/[\s"#\\\\\x00-\x1F\x7F]/'; + while ($line = fgets($proxies)){ $line = trim($line); + if ($line === '' || $line[0] === '#') { + continue; + } $proxyInfo = array_combine($keys, array_pad((explode(":", $line, 5)), 5, '')); $squid_conf = []; $cred = ''; @@ -28,6 +37,31 @@ continue; } + // Validate host: hostname or IPv4/IPv6 literal. No shell/conf metachars. + if (preg_match($reject_unsafe, $proxyInfo['host']) || + !preg_match('/^[A-Za-z0-9.:\[\]_-]+$/', $proxyInfo['host'])) { + fwrite(STDERR, "Skipping proxy with invalid host: " . $proxyInfo['host'] . PHP_EOL); + continue; + } + // Validate port: 1-65535. + if (!ctype_digit((string)$proxyInfo['port']) || + (int)$proxyInfo['port'] < 1 || (int)$proxyInfo['port'] > 65535) { + fwrite(STDERR, "Skipping proxy with invalid port: " . $proxyInfo['port'] . PHP_EOL); + continue; + } + // Validate credentials: no whitespace/control chars/quotes/backslash/#. + // (SOCKS5 RFC1929 allows up to 255 bytes of arbitrary octets, but we + // conservatively reject bytes that would break squid.conf.) + if (($proxyInfo['user'] !== '' && preg_match($reject_unsafe, $proxyInfo['user'])) || + ($proxyInfo['pass'] !== '' && preg_match($reject_unsafe, $proxyInfo['pass']))) { + fwrite(STDERR, "Skipping proxy with unsafe characters in credentials: " . $proxyInfo['host'] . PHP_EOL); + continue; + } + if (strlen($proxyInfo['user']) > 255 || strlen($proxyInfo['pass']) > 255) { + fwrite(STDERR, "Skipping proxy with credentials exceeding 255 bytes: " . $proxyInfo['host'] . PHP_EOL); + continue; + } + if(!$proxyInfo['scheme']){ //open proxy server IP:Port Pattern. //No create gost diff --git a/squid_patch/src/SocksPeerConnector.h b/squid_patch/src/SocksPeerConnector.h index 581e097..29aeb7d 100644 --- a/squid_patch/src/SocksPeerConnector.h +++ b/squid_patch/src/SocksPeerConnector.h @@ -127,20 +127,15 @@ static inline bool socks5Connect(int fd, const bool hasAuth = (!user.empty() && !pass.empty()); /* --- greeting ---------------------------------------------------- */ - uint8_t greeting[4]; - size_t gLen; - if (hasAuth) { - greeting[0] = 0x05; /* VER */ - greeting[1] = 0x02; /* NMETHODS */ - greeting[2] = 0x00; /* NO AUTHENTICATION */ - greeting[3] = 0x02; /* USERNAME / PASSWORD */ - gLen = 4; - } else { - greeting[0] = 0x05; - greeting[1] = 0x01; - greeting[2] = 0x00; - gLen = 3; - } + /* When credentials are configured, advertise ONLY USER/PASS to + * prevent a rogue SOCKS5 server from silently downgrading to + * "no authentication" and accepting traffic without verifying + * the credentials the operator explicitly provided. */ + uint8_t greeting[3]; + greeting[0] = 0x05; /* VER */ + greeting[1] = 0x01; /* NMETHODS */ + greeting[2] = hasAuth ? 0x02 : 0x00; /* USER/PASS or NO AUTH */ + const size_t gLen = 3; if (!syncSend(fd, greeting, gLen)) return false; @@ -182,7 +177,11 @@ static inline bool socks5Connect(int fd, return false; /* auth failed or wrong sub-negotiation version */ } else if (gResp[1] == 0x00) { - /* no auth required */ + /* Server chose "no authentication". Only accept this when the + * operator did NOT configure credentials; otherwise the server + * is downgrading away from the authentication we required. */ + if (hasAuth) + return false; } else { return false; /* unsupported or unacceptable method (includes 0xFF) */ } From 56afd4cf2dc5167032f94b4645ee405177d58f30 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 16:45:24 +0000 Subject: [PATCH 2/2] Address CodeRabbit: sanitize STDERR logs, drop IPv6 host chars CodeRabbit review on PR #31: 1. Log injection (setup/generate.php:43,49): the pre-validation values for host/port were written straight to STDERR, so a proxyList.txt line containing a newline could forge fake log entries in downstream log aggregation. Wrap both with rawurlencode() so any control chars and newlines are escaped before being printed. 2. Misleading IPv6 support (setup/generate.php:42): the host regex allowed ':' / '[' / ']', suggesting "[::1]:8080:..." would work, but the naive explode(":", $line, 5) parser splits IPv6 literals incorrectly. Tighten the regex to "[A-Za-z0-9._-]" so IPv6-looking entries are now rejected with a clear error instead of silently malformed, matching the parser's actual capability. --- setup/generate.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/setup/generate.php b/setup/generate.php index fbcec7c..e901bbc 100644 --- a/setup/generate.php +++ b/setup/generate.php @@ -37,16 +37,19 @@ continue; } - // Validate host: hostname or IPv4/IPv6 literal. No shell/conf metachars. + // Validate host: hostname or IPv4 literal. No shell/conf metachars. + // IPv6 literals are not supported: the naive explode(":") parser above + // cannot split "[::1]:8080:..." correctly, so ':' / '[' / ']' are rejected + // to avoid giving the impression that IPv6 is accepted. if (preg_match($reject_unsafe, $proxyInfo['host']) || - !preg_match('/^[A-Za-z0-9.:\[\]_-]+$/', $proxyInfo['host'])) { - fwrite(STDERR, "Skipping proxy with invalid host: " . $proxyInfo['host'] . PHP_EOL); + !preg_match('/^[A-Za-z0-9._-]+$/', $proxyInfo['host'])) { + fwrite(STDERR, "Skipping proxy with invalid host: " . rawurlencode($proxyInfo['host']) . PHP_EOL); continue; } // Validate port: 1-65535. if (!ctype_digit((string)$proxyInfo['port']) || (int)$proxyInfo['port'] < 1 || (int)$proxyInfo['port'] > 65535) { - fwrite(STDERR, "Skipping proxy with invalid port: " . $proxyInfo['port'] . PHP_EOL); + fwrite(STDERR, "Skipping proxy with invalid port: " . rawurlencode((string)$proxyInfo['port']) . PHP_EOL); continue; } // Validate credentials: no whitespace/control chars/quotes/backslash/#.