[HtmlSanitizer] Processing instructions misinterpreted as elements #54492
Labels
Comments
|
If i'm reading this correctly, we could add some checks here, to discard some DOMNode types:
Any others ? private function visitChildren(\DOMNode $domNode, Cursor $cursor): void
{
foreach ($domNode->childNodes ?? [] as $child) {
if ('#text' === $child->nodeName) {
// Add text directly for performance
$cursor->node->addChild(new TextNode($cursor->node, $child->nodeValue));
} elseif (!$child instanceof \DOMText) {
// Otherwise continue the visit recursively
// Ignore comments for security reasons (interpreted differently by browsers)
// -- ？Add node type checks here ？--
$this->visitNode($child, $cursor);
}
}
}/src/Symfony/Component/HtmlSanitizer/Visitor/DomVisitor.php Poke @tgalopin maybe ? |
|
As far as I can see, everything but processing instructions are already implicitly blocked because the other node types would start with a # symbol in their nodeName. PIs are the only cases where you can force a custom name (as shown in my OP example). |
|
Hey @nielsdos i tried to send a patch, could you check if that's OK for you ? :) |
|
Interesting, thanks for the report, and thanks @smnandre for the PR, I'm having a look! |
|
Thank you @tgalopin ! |
fabpot
added a commit
that referenced
this issue
Apr 8, 2024
This PR was merged into the 6.4 branch. Discussion ---------- [HtmlSanitizer] Ignore Processing Instructions | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #54492 | License | MIT Ignore Processing Instructions (as comments) to avoid mixing them with standard DOM nodes (see #54492) (targetting 6.4 as the component was released in 6.1)) Commits ------- 3582bdd [HtmlSanitizer] Ignore Processing Instructions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Symfony version(s) affected
7.0, perhaps lower too
Description
You're checking the node name in the node traversal, but not the node type.
So creating a processing instruction with the name of an allowed element results in a misinterpretation: the processing instruction will be considered as if it is an element. Fortunately, this has no security impact because we can only misinterpret nodes into an allowed element.
How to reproduce
Results in:
Possible Solution
Don't allow processing instructions, those aren't allowed by HTML5 anyway.
Additional Context
No response
The text was updated successfully, but these errors were encountered: