fix codeql findings: youtube host check and redos-prone url regex
- ExternalPlayer gated WebView navigation with
event.url.includes('www.youtube.com'), which also matches hostile
URLs like https://www.youtube.com.evil.com; parse and compare the
hostname exactly.
- isValidUrlAndDomain used a nested-quantifier regex that backtracks
exponentially on inputs like '//0.' + '00.'.repeat(n), and it runs on
every composer keystroke; rewritten as linear-time parsing with the
same semantics (http/https/ftp/protocol-relative, userinfo, public
IPv4 or domain host, 2-5 digit port, whitespace-free path). New test
file pins accept/reject behavior plus ReDoS canaries.
Both pre-existing (2024) issues surfaced by CodeQL on the migration PR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -85,10 +85,19 @@ function Player({
|
|||||||
// ensures we only load what's requested
|
// ensures we only load what's requested
|
||||||
// when it's a youtube video, we need to allow both bsky.app and youtube.com
|
// when it's a youtube video, we need to allow both bsky.app and youtube.com
|
||||||
const onShouldStartLoadWithRequest = useCallback(
|
const onShouldStartLoadWithRequest = useCallback(
|
||||||
(event: ShouldStartLoadRequest) =>
|
(event: ShouldStartLoadRequest) => {
|
||||||
event.url === params.playerUri ||
|
if (event.url === params.playerUri) return true
|
||||||
(params.source.startsWith('youtube') &&
|
if (!params.source.startsWith('youtube')) return false
|
||||||
event.url.includes('www.youtube.com')),
|
/*
|
||||||
|
* Compare the host exactly. A substring check like `includes` would also
|
||||||
|
* match hostile URLs such as `https://www.youtube.com.evil.com`.
|
||||||
|
*/
|
||||||
|
try {
|
||||||
|
return new URL(event.url).hostname === 'www.youtube.com'
|
||||||
|
} catch {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
},
|
||||||
[params.playerUri, params.source],
|
[params.playerUri, params.source],
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,115 @@
|
|||||||
|
import {describe, expect, it} from '@jest/globals'
|
||||||
|
|
||||||
|
import {type LinkFacetMatch, suggestLinkCardUri} from './text-input-util'
|
||||||
|
|
||||||
|
/*
|
||||||
|
* `isValidUrlAndDomain` is module-private, so it is exercised here through its
|
||||||
|
* only caller. In "suggest immediately" mode, suggestLinkCardUri returns a URI
|
||||||
|
* iff it passes isValidUrlAndDomain, which lets us assert URL validity directly.
|
||||||
|
*/
|
||||||
|
function isValidUrl(uri: string): boolean {
|
||||||
|
const next = new Map<string, LinkFacetMatch>([[uri, {} as LinkFacetMatch]])
|
||||||
|
const result = suggestLinkCardUri(
|
||||||
|
/*suggestLinkImmediately*/ true,
|
||||||
|
next,
|
||||||
|
new Map(),
|
||||||
|
new Set(),
|
||||||
|
)
|
||||||
|
return result === uri
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('isValidUrlAndDomain (via suggestLinkCardUri)', () => {
|
||||||
|
describe('accepts', () => {
|
||||||
|
it.each([
|
||||||
|
'http://example.com',
|
||||||
|
'https://example.com',
|
||||||
|
'ftp://example.com',
|
||||||
|
'//example.com',
|
||||||
|
'https://example.com/',
|
||||||
|
'https://example.com/path/to/page',
|
||||||
|
'https://example.com/path?query=1#frag',
|
||||||
|
'https://sub.example.co.uk',
|
||||||
|
'https://example.com:8080',
|
||||||
|
'https://example.com:8080/path',
|
||||||
|
'https://user:pass@example.com',
|
||||||
|
'https://user@example.com/path',
|
||||||
|
'https://xn--80ak6aa92e.com',
|
||||||
|
'https://münchen.de',
|
||||||
|
'https://ex-ample.com',
|
||||||
|
'https://a--b.example.com',
|
||||||
|
'https://1.example.com',
|
||||||
|
// public IPv4
|
||||||
|
'http://8.8.8.8',
|
||||||
|
'http://1.2.3.4',
|
||||||
|
'http://223.255.255.254',
|
||||||
|
'http://8.8.8.8:8080/path',
|
||||||
|
])('%s', uri => {
|
||||||
|
expect(isValidUrl(uri)).toBe(true)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('rejects private / reserved IPv4 ranges', () => {
|
||||||
|
it.each([
|
||||||
|
'http://10.0.0.1',
|
||||||
|
'http://10.255.255.255',
|
||||||
|
'http://127.0.0.1',
|
||||||
|
'http://169.254.1.1',
|
||||||
|
'http://192.168.0.1',
|
||||||
|
'http://172.16.0.1',
|
||||||
|
'http://172.31.255.255',
|
||||||
|
])('%s', uri => {
|
||||||
|
expect(isValidUrl(uri)).toBe(false)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('rejects out-of-range IPv4 octets', () => {
|
||||||
|
it.each([
|
||||||
|
'http://0.0.0.0', // first octet 0
|
||||||
|
'http://224.0.0.1', // first octet > 223
|
||||||
|
'http://999.1.1.1',
|
||||||
|
'http://1.2.3.0', // last octet 0
|
||||||
|
'http://1.2.3.255', // last octet 255
|
||||||
|
'http://256.1.1.1',
|
||||||
|
'http://1.2.3', // too few octets
|
||||||
|
'http://1.2.3.4.5', // too many octets
|
||||||
|
])('%s', uri => {
|
||||||
|
expect(isValidUrl(uri)).toBe(false)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('rejects malformed input', () => {
|
||||||
|
it.each([
|
||||||
|
'example.com', // no protocol / no leading //
|
||||||
|
'http:/example.com', // missing second slash
|
||||||
|
'httpx://example.com', // unsupported scheme
|
||||||
|
'gopher://example.com',
|
||||||
|
'https://example', // bare TLD-less host
|
||||||
|
'https://com', // single label
|
||||||
|
'https://example.c', // TLD too short
|
||||||
|
'https://-example.com', // leading hyphen
|
||||||
|
'https://example-.com', // trailing hyphen
|
||||||
|
'https://example.c0m', // digit in TLD
|
||||||
|
'https://example..com', // empty label
|
||||||
|
'https://example.com:1', // port too short
|
||||||
|
'https://example.com:123456', // port too long
|
||||||
|
'https://example.com:abc', // non-numeric port
|
||||||
|
'https://exa mple.com', // whitespace
|
||||||
|
'https://example.com/pa th', // whitespace in path
|
||||||
|
'https://@example.com', // empty userinfo
|
||||||
|
'',
|
||||||
|
'https://',
|
||||||
|
])('%s', uri => {
|
||||||
|
expect(isValidUrl(uri)).toBe(false)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('ReDoS canaries return quickly', () => {
|
||||||
|
it('rejects long "0." repetition without hanging', () => {
|
||||||
|
expect(isValidUrl('//' + '0.'.repeat(50000))).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('rejects long "00." repetition without hanging', () => {
|
||||||
|
expect(isValidUrl('//0.' + '00.'.repeat(50000))).toBe(false)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -84,11 +84,118 @@ export function suggestLinkCardUri(
|
|||||||
return suggestedUri
|
return suggestedUri
|
||||||
}
|
}
|
||||||
|
|
||||||
// https://stackoverflow.com/questions/8667070/javascript-regular-expression-to-validate-url
|
/**
|
||||||
// question credit Muhammad Imran Tariq https://stackoverflow.com/users/420613/muhammad-imran-tariq
|
* A single alphanumeric domain character (unicode letters or digits).
|
||||||
// answer credit Christian David https://stackoverflow.com/users/967956/christian-david
|
*/
|
||||||
function isValidUrlAndDomain(value: string) {
|
const DOMAIN_ALNUM = /^[a-z\u00a1-\uffff0-9]$/i
|
||||||
return /^(?:(?:(?:https?|ftp):)?\/\/)(?:\S+(?::\S*)?@)?(?:(?!(?:10|127)(?:\.\d{1,3}){3})(?!(?:169\.254|192\.168)(?:\.\d{1,3}){2})(?!172\.(?:1[6-9]|2\d|3[0-1])(?:\.\d{1,3}){2})(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]-*)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]-*)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,})))(?::\d{2,5})?(?:[/?#]\S*)?$/i.test(
|
/**
|
||||||
value,
|
* The final label (TLD): unicode letters only, 2 or more characters. Single
|
||||||
)
|
* repetition of a non-overlapping class, so this is linear-time.
|
||||||
|
*/
|
||||||
|
const DOMAIN_TLD = /^[a-z\u00a1-\uffff]{2,}$/i
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validates a single non-final domain label: unicode letters/digits with
|
||||||
|
* interior hyphens only (hyphen may not lead or trail), at least one character.
|
||||||
|
* Implemented charwise to stay unambiguously linear-time (no backtracking).
|
||||||
|
*/
|
||||||
|
function isValidDomainLabel(label: string): boolean {
|
||||||
|
if (label.length === 0) return false
|
||||||
|
if (!DOMAIN_ALNUM.test(label[0])) return false
|
||||||
|
if (!DOMAIN_ALNUM.test(label[label.length - 1])) return false
|
||||||
|
for (let i = 1; i < label.length - 1; i++) {
|
||||||
|
const ch = label[i]
|
||||||
|
if (ch !== '-' && !DOMAIN_ALNUM.test(ch)) return false
|
||||||
|
}
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
/**
|
||||||
|
* Exactly four dot-separated groups of 1-3 digits. Anchored and bounded, so it
|
||||||
|
* fails fast without backtracking.
|
||||||
|
*/
|
||||||
|
const IPV4_SHAPE = /^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validates a public IPv4 host, matching the octet ranges and private-range
|
||||||
|
* rejections of the original regex: first octet 1-223, middle octets 0-255,
|
||||||
|
* last octet 1-254; rejects 10.x, 127.x, 169.254.x, 192.168.x, and
|
||||||
|
* 172.16-31.x.
|
||||||
|
*/
|
||||||
|
function isPublicIpv4Host(host: string): boolean {
|
||||||
|
const [a, b, c, d] = host.split('.').map(Number)
|
||||||
|
if (a < 1 || a > 223) return false
|
||||||
|
if (b < 0 || b > 255) return false
|
||||||
|
if (c < 0 || c > 255) return false
|
||||||
|
if (d < 1 || d > 254) return false
|
||||||
|
if (a === 10 || a === 127) return false
|
||||||
|
if (a === 169 && b === 254) return false
|
||||||
|
if (a === 192 && b === 168) return false
|
||||||
|
if (a === 172 && b >= 16 && b <= 31) return false
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validates a hostname as a domain name: at least two labels, every non-final
|
||||||
|
* label a valid domain label, and a final letters-only TLD of 2+ characters.
|
||||||
|
*/
|
||||||
|
function isValidDomainHost(host: string): boolean {
|
||||||
|
const labels = host.split('.')
|
||||||
|
if (labels.length < 2) return false
|
||||||
|
if (!DOMAIN_TLD.test(labels[labels.length - 1])) return false
|
||||||
|
for (let i = 0; i < labels.length - 1; i++) {
|
||||||
|
if (!isValidDomainLabel(labels[i])) return false
|
||||||
|
}
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Rewritten (was a single StackOverflow URL regex with nested quantifiers that
|
||||||
|
* CodeQL flagged as ReDoS-prone - see git history). This runs on every composer
|
||||||
|
* keystroke via suggestLinkCardUri, so it must be linear-time. Behavior is
|
||||||
|
* preserved: the string must start with http://, https://, ftp://, or a
|
||||||
|
* protocol-relative //; may carry userinfo; the host is a public IPv4 or a
|
||||||
|
* domain name; an optional 2-5 digit port; and an optional path/query/fragment
|
||||||
|
* with no whitespace. Every regex below is applied to a bounded slice with no
|
||||||
|
* nested quantifiers.
|
||||||
|
*
|
||||||
|
* Original StackOverflow attribution:
|
||||||
|
* https://stackoverflow.com/questions/8667070/javascript-regular-expression-to-validate-url
|
||||||
|
* question credit Muhammad Imran Tariq https://stackoverflow.com/users/420613/muhammad-imran-tariq
|
||||||
|
* answer credit Christian David https://stackoverflow.com/users/967956/christian-david
|
||||||
|
*/
|
||||||
|
function isValidUrlAndDomain(value: string) {
|
||||||
|
// The whole match forbids whitespace (\S / anchored), so bail early.
|
||||||
|
if (/\s/.test(value)) return false
|
||||||
|
|
||||||
|
// A leading // is required; the scheme, if present, must be http/https/ftp.
|
||||||
|
let rest: string
|
||||||
|
if (value.startsWith('http://')) rest = value.slice('http://'.length)
|
||||||
|
else if (value.startsWith('https://')) rest = value.slice('https://'.length)
|
||||||
|
else if (value.startsWith('ftp://')) rest = value.slice('ftp://'.length)
|
||||||
|
else if (value.startsWith('//')) rest = value.slice('//'.length)
|
||||||
|
else return false
|
||||||
|
|
||||||
|
// The authority ends at the first path/query/fragment delimiter.
|
||||||
|
const authorityEnd = rest.search(/[/?#]/)
|
||||||
|
const authority = authorityEnd === -1 ? rest : rest.slice(0, authorityEnd)
|
||||||
|
|
||||||
|
// Optional userinfo: greedy up to the last @ (as \S+(?::\S*)?@ would match).
|
||||||
|
let hostPort = authority
|
||||||
|
const at = authority.lastIndexOf('@')
|
||||||
|
if (at !== -1) {
|
||||||
|
if (at === 0) return false // userinfo must be non-empty
|
||||||
|
hostPort = authority.slice(at + 1)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Optional :port of 2-5 digits.
|
||||||
|
let host = hostPort
|
||||||
|
const colon = hostPort.lastIndexOf(':')
|
||||||
|
if (colon !== -1) {
|
||||||
|
const port = hostPort.slice(colon + 1)
|
||||||
|
if (!/^\d{2,5}$/.test(port)) return false
|
||||||
|
host = hostPort.slice(0, colon)
|
||||||
|
}
|
||||||
|
|
||||||
|
if (IPV4_SHAPE.test(host)) return isPublicIpv4Host(host)
|
||||||
|
return isValidDomainHost(host)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user