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.

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

Jump to bottom

Requesting more details on GHSA-fqx8-v33p-4qcc (CVE-2022-23638) #71

Closed
ohader opened this issue Feb 14, 2022 · 1 comment
Closed

Requesting more details on GHSA-fqx8-v33p-4qcc (CVE-2022-23638) #71

ohader opened this issue Feb 14, 2022 · 1 comment

Comments

@ohader
Copy link
Contributor

ohader commented Feb 14, 2022 •
edited
Loading

It seems tag 0.15.0 addressed a security vulnerability, see corresponding advisory GHSA-fqx8-v33p-4qcc (CVE-2022-23638)

Corresponding commit at 17e12ba contains a new test case tests/data/htmlTest.svg.

Invoked as svg.svg in browser, mime-type image/svg+xml

<?xml version="1.0" encoding="UTF-8"?>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="-1 -1 2 2">
    <!--><img src onerror=alert(1)><!-->
    <?x ><img src onerror=alert(1)><?x?>
    <p/><![CDATA[ ><img src onerror=alert(1)> ]]>
    <font face=""/><![CDATA[ ><img src onerror=alert(1)> ]]>
</svg>

→ no problem since <img> is not a SVG element
-> not a vulnerability

Invoked as svg.html in browser, mime-type text/htm

<html>
<body>
<div>
    <svg xmlns="http://www.w3.org/2000/svg" viewBox="-1 -1 2 2">
        <!--><img src onerror=alert(1)><!-->
        <?x ><img src onerror=alert(1)><?x?>
        <p/><![CDATA[ ><img src onerror=alert(1)> ]]>
        <font face=""/><![CDATA[ ><img src onerror=alert(1)> ]]>
    </svg>
</div>
</body>
</html>

→ valid concern, since HTML is used in inline SVG
→ scripts are executed in browser
→ cross-site scripting vulnerability

Conclusion & Post-review

Request

  • @darylldoyle please report back, whether these assumptions are correct (it affects only SVG used inline in some HTML-context)
  • consider updating advisory details of GHSA-fqx8-v33p-4qcc - I can support with that task
@ohader ohader changed the title Requesting more details on GHSA-fqx8-v33p-4qcc Requesting more details on GHSA-fqx8-v33p-4qcc (CVE-2022-23638) Feb 14, 2022
@darylldoyle
Copy link
Owner

@ohader you're correct. The concern here was that any of the following tags will cause the HTML parser to break out of foreign content and back into HTML.

A start tag whose tag name is one of: "b", "big", "blockquote", "body", "br", "center", "code", "dd", "div", "dl", "dt", "em", "embed", "h1", "h2", "h3", "h4", "h5", "h6", "head", "hr", "i", "img", "li", "listing", "menu", "meta", "nobr", "ol", "p", "pre", "ruby", "s", "small", "span", "strong", "strike", "sub", "sup", "table", "tt", "u", "ul", "var"
A start tag whose tag name is "font", if the token has any attributes named "color", "face", or "size"
An end tag whose tag name is "br", "p"

As per: https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inforeign

Specifically this line was causing me issues:

<p/><![CDATA[ ><img src onerror=alert(1)> ]]>

DOMDocument was picking the img tag up as a DOMComment node with the following content, which led me to remove comments and CDATA so that this got stripped.

><img src onerror=alert(1)>

I'm happy to consider adding CDATA back in, but we need to be sure that there's no easy bypass. Maybe we should consider extending the tests in this area.

reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this issue Feb 21, 2022
@ohader
[TASK] Upgrade enshrined/svg-sanitize to ^0.15.4
d6e3d9c 
Recent release of enshrined/svg-sanitize addressed a XSS vulnerability.

The main purpose of having this library in TYPO3 is to protect against
user submitted images that contains markup - which is possible with
SVG files. In most TYPO3 scenarios these files would be stored in
https://example.org/fileadmin/evil.svg and can be fetched directly.

However, recent update for CVE-2022-23638 of the svg-sanitizer library
seems to address the usage of inline SVG, used in an embedded HTML
context, see darylldoyle/svg-sanitizer#71

Resolves: #96901
Releases: main, 11.5, 10.4
Change-Id: Iacbaf4b9c9725dee9c12df3646fc1131b7ed93ed
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/73516
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this issue Feb 21, 2022
@ohader
[TASK] Upgrade enshrined/svg-sanitize to ^0.15.4
9940def 
Recent release of enshrined/svg-sanitize addressed a XSS vulnerability.

The main purpose of having this library in TYPO3 is to protect against
user submitted images that contains markup - which is possible with
SVG files. In most TYPO3 scenarios these files would be stored in
https://example.org/fileadmin/evil.svg and can be fetched directly.

However, recent update for CVE-2022-23638 of the svg-sanitizer library
seems to address the usage of inline SVG, used in an embedded HTML
context, see darylldoyle/svg-sanitizer#71

Resolves: #96901
Releases: main, 11.5, 10.4
Change-Id: Iacbaf4b9c9725dee9c12df3646fc1131b7ed93ed
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/73628
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this issue Feb 21, 2022
@ohader
[TASK] Upgrade enshrined/svg-sanitize to ^0.15.4
5907ff3 
Recent release of enshrined/svg-sanitize addressed a XSS vulnerability.

The main purpose of having this library in TYPO3 is to protect against
user submitted images that contains markup - which is possible with
SVG files. In most TYPO3 scenarios these files would be stored in
https://example.org/fileadmin/evil.svg and can be fetched directly.

However, recent update for CVE-2022-23638 of the svg-sanitizer library
seems to address the usage of inline SVG, used in an embedded HTML
context, see darylldoyle/svg-sanitizer#71

Resolves: #96901
Releases: main, 11.5, 10.4
Change-Id: Iacbaf4b9c9725dee9c12df3646fc1131b7ed93ed
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/73627
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this issue Feb 21, 2022
@ohader
[TASK] Upgrade enshrined/svg-sanitize to ^0.15.4
9e9f238 
Recent release of enshrined/svg-sanitize addressed a XSS vulnerability.

The main purpose of having this library in TYPO3 is to protect against
user submitted images that contains markup - which is possible with
SVG files. In most TYPO3 scenarios these files would be stored in
https://example.org/fileadmin/evil.svg and can be fetched directly.

However, recent update for CVE-2022-23638 of the svg-sanitizer library
seems to address the usage of inline SVG, used in an embedded HTML
context, see darylldoyle/svg-sanitizer#71

Resolves: #96901
Releases: main, 11.5, 10.4
Change-Id: Iacbaf4b9c9725dee9c12df3646fc1131b7ed93ed
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/73516
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this issue Feb 21, 2022
@ohader
[TASK] Upgrade enshrined/svg-sanitize to ^0.15.4
300edd8 
Recent release of enshrined/svg-sanitize addressed a XSS vulnerability.

The main purpose of having this library in TYPO3 is to protect against
user submitted images that contains markup - which is possible with
SVG files. In most TYPO3 scenarios these files would be stored in
https://example.org/fileadmin/evil.svg and can be fetched directly.

However, recent update for CVE-2022-23638 of the svg-sanitizer library
seems to address the usage of inline SVG, used in an embedded HTML
context, see darylldoyle/svg-sanitizer#71

Resolves: #96901
Releases: main, 11.5, 10.4
Change-Id: Iacbaf4b9c9725dee9c12df3646fc1131b7ed93ed
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/73628
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this issue Feb 21, 2022
@ohader
[TASK] Upgrade enshrined/svg-sanitize to ^0.15.4
f206942 
Recent release of enshrined/svg-sanitize addressed a XSS vulnerability.

The main purpose of having this library in TYPO3 is to protect against
user submitted images that contains markup - which is possible with
SVG files. In most TYPO3 scenarios these files would be stored in
https://example.org/fileadmin/evil.svg and can be fetched directly.

However, recent update for CVE-2022-23638 of the svg-sanitizer library
seems to address the usage of inline SVG, used in an embedded HTML
context, see darylldoyle/svg-sanitizer#71

Resolves: #96901
Releases: main, 11.5, 10.4
Change-Id: Iacbaf4b9c9725dee9c12df3646fc1131b7ed93ed
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/73627
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
@ohader ohader mentioned this issue Mar 21, 2023
Closed
@ohader ohader closed this as completed Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ohader @darylldoyle