Skip to content

Commit d770435

Browse files
committed
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.
1 parent 02e7f47 commit d770435

2 files changed

Lines changed: 48 additions & 15 deletions

File tree

setup/generate.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,49 @@
1919
// the connection is direct to the target (not an HTTP proxy).
2020
$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';
2121

22+
// Reject any byte that could break squid.conf tokenization or inject
23+
// a new directive (whitespace, control chars, quotes, backslash, '#').
24+
// Applied to host/user/pass to prevent config injection from a tainted
25+
// proxyList.txt (e.g. one fetched from a remote URL).
26+
$reject_unsafe = '/[\s"#\\\\\x00-\x1F\x7F]/';
27+
2228
while ($line = fgets($proxies)){
2329
$line = trim($line);
30+
if ($line === '' || $line[0] === '#') {
31+
continue;
32+
}
2433
$proxyInfo = array_combine($keys, array_pad((explode(":", $line, 5)), 5, ''));
2534
$squid_conf = [];
2635
$cred = '';
2736
if(!$proxyInfo['host'] && !$proxyInfo['port']){
2837
continue;
2938
}
3039

40+
// Validate host: hostname or IPv4/IPv6 literal. No shell/conf metachars.
41+
if (preg_match($reject_unsafe, $proxyInfo['host']) ||
42+
!preg_match('/^[A-Za-z0-9.:\[\]_-]+$/', $proxyInfo['host'])) {
43+
fwrite(STDERR, "Skipping proxy with invalid host: " . $proxyInfo['host'] . PHP_EOL);
44+
continue;
45+
}
46+
// Validate port: 1-65535.
47+
if (!ctype_digit((string)$proxyInfo['port']) ||
48+
(int)$proxyInfo['port'] < 1 || (int)$proxyInfo['port'] > 65535) {
49+
fwrite(STDERR, "Skipping proxy with invalid port: " . $proxyInfo['port'] . PHP_EOL);
50+
continue;
51+
}
52+
// Validate credentials: no whitespace/control chars/quotes/backslash/#.
53+
// (SOCKS5 RFC1929 allows up to 255 bytes of arbitrary octets, but we
54+
// conservatively reject bytes that would break squid.conf.)
55+
if (($proxyInfo['user'] !== '' && preg_match($reject_unsafe, $proxyInfo['user'])) ||
56+
($proxyInfo['pass'] !== '' && preg_match($reject_unsafe, $proxyInfo['pass']))) {
57+
fwrite(STDERR, "Skipping proxy with unsafe characters in credentials: " . $proxyInfo['host'] . PHP_EOL);
58+
continue;
59+
}
60+
if (strlen($proxyInfo['user']) > 255 || strlen($proxyInfo['pass']) > 255) {
61+
fwrite(STDERR, "Skipping proxy with credentials exceeding 255 bytes: " . $proxyInfo['host'] . PHP_EOL);
62+
continue;
63+
}
64+
3165
if(!$proxyInfo['scheme']){
3266
//open proxy server IP:Port Pattern.
3367
//No create gost

squid_patch/src/SocksPeerConnector.h

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -127,20 +127,15 @@ static inline bool socks5Connect(int fd,
127127
const bool hasAuth = (!user.empty() && !pass.empty());
128128

129129
/* --- greeting ---------------------------------------------------- */
130-
uint8_t greeting[4];
131-
size_t gLen;
132-
if (hasAuth) {
133-
greeting[0] = 0x05; /* VER */
134-
greeting[1] = 0x02; /* NMETHODS */
135-
greeting[2] = 0x00; /* NO AUTHENTICATION */
136-
greeting[3] = 0x02; /* USERNAME / PASSWORD */
137-
gLen = 4;
138-
} else {
139-
greeting[0] = 0x05;
140-
greeting[1] = 0x01;
141-
greeting[2] = 0x00;
142-
gLen = 3;
143-
}
130+
/* When credentials are configured, advertise ONLY USER/PASS to
131+
* prevent a rogue SOCKS5 server from silently downgrading to
132+
* "no authentication" and accepting traffic without verifying
133+
* the credentials the operator explicitly provided. */
134+
uint8_t greeting[3];
135+
greeting[0] = 0x05; /* VER */
136+
greeting[1] = 0x01; /* NMETHODS */
137+
greeting[2] = hasAuth ? 0x02 : 0x00; /* USER/PASS or NO AUTH */
138+
const size_t gLen = 3;
144139

145140
if (!syncSend(fd, greeting, gLen))
146141
return false;
@@ -182,7 +177,11 @@ static inline bool socks5Connect(int fd,
182177
return false; /* auth failed or wrong sub-negotiation version */
183178

184179
} else if (gResp[1] == 0x00) {
185-
/* no auth required */
180+
/* Server chose "no authentication". Only accept this when the
181+
* operator did NOT configure credentials; otherwise the server
182+
* is downgrading away from the authentication we required. */
183+
if (hasAuth)
184+
return false;
186185
} else {
187186
return false; /* unsupported or unacceptable method (includes 0xFF) */
188187
}

0 commit comments

Comments
 (0)