Skip to content

Commit 9b59c01

Browse files
committed
fix(brainstorm-server): validate Host header to defeat DNS rebinding
Allowlist the HTTP `Host` header on both the request path and the WebSocket upgrade. Default allowlist covers `localhost`, `127.0.0.1`, `[::1]` (with and without :PORT), plus the configured `BRAINSTORM_HOST` and `BRAINSTORM_URL_HOST`. Operators can extend with `BRAINSTORM_ALLOWED_HOSTS` (comma-separated) for tunneled setups. Without this gate, a page on another origin can DNS-rebind its own hostname to 127.0.0.1, hit `/` or `/files/*` on the running brainstorm companion server, and read the active screen + content files even though the listener is loopback-only. The same rebind would also let the page complete a WebSocket upgrade and inject `{choice: ...}` events into `state_dir/events`, which the agent reads as the user's selection. PR #1110 / issue #1014 already cover the WebSocket Origin axis. This PR adds the complementary Host axis, which is the canonical defense against DNS rebinding (Origin alone doesn't cover `/files/*`). Tests: 6 new cases in tests/brainstorm-server/server.test.js cover loopback accept, foreign-Host 421, /files/* foreign-Host 421, and a forged-Host WebSocket upgrade. Existing 25 tests still pass (31/31). Detected by: Aeon (https://github.com/aaronjmars/aeon-aaron) + manual review against the brainstorm-server attack surface. Severity: medium (cross-origin info disclosure + agent input injection via DNS rebinding). CWE-346 (Origin Validation Error), CWE-350 (Reliance on Reverse DNS Resolution for a Security-Critical Action).
1 parent f2cbfbe commit 9b59c01

2 files changed

Lines changed: 135 additions & 0 deletions

File tree

skills/brainstorming/scripts/server.cjs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,51 @@ const CONTENT_DIR = path.join(SESSION_DIR, 'content');
8181
const STATE_DIR = path.join(SESSION_DIR, 'state');
8282
let ownerPid = process.env.BRAINSTORM_OWNER_PID ? Number(process.env.BRAINSTORM_OWNER_PID) : null;
8383

84+
// ========== Host Header Allowlist (DNS-rebinding defense) ==========
85+
//
86+
// Without validating the HTTP Host header, a page on another origin can
87+
// DNS-rebind its own hostname to 127.0.0.1 and then read this server's
88+
// `/` and `/files/*` responses as if same-origin. The browser sends the
89+
// rebound name in `Host`, so allowlisting Host on the loopback path
90+
// closes the rebinding surface even when the TCP connection itself is
91+
// genuinely local.
92+
//
93+
// Default allowlist: the configured URL_HOST (what the agent prints in
94+
// the start-server.sh URL) plus the literal loopback names, both bare
95+
// and with the listening port appended. Operators serving the companion
96+
// behind a tunnel or container hostname can extend the list with the
97+
// BRAINSTORM_ALLOWED_HOSTS env var (comma-separated, case-insensitive).
98+
function buildAllowedHosts() {
99+
const set = new Set();
100+
const portStr = String(PORT);
101+
const add = (h) => {
102+
if (!h) return;
103+
const hl = String(h).trim().toLowerCase();
104+
if (!hl) return;
105+
set.add(hl);
106+
set.add(hl + ':' + portStr);
107+
};
108+
// Always allow the loopback names: every legitimate browser session
109+
// resolves the server via one of these even when URL_HOST differs.
110+
add('localhost');
111+
add('127.0.0.1');
112+
add('[::1]');
113+
// Allow the printed/displayed URL_HOST and the bind HOST verbatim.
114+
add(URL_HOST);
115+
add(HOST);
116+
// Operator-provided extras for tunneled / containerized setups.
117+
const extra = process.env.BRAINSTORM_ALLOWED_HOSTS || '';
118+
for (const h of extra.split(',')) add(h);
119+
return set;
120+
}
121+
const ALLOWED_HOSTS = buildAllowedHosts();
122+
123+
function isHostAllowed(req) {
124+
const host = req.headers && req.headers.host;
125+
if (typeof host !== 'string' || host.length === 0) return false;
126+
return ALLOWED_HOSTS.has(host.toLowerCase());
127+
}
128+
84129
const MIME_TYPES = {
85130
'.html': 'text/html', '.css': 'text/css', '.js': 'application/javascript',
86131
'.json': 'application/json', '.png': 'image/png', '.jpg': 'image/jpeg',
@@ -127,6 +172,14 @@ function getNewestScreen() {
127172
// ========== HTTP Request Handler ==========
128173

129174
function handleRequest(req, res) {
175+
if (!isHostAllowed(req)) {
176+
// Reject DNS-rebound / cross-origin Host headers before touching state
177+
// or returning any content. 421 Misdirected Request is the canonical
178+
// status for "this connection is not authoritative for this Host".
179+
res.writeHead(421, { 'Content-Type': 'text/plain' });
180+
res.end('Misdirected Request');
181+
return;
182+
}
130183
touchActivity();
131184
if (req.method === 'GET' && req.url === '/') {
132185
const screenFile = getNewestScreen();
@@ -168,6 +221,12 @@ function handleUpgrade(req, socket) {
168221
const key = req.headers['sec-websocket-key'];
169222
if (!key) { socket.destroy(); return; }
170223

224+
// Same DNS-rebinding gate as handleRequest, applied before completing
225+
// the WebSocket handshake. Without this, a DNS-rebound page could
226+
// open a WS connection and inject events into state_dir/events even
227+
// though the TCP socket is loopback-bound.
228+
if (!isHostAllowed(req)) { socket.destroy(); return; }
229+
171230
const accept = computeAcceptKey(key);
172231
socket.write(
173232
'HTTP/1.1 101 Switching Protocols\r\n' +

tests/brainstorm-server/server.test.js

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,31 @@ async function fetch(url) {
4545
});
4646
}
4747

48+
// Like fetch() but lets the caller override the Host header so we can
49+
// exercise the DNS-rebinding allowlist directly. The TCP target stays
50+
// 127.0.0.1:PORT; only the HTTP/1.1 Host line is forged.
51+
async function fetchWithHost(path, hostHeader) {
52+
return new Promise((resolve, reject) => {
53+
const req = http.request({
54+
host: '127.0.0.1',
55+
port: TEST_PORT,
56+
path,
57+
method: 'GET',
58+
headers: { Host: hostHeader }
59+
}, (res) => {
60+
let data = '';
61+
res.on('data', chunk => data += chunk);
62+
res.on('end', () => resolve({
63+
status: res.statusCode,
64+
headers: res.headers,
65+
body: data
66+
}));
67+
});
68+
req.on('error', reject);
69+
req.end();
70+
});
71+
}
72+
4873
function startServer() {
4974
return spawn('node', [SERVER_PATH], {
5075
env: { ...process.env, BRAINSTORM_PORT: TEST_PORT, BRAINSTORM_DIR: TEST_DIR }
@@ -184,6 +209,57 @@ async function runTests() {
184209
assert.strictEqual(res.status, 404);
185210
});
186211

212+
// ========== Host Header Validation (DNS-rebinding defense) ==========
213+
console.log('\n--- Host Header Validation ---');
214+
215+
await test('accepts requests with Host: localhost:PORT', async () => {
216+
const res = await fetchWithHost('/', `localhost:${TEST_PORT}`);
217+
assert.strictEqual(res.status, 200);
218+
});
219+
220+
await test('accepts requests with Host: 127.0.0.1:PORT', async () => {
221+
const res = await fetchWithHost('/', `127.0.0.1:${TEST_PORT}`);
222+
assert.strictEqual(res.status, 200);
223+
});
224+
225+
await test('accepts bare loopback Host without port', async () => {
226+
const res = await fetchWithHost('/', 'localhost');
227+
assert.strictEqual(res.status, 200);
228+
});
229+
230+
await test('rejects DNS-rebound Host with 421', async () => {
231+
const res = await fetchWithHost('/', `evil.example:${TEST_PORT}`);
232+
assert.strictEqual(res.status, 421, 'foreign Host should be rejected');
233+
assert(!res.body.includes('Waiting for the agent'), 'should not leak screen content');
234+
});
235+
236+
await test('rejects DNS-rebound Host on /files/ endpoint', async () => {
237+
const res = await fetchWithHost('/files/anything', 'attacker.example');
238+
assert.strictEqual(res.status, 421, 'foreign Host should be rejected before file lookup');
239+
});
240+
241+
await test('rejects WebSocket upgrade from foreign Host', async () => {
242+
// Connect directly to the loopback address but pretend the Host is
243+
// a rebound name. The ws client sends `Host: <hostname>:<port>` by
244+
// default, so we point its URL at evil.example and route the TCP
245+
// connection back to the loopback listener.
246+
const ws = new WebSocket(`ws://evil.example:${TEST_PORT}`, {
247+
lookup: (_hostname, _opts, cb) => cb(null, '127.0.0.1', 4)
248+
});
249+
let opened = false;
250+
let errored = false;
251+
await new Promise((resolve) => {
252+
ws.on('open', () => { opened = true; resolve(); });
253+
ws.on('error', () => { errored = true; resolve(); });
254+
ws.on('unexpected-response', () => resolve());
255+
ws.on('close', resolve);
256+
setTimeout(resolve, 1500);
257+
});
258+
try { ws.terminate(); } catch (_) { /* ignore */ }
259+
assert(!opened, 'WS upgrade with foreign Host must not complete');
260+
assert(errored, 'WS client should observe a connection error');
261+
});
262+
187263
// ========== WebSocket Communication ==========
188264
console.log('\n--- WebSocket Communication ---');
189265

0 commit comments

Comments
 (0)