fix(security): validate CSS urls last, after every pass that moves text
Fifth bypass, and the same root cause as the first: sanitizeCSS validated, then kept rewriting. `<[^>]*>` deletes the span it matches, and `<">` takes a quote with it. So `--x:x<">;background:url(https://evil.example/p.gif);--y:x<">` was scanned with the url() safely inside a string, and the tag strip below then removed the quotes that made it so — shipping a live remote background with no warning. The file already carried the rule: "any pass that can join tokens has to happen before validation, not after." It has now been broken three separate times — by the HTML-comment strip (#1290), the control- character strip, and the tag strip. Rather than fix a third instance in place, the URL scan is now the LAST step, so what is validated is always the bytes that get served. All eight known bypass classes are pinned, together with the legitimate data: URI, quoted font stack and escaped selector that must survive untouched. Refs #1264
This commit is contained in:
@@ -260,6 +260,17 @@ describe('sanitizeCSS', () => {
|
|||||||
expect(asParsed(sanitized)).not.toContain('evil.example');
|
expect(asParsed(sanitized)).not.toContain('evil.example');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('blocks a url() the TAG strip would have un-quoted', () => {
|
||||||
|
// `<[^>]*>` deletes the span it matches, and `<">` takes a quote with it.
|
||||||
|
// Running that after the URL scan meant the scanner saw the url() safely
|
||||||
|
// inside a string and this pass then removed the quotes that made it so.
|
||||||
|
// URL validation has to be the last thing that looks at the text.
|
||||||
|
const { sanitized } = sanitizeCSS(
|
||||||
|
'--x:x<">;background:url(https://evil.example/p.gif);--y:x<">'
|
||||||
|
);
|
||||||
|
expect(asParsed(sanitized)).not.toContain('evil.example');
|
||||||
|
});
|
||||||
|
|
||||||
it('does not treat NBSP as CSS whitespace inside url()', () => {
|
it('does not treat NBSP as CSS whitespace inside url()', () => {
|
||||||
// JS `\s` matches U+00A0; CSS whitespace does not. Skipping it let the
|
// 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
|
// scanner read the following quote as a legitimate data: URI and swallow
|
||||||
|
|||||||
@@ -345,6 +345,19 @@ function sanitizeCSS(cssContent) {
|
|||||||
// below it then produced exactly the request the scan was there to
|
// below it then produced exactly the request the scan was there to
|
||||||
// prevent. Any pass that can join tokens has to happen before validation,
|
// prevent. Any pass that can join tokens has to happen before validation,
|
||||||
// not after.
|
// not after.
|
||||||
|
// Remove any remaining script-like content. This is the LAST pass that can
|
||||||
|
// move text, and so it must run before the URL scan, not after: it deletes
|
||||||
|
// the matched span, and a span like `<">` takes a quote with it. That is
|
||||||
|
// how `--x:x<">;background:url(https://evil.example/p.gif);--y:x<">` shipped
|
||||||
|
// a live background — the scanner saw the url() safely inside a string, and
|
||||||
|
// this line then removed the quotes that made it so.
|
||||||
|
sanitized = sanitized.replace(/<[^>]*>/g, '/* BLOCKED TAG */');
|
||||||
|
|
||||||
|
// URL validation runs LAST, deliberately. Every pass above rewrites the
|
||||||
|
// text, and each one that did so after this point has produced a bypass:
|
||||||
|
// the HTML-comment strip (#1290), the control-character strip, and the tag
|
||||||
|
// strip immediately above. Validating anything other than the final bytes
|
||||||
|
// means validating a string that is not the one that gets served.
|
||||||
const urlPass = stripDisallowedUrls(sanitized);
|
const urlPass = stripDisallowedUrls(sanitized);
|
||||||
if (urlPass.blocked > 0) {
|
if (urlPass.blocked > 0) {
|
||||||
warnings.push(
|
warnings.push(
|
||||||
@@ -354,9 +367,6 @@ function sanitizeCSS(cssContent) {
|
|||||||
sanitized = urlPass.sanitized;
|
sanitized = urlPass.sanitized;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Remove any remaining script-like content
|
|
||||||
sanitized = sanitized.replace(/<[^>]*>/g, '/* BLOCKED TAG */');
|
|
||||||
|
|
||||||
return { sanitized: sanitized.trim(), warnings };
|
return { sanitized: sanitized.trim(), warnings };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user