1442745 - Bleach sanitization bypass

Status: RESOLVED FIXED

(Webtools :: Bleach-security, defect)

Description

[Daniel Veditz [:dveditz]]

"stef" reported a sanitization bypass in bleach to the security mailing address. Since issues at https://github.com/mozilla/bleach are public filing this security issue here for now.

--- from stef's mail -------------

during an audit I found a way to circumvent the user input sanitizer bleach.

the following simple obfuscations allow the circumvention of bleach:

<a href="javas&#x09;cript:alert(1)">alert</a>
<a href="&#14;javascript:alert(1)">alert</a>

minimal testcase:

#!/usr/bin/env python3

import bleach
from bleach.sanitizer import Cleaner

cleaner = Cleaner(
    tags=bleach.sanitizer.ALLOWED_TAGS,
    attributes=bleach.sanitizer.ALLOWED_ATTRIBUTES,
    styles=bleach.sanitizer.ALLOWED_STYLES,
    protocols=bleach.sanitizer.ALLOWED_PROTOCOLS,
    strip=True,
    strip_comments=True,
)

print(cleaner.clean('<a href="javas&#x09;cript:alert(1)">alert</a>'))
print(cleaner.clean('<a href="&#14;javascript:alert(1)">alert</a>'))

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

I verified the issue. If you put the output of cleaner.clean() from the bug description in an html page, it prints the two alerts. That's with the Bleach 2.1.2 and html5lib 1.0.1--the current versions.

Interestingly, Bleach 2.0 does the right thing here. Thus this is broken specifically in Bleach 2.1, 2.1.1 and 2.1.2.

Greg and I poked around and we're pretty sure we know what's going on. A fix is tricky. I'm thinking maybe we can have a fix by end of Monday.

Comment 2

[April King [:April]]

It looks like we have a few dozen repositories that use bleach:

https://github.com/search?p=4&q=org%3Amozilla+%22import+bleach%22&type=Code

This seems to be a fairly serious issue, should I be opening up bugs for each project to make sure they are updated to the newest bleach?

Comment 3

[Greg G]

(In reply to April King [:April] from comment #2)
> It looks like we have a few dozen repositories that use bleach:
> 
> https://github.com/search?p=4&q=org%3Amozilla+%22import+bleach%22&type=Code
> 
> This seems to be a fairly serious issue, should I be opening up bugs for
> each project to make sure they are updated to the newest bleach?

We haven't patched this yet so updating won't help. Only the 2.1.X newer versions are broken.


For searching in github it looks like the following are affected:

mdn https://github.com/mozilla/kuma/blob/master/requirements/default.txt#L8

mozillians https://github.com/mozilla/mozillians/blob/8d057c2ac03cebea4a820d9cd898c3a74beb91c2/requirements/prod.txt#L105

standup https://github.com/mozilla/standup/blob/master/requirements.txt#L50

elmo https://github.com/mozilla/elmo/blob/a7430ec5ed9fa018d642b9a9d58fbd47d889a219/requirements/env.txt#L6

Comment 4

[Greg G]

also

pontoon https://github.com/mozilla/pontoon/blob/b76c53a78ef6d867a22aeb0f4c1970acd092e38b/requirements.txt#L123

these are from https://github.com/search?l=Text&q=org%3Amozilla+bleach&type=Code&utf8=%E2%9C%93

Comment 5

[Daniel Veditz [:dveditz]]

Adding the original reporter (z0jncr4lq)

Comment 6

[stf]

submitter here (stef): i know of one other significant project that is affected by this, but cannot disclose this project currently.

Comment 7

[Greg G]

Digging into how the above sites might be affected:

mdn doesn't have CSP, but trying to edit a page with the POC anchor tag sends me to a create new page permissions page  https://developer.mozilla.org/en-US/docs/new?slug=javas%26#x09;cript:alert(1)

mozillians has CSP that prevents unsafe-* srcs, only uses bleach.clean on announcements and funfacts (which require admin rights to created as far as I can tell), and markdown processed output (just the user profile where trying "[an example](javas	cript:alert(1)" linked back to the profile)

standup has CSP only allowing the 'self' script-src, and uses the cleaner with tags=[], so the entire anchor tag is stripped and we just get the tag contents

elmo / https://l10n.mozilla.org/ has CSP with default-src 'none' on the main page. Also, it only passes signoff comments through bleach (and django.utils.safestring.mark_safe but that doesn't sanitize it properly either)

pontoon has a CSP that prevents unsafe-inline


I don't see bleach in txt files for any repos in the mozilla-services github org https://github.com/search?q=org%3Amozilla-services+bleach&type=Code

Comment 8

[Greg G]

We aren't using bleach in the following github orgs:

mozilla-conduit https://github.com/search?q=org%3Amozilla-conduit+bleach&type=Code&utf8=%E2%9C%93
mozilla-platform-ops https://github.com/search?q=org%3Amozilla-platform-ops+bleach&type=Code&utf8=%E2%9C%93
mozilla-releng https://github.com/search?q=org%3Amozilla-releng+bleach&type=Code&utf8=%E2%9C%93
taskcluster https://github.com/search?q=org%3Ataskcluster+bleach&type=Code&utf8=%E2%9C%93

Comment 9

[Will Kahn-Greene [:willkg] ET needinfo? me]

Greg and I worked on this over the weekend. I think we can have a patch ready in the next day or two that covers this issue and a few related issues in the same section of code. Getting there...

Comment 10

[Greg G]

+jwhitlock as mdn security contact

Comment 11

[Will Kahn-Greene [:willkg] ET needinfo? me]

Attachment: 0001-Fix-url-sanitizing.patch

Preliminary patch addressing the security problem. This rewrites uri value sanitizing such that it converts character entities in uri values as well as improves the code that matches the uri schema to the list of allowed protocols.

This should be applied to master tip of Bleach code at: https://github.com/mozilla/bleach

Comment 12

[Will Kahn-Greene [:willkg] ET needinfo? me]

With the patch applied, the minimal test case prints this:

"""
<a>alert</a>
<a>alert</a>
"""

Comment 13

[Greg G]

Comment on attachment 8956089 [details] [diff] [review]
0001-Fix-url-sanitizing.patch

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

lgtm

::: bleach/sanitizer.py
@@ +555,5 @@
> +
> +        # Convert all character entities in the value
> +        new_value = convert_entities(value)
> +
> +        # Nix single quote, whitespace, and non-printable charcters

+1 for explaining these regexes

Comment 14

[Will Kahn-Greene [:willkg] ET needinfo? me]

After some scrutiny, I fixed that comment. Oops!

PR in Github: https://github.com/mozilla/bleach/pull/356

After that lands, I'll release 2.1.3 and send an email to the pyup and requires.io about how it's a security fix.

Comment 15

[April King [:April]]

Are there any concerns with backwards compatibility? If not, I'd be happy to help send PRs to all the Mozilla repos that are using affected versions of bleach.

Comment 16

[Greg G]

(In reply to April King [:April] from comment #15)
> Are there any concerns with backwards compatibility? If not, I'd be happy to
> help send PRs to all the Mozilla repos that are using affected versions of
> bleach.

I don't think so. If the repos in https://bugzilla.mozilla.org/show_bug.cgi?id=1442745#c7 won't get timely updates through pyup.io or requires.io then it'd be great if you could help send PRs.


stef: please let the other project using bleach you mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1442745#c6 know they can upgrade to 2.1.3 now

Comment 17

[Will Kahn-Greene [:willkg] ET needinfo? me]

I landed the fix. Then I:

1. Released Bleach v2.1.3: https://pypi.python.org/pypi/bleach/2.1.3
2. Updated ReadTheDocs: https://bleach.readthedocs.io/en/latest/changes.html#version-2-1-3-march-5th-2018
3. Sent an email to pyup folks and requires.io folks regarding it being a security release.

April: It's backwards-compatible with 2.1.2, so as long as they're using Bleach 2.1.x and not doing anything fancy (MDN does some fancy things, if I recall), upgrading should be a straight-forward version change.

Let me know if you hit any problems! I'll try to work through them as fast as I can.

Comment 18

[stf]

g-k: i contacted the project about the update.

Comment 19

[Greg G]

Thanks stef and :willkg.

hashes for the release:

pip hash -a sha256 bleach-2.1.3.tar.gz
bleach-2.1.3.tar.gz:
--hash=sha256:eb7386f632349d10d9ce9d4a838b134d4731571851149f9cc2c05a9a837a9a44

pip hash -a sha256 bleach-2.1.3-py2.py3-none-any.whl
bleach-2.1.3-py2.py3-none-any.whl:
--hash=sha256:b8fa79e91f96c2c2cd9fd1f9eda906efb1b88b483048978ba62fef680e962b34

PR for MDN: https://github.com/mozilla/kuma/pull/4697

Comment 20

[John Whitlock [:jwhitlock]]

Thanks g-k, taking a look. MDN is updating from 2.1.1 to 2.1.3. Tests pass, I wish I could trust them...

Comment 21

[Greg G]

Thanks :jwhitlock

PRs for the other repos:

* https://github.com/mozilla/mozillians/pull/2033
* https://github.com/mozilla/standup/pull/453
* https://github.com/mozilla/elmo/pull/30
* https://github.com/mozilla/pontoon/pull/880

CI passes, but it's hard to know if they cover everything.

Comment 22

[Will Kahn-Greene [:willkg] ET needinfo? me]

John: Feel free to tag me for the PR for MDN. I remember looking at the code before. I should be able to review it.

Comment 23

[John Whitlock [:jwhitlock]]

:g-k, thanks for mozilla/kuma#4697, and :willkg, thanks for the review on the follow-on commit. This change is deployed to staging and production for MDN.

Comment 24

[Greg G]

The above projects updated to 2.1.3 within ~24 hours of the PRs going out. Thanks all!

Comment 25

[stf]

original reporter here, why is this issue not public after 4 years and being fixed for long time.

Comment 26

[Frida K [:frida]]

Not sure, the issue has been disclosed in the changelog though: https://bleach.readthedocs.io/en/latest/changes.html#version-2-1-3-march-5th-2018. I can check with the team, we are in the process of improving our web bug bounty program, and issue disclosure is one of the points we will work on.