Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

security: fix bleach XSS security breach #854

Merged
merged 1 commit into from
Mar 18, 2020

Conversation

blankoworld
Copy link
Contributor

Tests were broken because of a security breach in bleach <=3.1.1.

  • Fixes bleach version to upgrade to bleach > 3.1.1

Co-Authored-by: Olivier DOSSMANN [email protected]

Why are you opening this PR?

A problem occurs in tests today:

PROGRAM: run-tests.sh
….
Checking installed package safety…
Notice: Ignoring CVE(s) 37752
38076: bleach <=3.1.1 resolved (3.1.1 installed)!
The ``bleach.clean`` behavior parsing embedded MathML and SVG content with RCDATA tags in Bleach versions <= 3.1.1 did not match browser behavior and could result in a mutation XSS.

Calls to ``bleach.clean`` with ``strip=False`` and ``math`` or ``svg`` tags and one or more of the RCDATA tags ``script``, ``noscript``, ``style``, ``noframes``, ``iframe``, ``noembed``, or ``xmp`` in the allowed tags whitelist were vulnerable to a mutation XSS.

This security issue was confirmed in Bleach version v3.1.1. Earlier versions are likely affected too.

How to test?

docker-compose down
pipenv --rm
pipenv run bootstrap
docker-compose up -d && ./docker/wait-for-services.sh
./run-tests.sh

Test shouldn't display the problem with bleach 3.1.1 breach.

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?
  • Extracted translations?

@blankoworld blankoworld requested review from BadrAly and reropag March 18, 2020 09:48
Pipfile Outdated
@@ -69,6 +69,8 @@ pycountry = ">=19.7.15"
xmltodict = "*"
# TODO: to be removed when the thumbnail will be refactored
angular-gettext-babel= ">=0.1"
# Avoid CVE 38076 on bleach <=3.1.1 ( asked by invenio-record-rest)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would write something like: dependency of invenio-record-reset to avoid confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I change it!

@blankoworld blankoworld force-pushed the doo-fix-bleach-version branch from 969660c to 367b562 Compare March 18, 2020 09:58
Tests were broken because of a security breach in bleach <=3.1.1.

* Fixes bleach version to upgrade to bleach > 3.1.1

Co-Authored-by: Olivier DOSSMANN <[email protected]>
@blankoworld blankoworld force-pushed the doo-fix-bleach-version branch from 367b562 to 4904c44 Compare March 18, 2020 10:00
Copy link
Contributor

@rerowep rerowep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it better to change the Pipfile or to add 38076 to pipenv check in run-tests.sh?

@blankoworld blankoworld merged commit 3cd35f9 into rero:dev Mar 18, 2020
@blankoworld blankoworld deleted the doo-fix-bleach-version branch March 30, 2020 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants