From ac9cd4ab84e8d7858aaeafcf485c3da793b7ca2e Mon Sep 17 00:00:00 2001 From: Said Date: Mon, 19 Oct 2026 23:02:46 +0100 Subject: [PATCH] fix(overpass): bound query validation and reject cross-site requests --- docs/CURRENT-STATE.md | 15 +- scripts/format-scope.json | 3 +- scripts/package-boundaries.json | 2 + server/providers/overpass.js | 20 ++ server/providers/overpass/query.js | 36 +++- .../overpass/requestAdmission.test.mjs | 178 ++++++++++++++++++ 6 files changed, 243 insertions(+), 11 deletions(-) create mode 100644 server/providers/overpass/requestAdmission.test.mjs diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 60937dd05..bcc4e6fd3 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -2733,11 +2733,16 @@ down | auth-failed` (plus the unchanged `missing-key`/`unsupported`) with > first later successful fetch. Smaller chunks reduce timeout risk; complete > camera batches still wait for their sequential requests, and unresolved > placement continues to use the existing prior until real heights arrive. -> - **Overpass cache admission:** `/api/overpass` parses and sanitizes requests, -> then checks fresh memory, identical in-flight work, and fresh disk entries -> before invoking its local 90/min limiter. Cache and single-flight responses -> therefore do not spend quota; upstream-bound misses retain the existing -> limiter, configured-upstream, stale, and sanitization behavior. An unconfigured transport serves dated stale data or a non-retryable capability error. +> - **Overpass cache admission:** `/api/overpass` refuses cross-site and forwarded +> POSTs through the shared local-request gate, then limits requests to 300/min +> per socket peer and 900/min globally before reading or validating bodies. +> Tag-filter brackets are scanned once; nested or unmatched brackets return 400. +> It then checks fresh memory, identical in-flight work, and fresh disk entries +> before invoking its separate local 90/min upstream limiter. Cache and +> single-flight responses therefore do not spend upstream quota; upstream-bound +> misses retain the existing limiter, configured-upstream, stale, and sanitization +> behavior. An unconfigured transport serves dated stale data or a non-retryable +> capability error. > - **CCTV world-click focus:** clicking an in-world CCTV icon or ambient card > activates it and routes the camera flight through the panel FOCUS policy. > Aircraft/satellite tracking releases outside cockpit; cockpit retains the diff --git a/scripts/format-scope.json b/scripts/format-scope.json index 543b0f9a4..c7a95e6e3 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -1070,5 +1070,6 @@ "scripts/assemble-changelog.mjs", "src/tooling/changelogFragments.test.mjs", "src/data/gbfsProxyRateLimit.test.mjs", - "src/terrainHeightsBounds.test.mjs" + "src/terrainHeightsBounds.test.mjs", + "server/providers/overpass/requestAdmission.test.mjs" ] diff --git a/scripts/package-boundaries.json b/scripts/package-boundaries.json index ca5dc0c7f..55effa760 100644 --- a/scripts/package-boundaries.json +++ b/scripts/package-boundaries.json @@ -253,6 +253,7 @@ "server/providers/common/http.js", "server/providers/common/rate-limit.js", "server/providers/common/request.js", + "server/providers/common/same-site.js", "server/providers/overpass.js", "server/providers/overpass/cache.js", "server/providers/overpass/constants.js", @@ -262,6 +263,7 @@ "server/providers/places/routes.js", "src/data/placeProviderPayloads.js", "src/data/routeSteps.js", + "src/localRequestGate.mjs", "src/sources/httpBody.js", "src/i18n/marker.js", "src/sources/overpass.js", diff --git a/server/providers/overpass.js b/server/providers/overpass.js index e6631e889..3b5a2e575 100644 --- a/server/providers/overpass.js +++ b/server/providers/overpass.js @@ -1,5 +1,6 @@ import { makeRateLimiter, clientKey } from './common/rate-limit.js'; import { readRequestBodyCapped } from './common/request.js'; +import { admitSameSite } from './common/same-site.js'; import { OVERPASS_MAX_BODY_BYTES, OVERPASS_MAX_CONCURRENT, @@ -27,6 +28,14 @@ const _overpassInFlight = new Map(); let _overpassConcurrent = 0; +// Bound body-reading and validation work, including malformed requests and +// cache hits. Keep the tighter upstream budget below independent of this gate. +const _overpassRequestLimiter = makeRateLimiter({ + windowMs: 60_000, + max: 300, + globalMax: 900, +}); + const _overpassRateLimiter = makeRateLimiter({ windowMs: 60_000, max: 90, @@ -94,6 +103,17 @@ function overpassProxy({ routing = {} } = {}) { return; } + if (admitSameSite(req, res)) return; + if (!_overpassRequestLimiter(clientKey(req))) { + res.writeHead(429, { + 'Content-Type': 'application/json', + 'Cache-Control': 'no-store', + 'Retry-After': '60', + }); + res.end(JSON.stringify({ error: 'Rate limit exceeded' })); + return; + } + // Collect POST body with a hard byte cap (Overpass QL queries are small) let body; try { diff --git a/server/providers/overpass/query.js b/server/providers/overpass/query.js index cee444d3a..4091c9efd 100644 --- a/server/providers/overpass/query.js +++ b/server/providers/overpass/query.js @@ -18,6 +18,29 @@ function isOverpassBoundaryQuery(cacheKey) { return /is_in\s*\(|\bpivot\b/i.test(String(cacheKey || '')); } +// Input has already had quoted literals and comments removed. Consume each +// character once: a regex that searches for a closing bracket from every `[` is +// quadratic on an unterminated filter. Nested or unmatched brackets are invalid. +function stripTagFilters(text) { + const parts = []; + let start = 0; + let inFilter = false; + for (let i = 0; i < text.length; i += 1) { + if (text[i] === '[') { + if (inFilter) return null; + parts.push(text.slice(start, i), ' '); + inFilter = true; + } else if (text[i] === ']') { + if (!inFilter) return null; + inFilter = false; + start = i + 1; + } + } + if (inFilter) return null; + parts.push(text.slice(start)); + return parts.join(''); +} + /** * Validate + clamp an Overpass form body. Defends the generic proxy against * planet-scale abuse: requires exactly one `data` query in which EVERY element @@ -50,6 +73,10 @@ function sanitizeOverpassBody(rawBody) { // Blank quoted literals + strip comments in one lexer pass so a fake bound or a // `//` inside a string can't hide an unbounded selector (or satisfy a bound). const stripped = stripOverpassNoise(data); + const withoutTags = stripTagFilters(stripped); + if (withoutTags === null) { + return { ok: false, error: 'Malformed Overpass tag filter' }; + } // Reject oversized radii in EVERY around form — point `around:r,lat,lon` AND the // input-set form `around.set:r` — and parse the full numeric token so scientific @@ -93,20 +120,19 @@ function sanitizeOverpassBody(rawBody) { // statement. So `way[...]->.a` (global assigned to a set) is rejected, while the // app's `is_in(...)->.a; area.a[...]` and `area(id)->.x; rel(pivot.x)` validate. const boundedSets = new Set(); - for (let stmt of stripped.split(';')) { + for (let stmt of withoutTags.split(';')) { stmt = stmt.trim(); if (!stmt || stmt.startsWith('[') || /^out\b/.test(stmt)) continue; - // Strip output-set assignments (NOT input bounds), then strip bracket tag - // filters so a tag KEY/value (e.g. `way[is_in]`, `node[around]`) can never be + // Strip output-set assignments (NOT input bounds). Bracket tag filters were + // removed above so a tag KEY/value (e.g. `way[is_in]`, `node[around]`) cannot be // misread as a spatial bound. Bounds live in (...) / function calls / set // refs, never inside [...], so the probe loses nothing real. const outSets = []; - const body = stmt.replace(/->\s*\.(\w+)/g, (_, name) => { + const probe = stmt.replace(/->\s*\.(\w+)/g, (_, name) => { outSets.push(name); return ' '; }); - const probe = body.replace(/\[[^\]]*\]/g, ' '); // Reject element-in-area scans on the TAG-STRIPPED probe, so a tag filter // between the selector and the area filter (way["highway"](area.a)) can't hide diff --git a/server/providers/overpass/requestAdmission.test.mjs b/server/providers/overpass/requestAdmission.test.mjs new file mode 100644 index 000000000..ecd642320 --- /dev/null +++ b/server/providers/overpass/requestAdmission.test.mjs @@ -0,0 +1,178 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { Readable } from 'node:stream'; +import { spawnSync } from 'node:child_process'; +import { overpassProxy } from '../overpass.js'; +import { sanitizeOverpassBody } from './query.js'; + +const form = (query) => `data=${encodeURIComponent(query)}`; + +test('bracket validation rejects malformed filters without trusting their contents as bounds', () => { + for (const query of [ + 'node[', + 'node]', + 'node[[name]](around:20,1,1);out;', + 'node[name=around:20];out;', + 'node[is_in];out;', + 'node[name="(around:20,1,1)"];out;', + 'node(around:20,1,1);way[name];out;', + 'way[highway](area.a);out;', + 'node[->.a](around:20,1,1);way.a;out;', + ]) { + assert.equal(sanitizeOverpassBody(form(query)).ok, false, query); + } +}); + +test('bounded queries preserve quoted brackets, comments, multiple tags and timeout clamping', () => { + for (const query of [ + '[out:json][timeout:999];node[name="[Town]; ]"](around:20,1,1);out;', + '/* [ */way[highway][name](30.1,-97.9,30.5,-97.5);out geom;', + 'is_in(30.2,-97.7)->.a;area.a[boundary="administrative"];out tags;', + 'area(3600039152)->.x;rel(pivot.x);out geom;', + ]) { + const result = sanitizeOverpassBody(form(query)); + assert.equal(result.ok, true, query); + assert.equal( + new URLSearchParams(result.body).get('data'), + query.replace('[timeout:999]', '[timeout:30]'), + ); + } +}); + +test('a burst of maximum-size unterminated filters completes within a bounded process deadline', () => { + // A child deadline can stop synchronous regex backtracking; a test timeout in + // the same event loop cannot. The old parser takes several seconds for this. + const result = spawnSync( + process.execPath, + [ + '--input-type=module', + '-e', + `import { sanitizeOverpassBody } from ${JSON.stringify(new URL('./query.js', import.meta.url).href)}; + for (let i = 0; i < 20; i++) { + if (sanitizeOverpassBody('data=node' + '['.repeat(24000)).ok) process.exit(1); + }`, + ], + { timeout: 2000, encoding: 'utf8' }, + ); + assert.ifError(result.error); + assert.equal(result.status, 0, result.stderr); +}); + +function handler(hook = 'configureServer') { + let route; + overpassProxy()[hook]({ + middlewares: { + use(path, fn) { + if (path === '/api/overpass') route = fn; + }, + }, + }); + return route; +} + +async function call( + route, + { + headers = {}, + peer = '127.0.0.1', + body = 'data=node[', + forbidRead = false, + method = 'POST', + url = '/', + } = {}, +) { + let reads = 0; + const req = new Readable({ + read() { + reads++; + assert.equal(forbidRead, false, 'refused request must not read its body'); + this.push(Buffer.from(body)); + this.push(null); + }, + }); + Object.assign(req, { + method, + url, + headers: { host: 'localhost:4173', ...headers }, + socket: { remoteAddress: peer }, + }); + const res = { + statusCode: 200, + headers: {}, + setHeader(key, value) { + this.headers[key.toLowerCase()] = value; + }, + writeHead(status, values) { + this.statusCode = status; + for (const [key, value] of Object.entries(values)) + this.setHeader(key, value); + }, + end(value) { + this.body = value; + }, + }; + await route(req, res); + return { ...res, reads }; +} + +for (const hook of ['configureServer', 'configurePreviewServer']) { + test(`${hook}: cross-site requests are rejected before consuming a body or fetching`, async (t) => { + t.mock.method(globalThis, 'fetch', () => + assert.fail('unexpected upstream fetch'), + ); + const route = handler(hook); + for (const headers of [ + { origin: 'https://foreign.example', 'content-type': 'text/plain' }, + { 'sec-fetch-site': 'cross-site' }, + { 'sec-fetch-site': 'same-site' }, + { origin: 'null' }, + { 'x-forwarded-for': '127.0.0.1' }, + ]) { + const response = await call(route, { headers, forbidRead: true }); + assert.equal(response.statusCode, 403); + assert.equal(response.reads, 0); + assert.equal(response.headers['cache-control'], 'no-store'); + } + }); +} + +test('same-origin, LAN and non-browser requests still reach validation; GET status stays available', async () => { + const route = handler(); + for (const headers of [ + {}, + { origin: 'http://localhost:4173', 'sec-fetch-site': 'same-origin' }, + { host: '192.168.1.10:4173', origin: 'http://192.168.1.10:4173' }, + ]) { + const response = await call(route, { headers }); + assert.equal(response.statusCode, 400); + assert.equal(response.reads, 1); + assert.match(response.body, /Malformed Overpass tag filter/); + } + const response = await call(route, { + method: 'GET', + url: '/status', + forbidRead: true, + }); + assert.equal(response.statusCode, 200); + assert.equal(typeof JSON.parse(response.body).configured, 'boolean'); +}); + +test('invalid requests spend admission quota before parsing, including on an unconfigured server', async (t) => { + let now = Date.now() + 120000; + t.mock.method(Date, 'now', () => now); + t.mock.method(globalThis, 'fetch', () => + assert.fail('unexpected upstream fetch'), + ); + const route = handler(); + const peer = 'admission-limit-test'; + for (let i = 0; i < 300; i++) { + assert.equal((await call(route, { peer })).statusCode, 400); + } + const denied = await call(route, { peer, forbidRead: true }); + assert.equal(denied.statusCode, 429); + assert.equal(denied.reads, 0); + assert.equal(denied.headers['retry-after'], '60'); + assert.equal((await call(route, { peer: 'other-peer' })).statusCode, 400); + now += 60001; + assert.equal((await call(route, { peer })).statusCode, 400); +});