Skip to content
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.

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

[SECURITY] Mitigate XSS in CDATA and HTML raw text elements #106

Merged
merged 2 commits into from
Dec 13, 2022

Conversation

ohader
Copy link
Member

@ohader ohader commented Dec 13, 2022

Security-References: CVE-2022-23499

Due to a parsing issue in upstream package `masterminds/html5`,
malicious markup used in a sequence with special HTML CDATA sections
cannot be filtered and sanitized.

This change mitigates the parsing problem by converting all
CDATA section contents to DOM text nodes like show below:

- from `<![CDATA[<any><span data-value="value"></any>*/]]>`
+ to   `&lt;any&gt;&lt;span data-value="value"&gt;&lt;/any&gt;*/`

In case an individual builder us used, which does not inherit behavior
declarations from `TYPO3\HtmlSanitizer\Builder\CommonBuilder`, those
implementations need to be adjusted manually.
Upstream package `masterminds/html5` provides HTML raw text elements
(`script`, `style`, `noframes`, `noembed` and (void element) `iframe`)
as `DOMText` nodes. Since those text nodes have not been processed by
`typo3/html-sanitizer`, sanitization was not applied to mentioned
HTML tag names.

None of them were defined in the default builder configuration,
that's why only custom behaviors, using one of those tag names,
were vulnerable to cross-site scripting.

In case any of those tags shall be processed as raw text
(for instance this would be reasonable for `script type="ld+json"`),
the new `Behavior\Tag::ALLOW_INSECURE_RAW_TEXT` flag needs to be
used explicitly.
@ohader ohader merged commit 385741b into TYPO3:v1.5 Dec 13, 2022
@ohader ohader deleted the CVE-2022-23499-v1.5 branch December 13, 2022 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant