Merge pull request from GHSA-vv2x-vrpj-qqpq

Fix ghsa vv2x vrpj qqpq
mozilla · Feb 1, 2021 · 79b7a3c · LilyFoote · Feb 18, 2021 · g-k
2 parents c045a8b + 842fcb4
commit 79b7a3c
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 6 deletions.
diff --git a/CHANGES b/CHANGES
@@ -1,6 +1,25 @@
 Bleach changes
 ==============
 
+Version 3.3.0 (February 1st, 2021)
+----------------------------------
+
+**Backwards incompatible changes**
+
+* clean escapes HTML comments even when strip_comments=False
+
+**Security fixes**
+
+* Fix bug 1621692 / GHSA-m6xf-fq7q-8743. See the advisory for details.
+
+**Features**
+
+None
+
+**Bug fixes**
+
+None
+
 Version 3.2.3 (January 26th, 2021)
 ----------------------------------
 

diff --git a/SECURITY.md b/SECURITY.md
@@ -7,8 +7,8 @@ currently being supported with security updates.
 
 | Version | Supported          |
 | ------- | ------------------ |
-| 3.2.x   | :white_check_mark: |
-| < 3.1   | :x:                |
+| 3.3.x   | :white_check_mark: |
+| < 3.2   | :x:                |
 
 ## Reporting a Vulnerability
 

diff --git a/bleach/__init__.py b/bleach/__init__.py
@@ -18,9 +18,9 @@
 
 
 # yyyymmdd
-__releasedate__ = "20210126"
+__releasedate__ = "20210201"
 # x.y.z or x.y.z.dev0 -- semver
-__version__ = "3.2.3"
+__version__ = "3.3.0"
 VERSION = packaging.version.Version(__version__)
 
 

diff --git a/bleach/html5lib_shim.py b/bleach/html5lib_shim.py
@@ -48,6 +48,7 @@
     HTMLInputStream,
 )  # noqa: E402 module level import not at top of file
 from bleach._vendor.html5lib.serializer import (
+    escape,
     HTMLSerializer,
 )  # noqa: E402 module level import not at top of file
 from bleach._vendor.html5lib._tokenizer import (

diff --git a/bleach/sanitizer.py b/bleach/sanitizer.py
@@ -371,6 +371,10 @@ def sanitize_token(self, token):
 
         elif token_type == "Comment":
             if not self.strip_html_comments:
+                # call lxml.sax.saxutils to escape &, <, and > in addition to " and '
+                token["data"] = html5lib_shim.escape(
+                    token["data"], entities={'"': "&quot;", "'": "&#x27;"}
+                )
                 return token
             else:
                 return None

diff --git a/tests/test_clean.py b/tests/test_clean.py
@@ -739,6 +739,53 @@ def test_namespace_rc_data_element_strip_false(
     )
 
 
+@pytest.mark.parametrize(
+    "namespace_tag, end_tag, data, expected",
+    [
+        (
+            "math",
+            "p",
+            "<math></p><style><!--</style><img src/onerror=alert(1)>",
+            "<math><p></p><style><!--&lt;/style&gt;&lt;img src/onerror=alert(1)&gt;--></style></math>",
+        ),
+        (
+            "math",
+            "br",
+            "<math></br><style><!--</style><img src/onerror=alert(1)>",
+            "<math><br><style><!--&lt;/style&gt;&lt;img src/onerror=alert(1)&gt;--></style></math>",
+        ),
+        (
+            "svg",
+            "p",
+            "<svg></p><style><!--</style><img src/onerror=alert(1)>",
+            "<svg><p></p><style><!--&lt;/style&gt;&lt;img src/onerror=alert(1)&gt;--></style></svg>",
+        ),
+        (
+            "svg",
+            "br",
+            "<svg></br><style><!--</style><img src/onerror=alert(1)>",
+            "<svg><br><style><!--&lt;/style&gt;&lt;img src/onerror=alert(1)&gt;--></style></svg>",
+        ),
+    ],
+)
+def test_html_comments_escaped(namespace_tag, end_tag, data, expected):
+    # refs: bug 1689399 / GHSA-vv2x-vrpj-qqpq
+    #
+    # p and br can be just an end tag (e.g. </p> == <p></p>)
+    #
+    # In browsers:
+    #
+    # * img and other tags break out of the svg or math namespace (e.g. <svg><img></svg> == <svg><img></svg>)
+    # * style does not (e.g. <svg><style></svg> == <svg><style></style></svg>)
+    # * the breaking tag ejects trailing elements (e.g. <svg><img><style></style></svg> == <svg></svg><img><style></style>)
+    #
+    # the ejected elements can trigger XSS
+    assert (
+        clean(data, tags=[namespace_tag, end_tag, "style"], strip_comments=False)
+        == expected
+    )
+
+
 def get_ids_and_tests():
     """Retrieves regression tests from data/ directory
 

diff --git a/tests_website/index.html b/tests_website/index.html
@@ -2,7 +2,7 @@
 <html>
     <head>
         <meta charset="UTF-8">
-        <title>Python Bleach 3.2.3</title>
+        <title>Python Bleach 3.3.0</title>
         <style>
          textarea, iframe {
              width: 95%;
@@ -20,7 +20,7 @@
         </style>
     </head>
     <body>
-        <h2>Python Bleach 3.2.3</h2>
+        <h2>Python Bleach 3.3.0</h2>
         <p>
             <a href="http://badge.fury.io/py/bleach"><img style="max-width:100%;" alt="pypi version" src="https://badge.fury.io/py/bleach.svg"></a>
             <a href="https://github.com/mozilla/bleach/actions?query=workflow%3ATest"><img style="max-width:100%;" alt="Build Status" src="https://github.com/mozilla/bleach/workflows/Test/badge.svg"></a>