Closed Bug 1442745 Opened 4 years ago Closed 4 years ago
Bleach sanitization bypass
Group: cloud-services-security → webtools-security
Component: Security → Bleach-security
Product: Cloud Services → Webtools
QA Contact: jvehent
Version: unspecified → other
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.
4 years ago
Assignee: nobody → willkg
Status: NEW → ASSIGNED
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
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
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...
+jwhitlock as mdn security contact
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)
With the patch applied, the minimal test case prints this: """ <a>alert</a> <a>alert</a> """
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+
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.
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.
(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
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.
g-k: i contacted the project about the update.
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
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...
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.
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.
: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.
The above projects updated to 2.1.3 within ~24 hours of the PRs going out. Thanks all!
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.