Skip to content

Commit b25b496

Browse files
mcollinaaduh95
mcollina
authored and
aduh95
committed
http: redact proxy credentials in invalid proxy URL errors
The earlier fix to redact proxy credentials only covered the tunnel error and the ProxyConfig.href property. Four other error paths still interpolate the raw proxy URL, credentials included, into ERR_PROXY_INVALID_CONFIG messages: * lib/internal/http.js - ProxyConfig constructor when new URL() fails. * lib/internal/http.js - parseProxyUrl when the URL contains CR or LF. * lib/http.js - setGlobalProxyFromEnv() with an unparseable http_proxy. * lib/http.js - setGlobalProxyFromEnv() with an unparseable https_proxy Add a redactProxyUrl() helper that strips userinfo when parsing succeeds and falls back to a textual strip for the unparseable cases that reach the vulnerable paths, then route all four sites through it. Refs: https://hackerone.com/reports/3966924 Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: nodejs-private/node-private#1001 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> CVE-ID: CVE-2026-123871
1 parent 46fd313 commit b25b496

3 files changed

Lines changed: 178 additions & 4 deletions

File tree

‎lib/http.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ const {
5858
const {
5959
parseProxyUrl,
6060
getGlobalAgent,
61+
redactProxyUrl,
6162
} = require('internal/http');
6263
const { URL } = require('internal/url');
6364
let maxHeaderSize;
@@ -153,10 +154,10 @@ function setGlobalProxyFromEnv(env = process.env) {
153154
}
154155

155156
if (httpProxy && !URL.canParse(httpProxy)) {
156-
throw new ERR_PROXY_INVALID_CONFIG(httpProxy);
157+
throw new ERR_PROXY_INVALID_CONFIG(redactProxyUrl(httpProxy));
157158
}
158159
if (httpsProxy && !URL.canParse(httpsProxy)) {
159-
throw new ERR_PROXY_INVALID_CONFIG(httpsProxy);
160+
throw new ERR_PROXY_INVALID_CONFIG(redactProxyUrl(httpsProxy));
160161
}
161162

162163
let originalDispatcher, originalHttpsAgent, originalHttpAgent;

‎lib/internal/http.js‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,13 +112,40 @@ function ipToInt(ip) {
112112
* @property {Array<string>} bypassList - List of hosts to bypass the proxy.
113113
* @property {object} proxyConnectionOptions - Options for connecting to the proxy server.
114114
*/
115+
116+
/**
117+
* Redacts the userinfo (username/password) from a proxy URL so that credentials
118+
* never surface in error messages, which can end up in logs and crash reports.
119+
* This must not depend on parsing succeeding: it is used precisely when the URL
120+
* failed to parse.
121+
* @param {string} proxyUrl - The proxy URL to redact.
122+
* @returns {string} The proxy URL with any userinfo removed.
123+
*/
124+
function redactProxyUrl(proxyUrl) {
125+
const raw = `${proxyUrl}`;
126+
try {
127+
const parsedURL = new URL(raw);
128+
if (parsedURL.username || parsedURL.password) {
129+
parsedURL.username = '';
130+
parsedURL.password = '';
131+
return parsedURL.href;
132+
}
133+
return raw;
134+
} catch {
135+
// Unparseable: strip the userinfo textually. Greedy up to the last '@' in the
136+
// authority so a password containing '@' is redacted whole rather than
137+
// leaving a trailing fragment.
138+
return raw.replace(/\/\/[^/]*@/, '//[redacted]@');
139+
}
140+
}
141+
115142
class ProxyConfig {
116143
constructor(proxyUrl, keepAlive, noProxyList) {
117144
let parsedURL;
118145
try {
119146
parsedURL = new URL(proxyUrl);
120147
} catch {
121-
throw new ERR_PROXY_INVALID_CONFIG(`Invalid proxy URL: ${proxyUrl}`);
148+
throw new ERR_PROXY_INVALID_CONFIG(`Invalid proxy URL: ${redactProxyUrl(proxyUrl)}`);
122149
}
123150
const { hostname, port, protocol, username, password } = parsedURL;
124151

@@ -210,7 +237,7 @@ function parseProxyUrl(env, protocol) {
210237
}
211238

212239
if (proxyUrl.includes('\r') || proxyUrl.includes('\n')) {
213-
throw new ERR_PROXY_INVALID_CONFIG(`Invalid proxy URL: ${proxyUrl}`);
240+
throw new ERR_PROXY_INVALID_CONFIG(`Invalid proxy URL: ${redactProxyUrl(proxyUrl)}`);
214241
}
215242

216243
return proxyUrl;
@@ -290,4 +317,5 @@ module.exports = {
290317
filterEnvForProxies,
291318
getGlobalAgent,
292319
parseProxyUrl,
320+
redactProxyUrl,
293321
};

‎test/client-proxy/test-http-proxy-redact-credentials.mjs‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
// Regression test for the incomplete CVE-2026-48615 fix: proxy credentials must
2+
// never appear in ERR_PROXY_INVALID_CONFIG messages. Four error paths interpolate
3+
// the raw proxy URL (userinfo included):
4+
// 1. lib/internal/http.js - new URL() fails in the ProxyConfig constructor.
5+
// 2. lib/internal/http.js - proxy URL contains CR or LF.
6+
// 3. lib/http.js - http.setGlobalProxyFromEnv() with an unparseable http_proxy.
7+
// 4. lib/http.js - http.setGlobalProxyFromEnv() with an unparseable https_proxy.
8+
//
9+
// The well-formed credential control confirms redaction on the normal path is
10+
// preserved.
11+
12+
import '../common/index.mjs';
13+
import assert from 'node:assert';
14+
import { spawnSync } from 'node:child_process';
15+
16+
// A proxy credential that both contains '@' and yields substrings in every
17+
// meaningful piece that must not survive in any error message.
18+
const SECRET = 'S3cr3t-Pr0xy-P@ssw0rd';
19+
20+
// Asserts that a message does not contain any substring of length >= 4 derived
21+
// from the secret. Length-4 substrings are enough to catch partial leaks while
22+
// avoiding false positives from unrelated words.
23+
function assertNoLeak(message, label) {
24+
assert.doesNotMatch(message, /S3cr3t/, `${label}: leaked username`);
25+
assert.doesNotMatch(message, /Pr0xy/, `${label}: leaked password fragment`);
26+
assert.doesNotMatch(message, /ssw0rd/, `${label}: leaked password fragment`);
27+
assert.doesNotMatch(message, /P@ssw0rd/, `${label}: leaked password`);
28+
// A malicious proxy URL must not survive verbatim.
29+
assert.notStrictEqual(message, SECRET, `${label}: leaked the whole secret`);
30+
}
31+
32+
function runNode(code, env = {}) {
33+
const res = spawnSync(process.execPath, ['-e', code], {
34+
env: { ...process.env, ...env },
35+
encoding: 'utf8',
36+
});
37+
return res;
38+
}
39+
40+
function setGlobalProxyFromEnv(proxyEnv) {
41+
return `try {
42+
require('http').setGlobalProxyFromEnv(${JSON.stringify(proxyEnv)});
43+
} catch (e) {
44+
console.log(e.message);
45+
}
46+
`;
47+
}
48+
49+
// --- Sites 3 & 4: public http.setGlobalProxyFromEnv(), reachable without flags ---
50+
51+
{
52+
// Site 3: unparseable http_proxy.
53+
const { stdout, stderr } = runNode(setGlobalProxyFromEnv({
54+
http_proxy: `http://alice:${SECRET}@ `,
55+
}));
56+
assertNoLeak(stdout, 'site 3');
57+
assertNoLeak(stderr, 'site 3 stderr');
58+
assert.match(stdout, /http:\/\/\[redacted\]@/);
59+
}
60+
61+
{
62+
// Site 4: unparseable https_proxy.
63+
const { stdout, stderr } = runNode(setGlobalProxyFromEnv({
64+
https_proxy: `http://alice:${SECRET}@::bad`,
65+
}));
66+
assertNoLeak(stdout, 'site 4');
67+
assertNoLeak(stderr, 'site 4 stderr');
68+
assert.match(stdout, /http:\/\/\[redacted\]@/);
69+
}
70+
71+
// --- Site 1: ProxyConfig constructor, new URL() throws (reachable via Agent) ---
72+
73+
{
74+
const code = `try {
75+
new http.Agent({
76+
proxyEnv: { HTTP_PROXY: 'http://alice:${SECRET}@::bad' },
77+
});
78+
} catch (e) {
79+
console.log(e.message);
80+
}`;
81+
const { stdout, stderr } = runNode(code);
82+
assertNoLeak(stdout, 'site 1');
83+
assertNoLeak(stderr, 'site 1 stderr');
84+
assert.match(stdout, /Invalid proxy URL: http:\/\/\[redacted\]@/);
85+
}
86+
87+
// --- Site 2: proxy URL containing CR or LF ---
88+
89+
{
90+
const code = `try {
91+
require('http').setGlobalProxyFromEnv({
92+
http_proxy: 'http://alice:${SECRET}@\\nhost',
93+
});
94+
} catch (e) {
95+
console.log(e.message);
96+
}`;
97+
const { stdout, stderr } = runNode(code);
98+
assertNoLeak(stdout, 'site 2');
99+
assertNoLeak(stderr, 'site 2 stderr');
100+
}
101+
102+
// --- Site 1 & 2 via the environment-driven, opt-in path (NODE_USE_ENV_PROXY) ---
103+
104+
{
105+
const code = `require('http').request('http://example.com/').on('error', () => {}).end()`;
106+
const { stdout, stderr } = runNode(code, {
107+
NODE_USE_ENV_PROXY: '1',
108+
http_proxy: `http://alice:${SECRET}@:::bad`,
109+
});
110+
// Sites 1/2 throw during builtin module init, so stderr carries the message and
111+
// the process exits non-zero. The application cannot catch it.
112+
assertNoLeak(stdout, 'env-driven stdout');
113+
assertNoLeak(stderr, 'env-driven stderr');
114+
const { status } = runNode(code, {
115+
NODE_USE_ENV_PROXY: '1',
116+
http_proxy: `http://alice:${SECRET}@:::bad`,
117+
});
118+
assert.notStrictEqual(status, 0);
119+
}
120+
121+
// --- Control: well-formed URL with credentials is redacted on the normal path ---
122+
123+
{
124+
const code = `try {
125+
require('http').setGlobalProxyFromEnv({
126+
http_proxy: 'http://alice:${SECRET}@localhost:8080',
127+
});
128+
console.log('ok');
129+
} catch (e) {
130+
console.log('unexpected throw: ' + e.message);
131+
}`;
132+
const { stdout, status } = runNode(code);
133+
assert.strictEqual(status, 0);
134+
assert.strictEqual(stdout.trim(), 'ok');
135+
}
136+
137+
// --- Control: an unparseable URL with no credentials is left untouched ---
138+
139+
{
140+
const { stdout } = runNode(setGlobalProxyFromEnv({
141+
http_proxy: 'http://host:port',
142+
}));
143+
assert.strictEqual(stdout.trim(), 'http://host:port');
144+
assert.doesNotMatch(stdout, /\[redacted\]/);
145+
}

0 commit comments

Comments
 (0)