fix(security): use CSS whitespace, not JavaScript's, in the url() reader
Third bypass of this scanner found in one review pass, and the same
shape as the others: the lexer and a browser disagreeing about where a
token begins.
JavaScript's `\s` matches U+00A0; CSS whitespace is exactly space, tab,
LF, CR and FF. Skipping an NBSP as whitespace let the scanner read the
quote after it as a legitimate quoted data: URI and swallow a remote
url() inside that "string" —
.a{background:url(<NBSP>"data:image/png);background:url(https://evil…);--x:");}
came through untouched, with no warning, and survived re-sanitising. A
browser treats NBSP as an ordinary character, so that is an UNQUOTED
url-token ending at the first `)`, leaving the remote background live.
All three token readers now use an explicit CSS whitespace class.
Ordinary spacing around a data: URI still works, and is pinned.
Refs #1264
This commit is contained in:
@@ -260,6 +260,25 @@ describe('sanitizeCSS', () => {
|
|||||||
expect(asParsed(sanitized)).not.toContain('evil.example');
|
expect(asParsed(sanitized)).not.toContain('evil.example');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('does not treat NBSP as CSS whitespace inside url()', () => {
|
||||||
|
// JS `\s` matches U+00A0; CSS whitespace does not. Skipping it let the
|
||||||
|
// scanner read the following quote as a legitimate data: URI and swallow
|
||||||
|
// a remote url() inside the "string", while a browser sees an unquoted
|
||||||
|
// url-token ending at the first `)` and fetches the remote background.
|
||||||
|
const NBSP = '\u00a0';
|
||||||
|
const { sanitized } = sanitizeCSS(
|
||||||
|
`.a{background:url(${NBSP}"data:image/png);background:url(https://evil.example/p.gif);--x:");}`
|
||||||
|
);
|
||||||
|
expect(asParsed(sanitized)).not.toContain('evil.example');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still allows ordinary CSS whitespace around a data: URI', () => {
|
||||||
|
const spaced = sanitizeCSS('.a{background:url( "data:image/png;base64,iVBORw0KGgo=" )}');
|
||||||
|
expect(spaced.sanitized).toContain('data:image/png');
|
||||||
|
const tabbed = sanitizeCSS('.a{background:url(\t"data:image/png;base64,iVBORw0KGgo=")}');
|
||||||
|
expect(tabbed.sanitized).toContain('data:image/png');
|
||||||
|
});
|
||||||
|
|
||||||
it('does not let an unterminated quote hide everything after it', () => {
|
it('does not let an unterminated quote hide everything after it', () => {
|
||||||
// An unclosed quote is a parse error. Trusting it meant one stray
|
// An unclosed quote is a parse error. Trusting it meant one stray
|
||||||
// apostrophe disabled scanning for the remainder of the stylesheet, so
|
// apostrophe disabled scanning for the remainder of the stylesheet, so
|
||||||
|
|||||||
@@ -177,7 +177,7 @@ function stripDisallowedUrls(css) {
|
|||||||
const ident = readIdentifier(input, i);
|
const ident = readIdentifier(input, i);
|
||||||
if (ident.end > i && decodeCssEscapes(ident.raw).toLowerCase() === 'url') {
|
if (ident.end > i && decodeCssEscapes(ident.raw).toLowerCase() === 'url') {
|
||||||
let j = ident.end;
|
let j = ident.end;
|
||||||
while (j < input.length && /\s/.test(input[j])) j += 1;
|
while (j < input.length && CSS_WS.test(input[j])) j += 1;
|
||||||
if (input[j] === '(') {
|
if (input[j] === '(') {
|
||||||
const token = readUrlToken(input, j);
|
const token = readUrlToken(input, j);
|
||||||
if (token) {
|
if (token) {
|
||||||
@@ -238,6 +238,15 @@ function matchEscape(input, start) {
|
|||||||
* escaped Tailwind selector) is emitted byte-identical rather than silently
|
* escaped Tailwind selector) is emitted byte-identical rather than silently
|
||||||
* rewritten to `.w-1/2`, which is a different selector.
|
* rewritten to `.w-1/2`, which is a different selector.
|
||||||
*/
|
*/
|
||||||
|
// CSS whitespace is exactly space, tab, LF, CR and FF. JavaScript's `\s`
|
||||||
|
// is NOT the same set — it also matches NBSP and the other Unicode spaces,
|
||||||
|
// and that difference was a bypass: in `url(\u00a0"data:image/png);...")`
|
||||||
|
// the scanner skipped the NBSP as whitespace and read the following quote as
|
||||||
|
// a legitimate quoted data: URI, swallowing a remote url() inside it. A
|
||||||
|
// browser treats NBSP as an ordinary character, making that an UNQUOTED
|
||||||
|
// url-token that ends at the first `)` — leaving the remote background live.
|
||||||
|
const CSS_WS = /[ \t\n\r\f]/;
|
||||||
|
|
||||||
function readIdentifier(input, start) {
|
function readIdentifier(input, start) {
|
||||||
let j = start;
|
let j = start;
|
||||||
let raw = '';
|
let raw = '';
|
||||||
@@ -257,7 +266,7 @@ function readIdentifier(input, start) {
|
|||||||
function readUrlToken(input, openParen) {
|
function readUrlToken(input, openParen) {
|
||||||
let j = openParen + 1;
|
let j = openParen + 1;
|
||||||
let target = '';
|
let target = '';
|
||||||
while (j < input.length && /\s/.test(input[j])) j += 1;
|
while (j < input.length && CSS_WS.test(input[j])) j += 1;
|
||||||
|
|
||||||
if (input[j] === '"' || input[j] === '\'') {
|
if (input[j] === '"' || input[j] === '\'') {
|
||||||
// Quoted: the quote closes the value, so ")" inside it is content.
|
// Quoted: the quote closes the value, so ")" inside it is content.
|
||||||
@@ -278,7 +287,7 @@ function readUrlToken(input, openParen) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
while (j < input.length && /\s/.test(input[j])) j += 1;
|
while (j < input.length && CSS_WS.test(input[j])) j += 1;
|
||||||
// Unterminated url( — malformed. Leave it alone rather than swallowing the
|
// Unterminated url( — malformed. Leave it alone rather than swallowing the
|
||||||
// remainder of the stylesheet.
|
// remainder of the stylesheet.
|
||||||
if (input[j] !== ')') return null;
|
if (input[j] !== ')') return null;
|
||||||
|
|||||||
Reference in New Issue
Block a user