Skip to content

fix(core): reparse current markup in sanitizer stabilization loop #69836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
arshsmith1 wants to merge 1 commit into
base: main
Choose a base branch
from
+11 −4
Open

fix(core): reparse current markup in sanitizer stabilization loop #69836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/core/src/sanitization/html_sanitizer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ export function _sanitizeHtml(defaultDoc: any, unsafeHtmlInput: string): Trusted
inertBodyHelper = inertBodyHelper || getInertBodyHelper(defaultDoc);
// Make sure unsafeHtml is actually a string (TypeScript types are not enforced at runtime).
let unsafeHtml = unsafeHtmlInput ? String(unsafeHtmlInput) : '';
inertBodyElement = inertBodyHelper.getInertBodyElement(unsafeHtml);

// mXSS protection. Repeatedly parse the document to make sure it stabilizes, so that a browser
// trying to auto-correct incorrect HTML cannot cause formerly inert HTML to become dangerous.
Expand All @@ -321,8 +320,8 @@ export function _sanitizeHtml(defaultDoc: any, unsafeHtmlInput: string): Trusted
mXSSAttempts--;

unsafeHtml = parsedHtml;
parsedHtml = inertBodyElement!.innerHTML;
inertBodyElement = inertBodyHelper.getInertBodyElement(unsafeHtml);
parsedHtml = inertBodyElement!.innerHTML;
} while (unsafeHtml !== parsedHtml);

const sanitizer = new SanitizingHtmlSerializer();
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/sanitization/inert_body.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ class DOMParserHelper implements InertBodyHelper {
// e.g. leading whitespace is maintained and tags like `<meta>` do not get hoisted to the
// `<head>` tag. Note that the `<body>` tag is closed implicitly to prevent unclosed tags
// in `html` from consuming the otherwise explicit `</body>` tag.
html = '<body><remove></remove>' + html;
const prefixedHtml = '<body><remove></remove>' + html;
try {
const body = new window.DOMParser().parseFromString(
trustedHTMLFromString(html) as string,
trustedHTMLFromString(prefixedHtml) as string,
'text/html',
).body as HTMLBodyElement;
if (body === null) {
Expand Down
8 changes: 8 additions & 0 deletions packages/core/test/sanitization/html_sanitizer_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,14 @@ describe('HTML sanitizer', () => {
});

if (isDOMParserAvailable()) {
it('should reject markup that never stabilizes when reparsed', () => {
// The tokenizer never leaves the PLAINTEXT state, so every reparse appends another
// literal `</plaintext>` and the markup has no fixed point.
expect(() => sanitizeHtml(defaultDoc, '<plaintext>a')).toThrowError(
/Failed to sanitize html because the input is unstable/,
);
});

it('should work even if DOMParser returns a null body', () => {
// Simulate `DOMParser.parseFromString()` returning a null body.
// See https://github.com/angular/angular/issues/39834
Expand Down
Loading