From d1a95b864c0245922155452d38867cd091cee85c Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Fri, 21 Aug 2026 14:38:29 +0000 Subject: [PATCH] A dot is not a hostname `normalizeUrl` guarded the host with `hostname.includes('.')`, and `new URL()` is far more permissive about hosts than DNS is: quotes, parens, `=` and `&` are not forbidden host code points. So `version="1.0"`, `text="gg.deals`, `zombies.)` and `z.` all parsed, all contained a dot, and all became feeds. That is how ~3,700 rows like `https://version="1.0"/` and `https://zombies.)/` reached the directory. The bulk-upload scanner splits pasted text on whitespace and offers every token as a URL, so pasting raw OPML instead of a URL list fed the markup itself in -- XML attributes, and sentences whose last word ended in a full stop. Each one is then re-crawled forever on a cadence and fails `blocked-host`; they were most of a 960/hour spike in that error. The check goes in `normalizeUrl` rather than in the scanner because it is the one gate every entry path shares -- web submit, OPML import, the queue drain and discovery -- and because the scanner runs in the browser, where importing `@rssamplifier/feed` fails the build on `node:dns`. It rejects what cannot be a hostname, not what is merely unusual: IDN is already punycode by then, a port and a dotted quad still pass, and private ranges go on being refused later by `isPublicHost`. The second test is the half that would break the directory if this were too strict, so it is spelled out. Co-Authored-By: Claude Opus 5 (1M context) --- packages/feed/src/discover.js | 57 ++++++++++++++++++++++++++++- packages/feed/test/security.test.js | 38 +++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/packages/feed/src/discover.js b/packages/feed/src/discover.js index 5db09a4..96a569d 100644 --- a/packages/feed/src/discover.js +++ b/packages/feed/src/discover.js @@ -52,7 +52,7 @@ export function normalizeUrl(input) { try { const u = new URL(raw); if (u.protocol !== 'http:' && u.protocol !== 'https:') return null; - if (!u.hostname.includes('.')) return null; + if (!plausibleHost(u.hostname)) return null; u.hash = ''; return u.toString(); } catch { @@ -60,6 +60,61 @@ export function normalizeUrl(input) { } } +/** + * Is this hostname something a publisher could actually be at? + * + * `new URL()` is far more permissive about hosts than DNS is. Quotes, parens, + * `=` and `&` are not forbidden host code points, so `new URL('https://x')` + * happily parses `version="1.0"`, `zombies.)` and `z.` into a hostname — and + * the check this replaced, `hostname.includes('.')`, waved all three through. + * + * That is not a theoretical gap. The bulk-upload scanner splits pasted text on + * whitespace and offers every token as a URL, so pasting raw OPML instead of a + * URL list turned the markup itself into feeds: `https://version="1.0"/`, + * `https://text="gg.deals/`, and a sentence ending in "zombies." into + * `https://zombies.)/`. Roughly 3,700 such rows reached the directory, and + * every one of them is crawled forever on a cadence, failing `blocked-host`. + * + * Validating here rather than in the scanner is deliberate: `normalizeUrl` is + * the single gate every entry path goes through -- web submit, OPML import, + * the queue drain and discovery -- so one check covers all of them, and the + * scanner cannot import this module anyway (it runs in the browser, and + * pulling `@rssamplifier/feed` into client code fails the build on `node:dns`). + * + * Kept deliberately loose about what a real domain looks like: this rejects + * things that cannot be hostnames, not things that are merely unusual. IDN is + * already punycode by the time it arrives, and a bare IPv4 literal is allowed + * through so that a feed genuinely served from one is not newly rejected -- + * private ranges are refused later, by `isPublicHost`. + * + * @param {string} hostname as parsed by `new URL`, so lowercased and punycoded + * @returns {boolean} + */ +function plausibleHost(hostname) { + const host = String(hostname ?? ''); + if (!host || host.length > 253) return false; + + // A dotted-quad is a legitimate, if unusual, place for a feed to live. + if (/^\d{1,3}(\.\d{1,3}){3}$/.test(host)) { + return host.split('.').every((n) => Number(n) <= 255); + } + + // Anything outside the DNS alphabet is markup or prose, not a host. + if (!/^[a-z0-9.-]+$/.test(host)) return false; + + const labels = host.split('.'); + // Two labels at least, none empty -- which is what rules out `z.` and `you.`, + // whose trailing dot leaves an empty final label. + if (labels.length < 2) return false; + if (labels.some((l) => l === '' || l.length > 63 || l.startsWith('-') || l.endsWith('-'))) { + return false; + } + + // A public suffix is letters. This is the part that rejects `1.0` while + // leaving `example.co.uk` and `xn--bcher-kva.de` alone. + return /^[a-z]{2,}$/.test(labels[labels.length - 1]); +} + /** * Extract feed URLs advertised by a page's tags. * diff --git a/packages/feed/test/security.test.js b/packages/feed/test/security.test.js index e63fa98..a3c6859 100644 --- a/packages/feed/test/security.test.js +++ b/packages/feed/test/security.test.js @@ -47,6 +47,44 @@ test('normalizeUrl adds a scheme but rejects dangerous ones', () => { assert.equal(normalizeUrl(''), null); }); +test('normalizeUrl rejects markup and prose that a dot alone would let through', () => { + // The bulk-upload scanner splits pasted text on whitespace and offers every + // token as a URL, so pasting raw OPML instead of a URL list turned the markup + // itself into feeds. The old check was `hostname.includes('.')`, and every + // one of these contains a dot: ~3,700 reached production, where they are + // re-crawled forever and fail `blocked-host`. + // + // These are the real stored feed_urls, with the `https://` the old code added + // and the trailing slash `URL.toString()` gave them. + for (const token of [ + 'version="1.0"', // an XML declaration + 'text="gg.deals', // an OPML attribute, torn at the quote + 'title="arnika.org', + 'zombies.)', // a sentence ending in a word and a bracket + 'yourself."', + 'z.', // a single letter and a full stop + 'you.', + 'york.', + ]) { + assert.equal(normalizeUrl(token), null, `${token} is not a hostname`); + } +}); + +test('normalizeUrl still accepts the unusual hosts that are real', () => { + // The gate rejects what cannot be a hostname, not what is merely unusual -- + // so this is the half of the change that would break the directory if it + // were wrong. + assert.equal(normalizeUrl('sub.domain.co.uk'), 'https://sub.domain.co.uk/'); + assert.equal(normalizeUrl('xn--bcher-kva.de'), 'https://xn--bcher-kva.de/', 'IDN arrives punycoded'); + assert.equal(normalizeUrl('EXAMPLE.COM'), 'https://example.com/'); + assert.equal(normalizeUrl('news.ycombinator.com/rss'), 'https://news.ycombinator.com/rss'); + assert.equal(normalizeUrl('http://example.com:8080/feed'), 'http://example.com:8080/feed', 'a port is fine'); + // A feed genuinely served from a dotted quad is odd but not impossible, and + // private ranges are refused later by isPublicHost rather than here. + assert.equal(normalizeUrl('192.168.1.1'), 'https://192.168.1.1/'); + assert.equal(normalizeUrl('999.1.1.1'), null, 'but not a quad that cannot be one'); +}); + test('findFeedLinks reads alternate links regardless of attribute order', () => { const html = `