Closed
Bug 1442745
Opened 7 years ago
Closed 7 years ago
Bleach sanitization bypass
Categories
(Webtools :: Bleach-security, defect)
Webtools
Bleach-security
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: dveditz, Assigned: willkg)
Details
(Keywords: reporter-external, sec-high, wsec-xss)
Attachments
(1 file)
10.28 KB,
patch
|
u581815
:
review+
|
Details | Diff | Splinter Review |
"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	cript:alert(1)">alert</a>
<a href="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	cript:alert(1)">alert</a>'))
print(cleaner.clean('<a href="javascript:alert(1)">alert</a>'))
Flags: sec-bounty?
Reporter | ||
Updated•7 years ago
|
Group: cloud-services-security → webtools-security
Component: Security → Bleach-security
Product: Cloud Services → Webtools
QA Contact: jvehent
Version: unspecified → other
Assignee | ||
Comment 1•7 years ago
|
||
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.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → willkg
Status: NEW → ASSIGNED
Comment 2•7 years ago
|
||
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?
(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
Reporter | ||
Comment 5•7 years ago
|
||
Adding the original reporter (z0jncr4lq)
submitter here (stef): i know of one other significant project that is affected by this, but cannot disclose this project currently.
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
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
Assignee | ||
Comment 9•7 years ago
|
||
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•7 years ago
|
||
+jwhitlock as mdn security contact
Assignee | ||
Comment 11•7 years ago
|
||
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
Attachment #8956089 -
Flags: review?(gguthe)
Assignee | ||
Comment 12•7 years ago
|
||
With the patch applied, the minimal test case prints this:
"""
<a>alert</a>
<a>alert</a>
"""
Comment 13•7 years ago
|
||
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
Attachment #8956089 -
Flags: review?(gguthe) → review+
Assignee | ||
Comment 14•7 years ago
|
||
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•7 years ago
|
||
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•7 years ago
|
||
(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
Assignee | ||
Comment 17•7 years ago
|
||
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•7 years ago
|
||
g-k: i contacted the project about the update.
Comment 19•7 years ago
|
||
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•7 years ago
|
||
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•7 years ago
|
||
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.
Assignee | ||
Comment 22•7 years ago
|
||
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•7 years ago
|
||
: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•7 years ago
|
||
The above projects updated to 2.1.3 within ~24 hours of the PRs going out. Thanks all!
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Flags: sec-bounty? → sec-bounty+
Updated•3 years ago
|
Comment 25•3 years ago
|
||
original reporter here, why is this issue not public after 4 years and being fixed for long time.
Comment 26•3 years ago
|
||
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.
Updated•3 years ago
|
Group: webtools-security
Updated•6 months ago
|
Keywords: reporter-external
You need to log in
before you can comment on or make changes to this bug.
Description
•