fix: escape fallback raw-content ancestor tags in processing instructions - #38
Closed
RubenPari wants to merge 1 commit into
Closed
fix: escape fallback raw-content ancestor tags in processing instructions#38RubenPari wants to merge 1 commit into
RubenPari wants to merge 1 commit into
Conversation
…ions serializeOne()'s PROCESSING_INSTRUCTION_NODE branch never called fallbackRawContentTags()/escapeMatchingClosingTag(), unlike the COMMENT_NODE branch fixed in fc7e40a. A literal "</noscript" (or </iframe, </noembed, </noframes) sequence in PI data survives escapeProcessingInstructionContent() (which only escapes '>'), so it reaches the browser's RAWTEXT tokenizer unescaped and closes the fallback element early, exposing following sibling markup as live DOM. Mirrors the fix already applied to the comment-node branch.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Member
|
THE CLA wasn't signed. We can't proceed with this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the remaining part of the regression reported in angular/angular#70050 / #70055, tracked as angular/angular#70146.
serializeOne()'sPROCESSING_INSTRUCTION_NODEbranch (case 7) never calledfallbackRawContentTags()/escapeMatchingClosingTag(), unlike theCOMMENT_NODEbranch (case 8) fixed in fc7e40a.escapeProcessingInstructionContent()only escapes>, so a literal</noscript(or</iframe,</noembed,</noframes) sequence in PI data passes through untouched and reaches the browser's RAWTEXT tokenizer unescaped, closing the fallback element early and exposing following sibling markup (e.g. an<img onerror>) as live DOM.This mirrors the fix already applied to the comment-node branch, escaping the ancestor's closing tag inside PI content the same way.
Test plan
noscriptProcessingInstructionAncestorClosingTagEscapedtotest/xss.js, verifying both the exact escaped serialization and that no alert fires when the resulting HTML is loaded in a real browser (puppeteer).npx mocha test/domino.js test/parsing.js test/readonly.js test/xss.js— all passing (excluding one pre-existing, unrelated timeout infallbackRawTextNestedRawTextElementsEscapeAncestorClosingTagthat reproduces identically without this change).