From ca0f2f2c325f3c01ce7e525a64b48be918a543de Mon Sep 17 00:00:00 2001 From: s4luorth Date: Mon, 8 Jun 2026 06:52:22 +0200 Subject: [PATCH] fix: SSRF-haertung (IP-pinning) + blank-page-schutz aus security-review - scan & releases/from-url: verbindung wird an die bereits validierte IP gepinnt (node:http/https lookup-option), statt fetch erneut aufloesen zu lassen. Schliesst DNS-rebinding/TOCTOU, mit dem ein lizenzierter Kunde ueber die DNS seiner eigenen Domain interne Dienste/Cloud-Metadaten erreichen koennte. - releases/from-url folgt redirects jetzt manuell und validiert jeden hop (protokoll + private-IP-guard); gitea-token nur an den ausgangs-host. - pinnedRequest: settle-guard + body-cap loesen das promise auch bei ueberlangen antworten (kein haengen). - autodetect process(): kein blank-page mehr bei PCRE-fehler (fallback auf original-HTML statt (string) null = ""). - cleanup: ungenutztes extract_iframe_src() entfernt; redundantes &-replace in get_src() entfernt (DOMDocument dekodiert bereits). Co-Authored-By: Claude Opus 4.8 --- .../includes/class-autodetect.php | 16 ++- .../includes/class-renderer.php | 5 - license-backend/src/server.js | 124 +++++++++++++++--- 3 files changed, 113 insertions(+), 32 deletions(-) diff --git a/gdpr-content-blocker/includes/class-autodetect.php b/gdpr-content-blocker/includes/class-autodetect.php index 6c2c941..075e25a 100644 --- a/gdpr-content-blocker/includes/class-autodetect.php +++ b/gdpr-content-blocker/includes/class-autodetect.php @@ -52,13 +52,18 @@ class CB_Autodetect { } // Locate iframe blocks only. Attribute parsing happens via DOMDocument below. - return (string) preg_replace_callback( + $out = preg_replace_callback( '#]*>.*?#is', function ( array $m ) use ( $services ): string { return self::maybe_replace_iframe( $m[0], $services ); }, $html ); + + // On a PCRE error (e.g. backtrack/recursion limit on a huge page), + // preg_replace_callback returns null. Never blank the page — fall back to + // the original, unmodified HTML. + return $out === null ? $html : $out; } /** @@ -99,11 +104,8 @@ class CB_Autodetect { return ''; } - $src = trim( $node->getAttribute( 'src' ) ); - - // Normalise protocol-relative and HTML-entity-encoded ampersands. - $src = str_replace( '&', '&', $src ); - - return $src; + // getAttribute() already returns the entity-decoded value (e.g. & → &), + // so no further unescaping is needed here. + return trim( $node->getAttribute( 'src' ) ); } } diff --git a/gdpr-content-blocker/includes/class-renderer.php b/gdpr-content-blocker/includes/class-renderer.php index fbe39cd..3efd59d 100644 --- a/gdpr-content-blocker/includes/class-renderer.php +++ b/gdpr-content-blocker/includes/class-renderer.php @@ -288,11 +288,6 @@ class CB_Renderer { . ''; } - /** Pull the src attribute out of an iframe string. */ - public static function extract_iframe_src( string $html ): string { - return self::extract_iframe_attrs( $html )['src']; - } - /** * Pull src + width + height out of an iframe string. * Height is also read from an inline style="height:NNNpx" if no attribute. diff --git a/license-backend/src/server.js b/license-backend/src/server.js index c77b19c..5dd9769 100644 --- a/license-backend/src/server.js +++ b/license-backend/src/server.js @@ -1,4 +1,6 @@ import express from 'express'; +import http from 'node:http'; +import https from 'node:https'; import { lookup } from 'node:dns/promises'; import { mkdirSync, writeFileSync, existsSync, statSync, createReadStream } from 'node:fs'; import { join } from 'node:path'; @@ -35,6 +37,60 @@ const GITEA_TOKEN = process.env.GITEA_TOKEN || ''; mkdirSync(RELEASES_DIR, { recursive: true }); +/** + * HTTP(S) GET pinned to a pre-resolved, already-validated IP address. The socket + * connects to `address` while SNI/Host stay the original hostname, so TLS still + * verifies against the certificate. This closes the DNS-rebinding/TOCTOU gap + * where the global fetch() would re-resolve the host (possibly to a private IP) + * AFTER our isPrivateIp() check. Does not follow redirects. + * Resolves to { status, headers, buffer }; rejects on network/timeout error. + */ +function pinnedRequest(targetUrl, address, family, { maxBytes, timeoutMs, headers = {} }) { + return new Promise((resolve, reject) => { + const u = new URL(targetUrl); + const mod = u.protocol === 'https:' ? https : http; + let settled = false; + const finish = (fn, arg) => { + if (settled) return; + settled = true; + fn(arg); + }; + const req = mod.request( + targetUrl, + { + method: 'GET', + headers, + // Force the connection to the validated IP (handles both lookup + // callback signatures: with and without options.all). + lookup: (_hostname, opts, cb) => + opts && opts.all ? cb(null, [{ address, family }]) : cb(null, address, family), + }, + (res) => { + const chunks = []; + let bytes = 0; + const result = () => ({ status: res.statusCode || 0, headers: res.headers, buffer: Buffer.concat(chunks) }); + res.on('data', (c) => { + if (settled) return; + bytes += c.length; + if (bytes <= maxBytes) { + chunks.push(c); + } else { + // Cap the body: stop reading and resolve with what we have now. + // (destroy() suppresses 'end', so we must settle here ourselves.) + res.destroy(); + finish(resolve, result()); + } + }); + res.on('end', () => finish(resolve, result())); + res.on('error', (e) => finish(reject, e)); + } + ); + req.setTimeout(timeoutMs, () => req.destroy(new Error('request timed out'))); + req.on('error', (e) => finish(reject, e)); + req.end(); + }); +} + const PORT = Number(process.env.PORT || 8080); const ADMIN_TOKEN = process.env.ADMIN_API_TOKEN || ''; @@ -299,9 +355,9 @@ app.post('/api/v1/scan', async (req, res) => { // SSRF hardening: resolve the host and refuse private/link-local IPs // (e.g. a public hostname pointed at 169.254.169.254 cloud metadata). const host = new URL(t).hostname; - let address; + let address, family; try { - ({ address } = await lookup(host)); + ({ address, family } = await lookup(host)); } catch { pages.push({ url: t, error: 'dns lookup failed', resources: [] }); continue; @@ -311,17 +367,20 @@ app.post('/api/v1/scan', async (req, res) => { continue; } - const r = await fetch(t, { + // Connect to the validated IP (no re-resolution); do not follow redirects. + const r = await pinnedRequest(t, address, family, { + maxBytes: MAX_SCAN_BYTES, + timeoutMs: SCAN_TIMEOUT_MS, headers: { 'User-Agent': 'ContentBlockerScanner/1.0', Accept: 'text/html' }, - redirect: 'manual', // do not auto-follow into unvalidated hosts - signal: AbortSignal.timeout(SCAN_TIMEOUT_MS), }); if (r.status >= 300 && r.status < 400) { pages.push({ url: t, error: `redirect (${r.status}) not followed`, resources: [] }); continue; } - const buf = await r.text(); - pages.push({ url: t, resources: extractResources(buf.slice(0, MAX_SCAN_BYTES), t) }); + pages.push({ + url: t, + resources: extractResources(r.buffer.toString('utf8').slice(0, MAX_SCAN_BYTES), t), + }); } catch (e) { pages.push({ url: t, error: String(e?.message || e), resources: [] }); } @@ -475,21 +534,46 @@ app.post('/api/v1/releases/from-url', adminOnly, async (req, res) => { return fail(res, 400, 'zip_url not allowed (must start with GITEA_BASE_URL)'); } - // SSRF guard: refuse private/loopback targets. - try { - const { address } = await lookup(url.hostname); - if (isPrivateIp(address)) return fail(res, 400, 'zip_url resolves to a private address'); - } catch { - return fail(res, 400, 'dns lookup failed for zip_url'); - } - + // Fetch the ZIP, following redirects manually so EACH hop is re-validated + // (protocol + DNS → private-IP guard + IP pinning). The global fetch with + // redirect:'follow' would only check the first URL and could be bounced into + // an internal target. let buf; try { - const headers = { 'User-Agent': 'ContentBlockerReleaseFetcher/1.0' }; - if (GITEA_TOKEN) headers.Authorization = 'token ' + GITEA_TOKEN; - const r = await fetch(zipUrl, { headers, redirect: 'follow', signal: AbortSignal.timeout(30000) }); - if (!r.ok) return fail(res, 502, 'fetch failed: HTTP ' + r.status); - buf = Buffer.from(await r.arrayBuffer()); + const initialHost = url.hostname; + let current = zipUrl; + for (let hop = 0; ; hop++) { + if (hop > 5) return fail(res, 502, 'too many redirects for zip_url'); + const u = new URL(current); + if (!/^https?:$/.test(u.protocol)) return fail(res, 400, 'zip_url redirect to non-http(s)'); + + let address, family; + try { + ({ address, family } = await lookup(u.hostname)); + } catch { + return fail(res, 400, 'dns lookup failed for zip_url'); + } + if (isPrivateIp(address)) return fail(res, 400, 'zip_url resolves to a private address'); + + const headers = { 'User-Agent': 'ContentBlockerReleaseFetcher/1.0' }; + // Only attach the Gitea token to the original host — never leak it to a + // redirect target. + if (GITEA_TOKEN && u.hostname === initialHost) headers.Authorization = 'token ' + GITEA_TOKEN; + + const r = await pinnedRequest(current, address, family, { + maxBytes: MAX_ZIP_BYTES, + timeoutMs: 30000, + headers, + }); + + if (r.status >= 300 && r.status < 400 && r.headers.location) { + current = new URL(r.headers.location, current).href; + continue; + } + if (r.status < 200 || r.status >= 300) return fail(res, 502, 'fetch failed: HTTP ' + r.status); + buf = r.buffer; + break; + } } catch (e) { return fail(res, 502, 'fetch error: ' + String(e?.message || e)); }