1326384 - Stored drive-by XSS with IFRAME after <script>.

Status: RESOLVED FIXED

(developer.mozilla.org :: Security, defect)

Description

[Tooru Fujisawa [:arai]]

Stored XSS that is drive-by and also can steal cookie, or run arbitrary script on MDN page.

Steps to Reproduce:
  1. create a page
  2. choose "Source" for editing
  3. enter the following code

<script><iframe src="data:text/html;base64,PHNjcmlwdD52YXIgZD13aW5kb3cucGFyZW50LmRvY3VtZW50LHM9ZC5jcmVhdGVFbGVtZW50KCJzY3JpcHQiKTtzLmFwcGVuZENoaWxkKGQuY3JlYXRlVGV4dE5vZGUoImFsZXJ0KGRvY3VtZW50LmNvb2tpZSkiKSk7ZC5ib2R5LmFwcGVuZENoaWxkKHMpOzwvc2NyaXB0Pg=="></iframe>

  4. publish
  5. open the published page

the content of data URI is following, that gets cookie:
<script>var d=window.parent.document,s=d.createElement("script");s.appendChild(d.createTextNode("alert(document.cookie)"));d.body.appendChild(s);</script>

Actual result:
  script in the IFRAME is executed, the script adds SCRIPT element on parent frame, and cookie, including csrftoken is stolen.

Expected result:
  The src attribute is filtered and nothing happens


This issue doesn't happens when there's no <script> at the beginning.
<script> tag itself is sanitized properly, but the content isn't handled correctly.

Comment 1

[Tooru Fujisawa [:arai]]

The issue is that filterIframeHosts is applied before bleach.

https://github.com/mozilla/kuma/blob/d087906b7cc69aae450d101b99075298d031844c/kuma/wiki/managers.py#L29-L44

the issue can be demonstrated with the following code:

[code]

import html5lib

def test(src):
    print src
    tree = html5lib.treebuilders.getTreeBuilder("etree")
    parser = html5lib.HTMLParser(tree=tree, namespaceHTMLElements=False)
    doc = parser.parseFragment(src)
    stream = html5lib.treewalkers.getTreeWalker("etree")(doc)
    for x in stream:
        print x
    print

test('<script><iframe src="hello"></iframe>')
test('<iframe src="hello"></iframe>')

[output]

<script><iframe src="hello"></iframe>
{u'namespace': None, u'type': u'StartTag', u'name': u'script', u'data': OrderedDict()}
{u'data': u'<iframe src="hello"></iframe>', u'type': u'Characters'}
{u'namespace': None, u'type': u'EndTag', u'name': u'script'}

<iframe src="hello"></iframe>
{u'namespace': None, u'type': u'StartTag', u'name': u'iframe', u'data': OrderedDict([((None, u'src'), u'hello')])}
{u'namespace': None, u'type': u'EndTag', u'name': u'iframe'}


If there's "<script>", the text after that is yielded with 'type': 'Characters',
and any kind of filtering doesn't work.
"<script>" should be sanitized to "&lt;script&gt;" before doing filtering based on parse result.
there could be some more instances other than "<script>" that changes the parser state.

Comment 2

[Tooru Fujisawa [:arai]]

this issue happens also with <style>, that also changes the parser state and also be sanitized by bleach.

<textarea> also changes the parser state, but it's not sanitized and the issue doesn't happen with it.

Comment 3

[Tooru Fujisawa [:arai]]

filterAHrefProtocols is also affected.
the filtering doesn't work on following code:

<script><a href="data:text/html;base64,PHNjcmlwdD5hbGVydChkb2N1bWVudC5kb21haW4pPC9zY3JpcHQ+">click me</a>

but bug 1325125 fixed client-side check and the link doesn't work even if class="no-track" is added.

Comment 4

[Tooru Fujisawa [:arai]]

so, the question is, how do we handle security-sensitive change in github?
maybe better posting patch here?

Comment 5

[John Whitlock [:jwhitlock]]

Here's the MDN staff process:

1) Open a branch in a GitHub private repo.
2) Ask one or more relevant Mozilla staff members to review it.
3) Manually merge the branch to master, deploy to stage and production.
4) Mark the bug as fixed, remove the security sensitive flag.

If you are submitting a patch, you can attach it here, or create a private GitHub repo (I think it costs $7 / month).

I intend to start on this and the other security bugs when I return from vacation tomorrow.

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Filter iframe src and a href after bleach.clean.

thanks :)

fwiw, here's WIP patch.
it at least passes test_content.py
haven't yet done other tests, since running tests takes so long time here on docker :P

Comment 7

[John Whitlock [:jwhitlock]]

All tests pass locally for me. I'm looking closely to understand the issue and make sure the patch fixes it. 

Docker tests are faster this way:

$ make bash
$ py.test --reuse-db --create-db kuma/wiki/tests/test_content.py  # Slow, but required for first run
$ py.test --reuse-db kuma/wiki/tests/test_content.py              # Later runs are fast

I'm working on making this the default, but I think a py.test upgrade is needed first, to detect that the database isn't created yet:

https://github.com/mozilla/kuma/pull/4040

Comment 8

[John Whitlock [:jwhitlock]]

This code works well, and I believe it is the right thing to bleach the content before applying the remaining filters.

:willkg pointed out that Bleach 1.5.0 includes an improved protocol filter that handles this issue:

http://bleach.readthedocs.io/en/latest/changes.html#version-1-5-november-4th-2016

I'm adding a Bleach upgrade as a follow-on commit, and dropping the existing filter.  I'm aiming to merge tomorrow, near the start of my day.

Comment 9

[John Whitlock [:jwhitlock]]

Attachment: 0002-bug-1326384-Upgrade-bleach-add-ALLOWED_PROTOCOLS.patch

Comment 10

[John Whitlock [:jwhitlock]]

Attachment: 0003-bug-1326384-Deprecate-a-href-.-filter.patch

Comment 11

[John Whitlock [:jwhitlock]]

Fix pushed to stage and production. Relevant commits:

https://github.com/mozilla/kuma/commit/020b9ccdc289de85b75f629353fb677dcbcc21cb - 0001
https://github.com/mozilla/kuma/commit/729fe608b4a0bb1a67982a63e54ba29470833f67 - 0002
https://github.com/mozilla/kuma/commit/5f602dc5937966d326c31c3355d80553bc164945 - 0003
https://github.com/mozilla/kuma/commit/ec487dac8abd6cfc5ae153b81c18e5a10caf70a2

Comment 12

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

MDN is not an eligible bounty property.

Comment 13

[John Whitlock [:jwhitlock]]

Since mitigations are deployed, removing security flag.

Comment 14

[[github robot]]

Commits pushed to master at https://github.com/mozilla/kuma

https://github.com/mozilla/kuma/commit/d48ca4af5d42c9a94b1967dcdcc3f945ae8ae338
bug 1326384: Remove deprecated protocol filtering

Protocol filtering has been handled by Bleach since 1.5 (Nov 2016).
AHrefProtocolFilter and the associated code hasn't been used since
January 2017.

https://github.com/mozilla/kuma/commit/760d3fe2ce20f1e5d5c07baa3827809e80968795
Merge pull request #4492 from jwhitlock/remove-protocol-filter-1326384

bug 1326384: Remove deprecated protocol filtering