CVE-2016-9909: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: The serializer in html5lib before 0.99999999 might allow remote attackers to conduct cross-site scripting (XSS) attacks by leveraging mishandling of the < (less than) character in attribute values.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2017-02-22T16:59Z
Repository: https://github.com/html5lib/html5lib-python
Language: Python
Files changed: CHANGES.rst, html5lib/serializer/htmlserializer.py, html5lib/tests/serializer-testdata/core.test, html5lib/tests/serializer-testdata/options.test, html5lib/tests/test_serializer.py
Commit date: 2016-05-11 16:55:45 -0700
Commit message: Fix #11, #12: quote attributes that need escaping in legacy browsers

These are mostly out of the market now, so this isn't massively
needed any more; nevertheless, avoiding XSS as much as possible is
inevitably desirable.

This alters the API so that quote_attr_values is now a ternary
setting, choosing between legacy-safe behaviour, spec behaviour, and
always quoting.

Diff: diff --git a/CHANGES.rst b/CHANGES.rst
index 62a6a23..1b55781 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -31,10 +31,17 @@ Released on XXX
 * **Fix comments containing double-dash with lxml 3.5 and above.**
 
 * **Use scripting disabled by default (as we don't implement
   scripting).**
 
+* **Fix #11, avoiding the XSS bug potentially caused by serializer
+  allowing attribute values to be escaped out of in old browser versions,
+  changing the quote_attr_values option on serializer to take one of
+  three values, "always" (the old True value), "legacy" (the new option,
+  and the new default), and "spec" (the old False value, and the old
+  default).**
+
 
 0.9999999/1.0b8
 ~~~~~~~~~~~~~~~
 
 Released on September 10, 2015
diff --git a/html5lib/serializer/htmlserializer.py b/html5lib/serializer/htmlserializer.py
index 6c983f2..afe2e0e 100644
--- a/html5lib/serializer/htmlserializer.py
+++ b/html5lib/serializer/htmlserializer.py
@@ -8,11 +8,20 @@ from ..constants import rcdataElements, entities, xmlEntities
 from .. import utils
 from xml.sax.saxutils import escape
 
 spaceCharacters = "".join(spaceCharacters)
 
-quoteAttributeSpec = re.compile("[" + spaceCharacters + "\"'=<>`]")
+quoteAttributeSpecChars = spaceCharacters + "\"'=<>`"
+quoteAttributeSpec = re.compile("[" + quoteAttributeSpecChars + "]")
+quoteAttributeLegacy = re.compile("[" + quoteAttributeSpecChars +
+                                  "\x00\x01\x02\x03\x04\x05\x06\x07\x08\t\n"
+                                  "\x0b\x0c\r\x0e\x0f\x10\x11\x12\x13\x14\x15"
+                                  "\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f"
+                                  "\x20\x2f\x60\xa0\u1680\u180e\u180f\u2000"
+                                  "\u2001\u2002\u2003\u2004\u2005\u2006\u2007"
+                                  "\u2008\u2009\u200a\u2028\u2029\u202f\u205f"
+                                  "\u3000]")
 
 try:
     from codecs import register_error, xmlcharrefreplace_errors
 except ImportError:
     unicode_encode_errors = "strict"
@@ -70,11 +79,11 @@ else:
 
 
 class HTMLSerializer(object):
 
     # attribute quoting options
-    quote_attr_values = False
+    quote_attr_values = "legacy"  # be secure by default
     quote_char = '"'
     use_best_quote_char = True
 
     # tag syntax options
     omit_optional_tags = True
@@ -106,13 +115,13 @@ class HTMLSerializer(object):
         Keyword options (default given first unless specified) include:
 
         inject_meta_charset=True|False
           Whether it insert a meta element to define the character set of the
           document.
-        quote_attr_values=True|False
+        quote_attr_values="legacy"|"spec"|"always"
           Whether to quote attribute values that don't require quoting
-          per HTML5 parsing rules.
+          per legacy browser behaviour, when required by the standard, or always.
         quote_char=u'"'|u"'"
           Use given quote character for attribute quoting. Default is to
           use double quote unless attribute value contains a double quote,
           in which case single quotes are used instead.
         escape_lt_in_attrs=False|True
@@ -237,14 +246,19 @@ class HTMLSerializer(object):
                     yield self.encodeStrict(k)
                     if not self.minimize_boolean_attributes or \
                         (k not in booleanAttributes.get(name, tuple()) and
                          k not in booleanAttributes.get("", tuple())):
                         yield self.encodeStrict("=")
-                        if self.quote_attr_values:
+                        if self.quote_attr_values == "always" or len(v) == 0:
                             quote_attr = True
+                        elif self.quote_attr_values == "spec":
+                            quote_attr = quoteAttributeSpec.search(v) is not None
+                        elif self.quote_attr_values == "legacy":
+                            quote_attr = quoteAttributeLegacy.search(v) is not None
                         else:
-                            quote_attr = len(v) == 0 or quoteAttributeSpec.search(v)
+                            raise ValueError("quote_attr_values must be one of: "
+                                             "'always', 'spec', or 'legacy'")
                         v = v.replace("&", "&amp;")
                         if self.escape_lt_in_attrs:
                             v = v.replace("<", "&lt;")
                         if quote_attr:
                             quote_char = self.quote_char
diff --git a/html5lib/tests/serializer-testdata/core.test b/html5lib/tests/serializer-testdata/core.test
index d18b7bf..70828d0 100644
--- a/html5lib/tests/serializer-testdata/core.test
+++ b/html5lib/tests/serializer-testdata/core.test
@@ -240,11 +240,11 @@
             ],
             "description": "proper attribute value quoting (with CR)"
         },
         {
             "expected": [
-                "<span title=foo\u000bbar>"
+                "<span title=\"foo\u000bbar\">"
             ],
             "input": [
                 [
                     "StartTag",
                     "http://www.w3.org/1999/xhtml",
diff --git a/html5lib/tests/serializer-testdata/options.test b/html5lib/tests/serializer-testdata/options.test
index ba48064..eedcb3f 100644
--- a/html5lib/tests/serializer-testdata/options.test
+++ b/html5lib/tests/serializer-testdata/options.test
@@ -39,13 +39,13 @@
                             "value": "disabled"
                         }
                     ]
                 ]
             ],
-            "description": "quote_attr_values=true",
+            "description": "quote_attr_values='always'",
             "options": {
-                "quote_attr_values": true
+                "quote_attr_values": "always"
             }
         },
         {
             "expected": [
                 "<div irrelevant>"
@@ -62,13 +62,82 @@
                             "value": "irrelevant"
                         }
                     ]
                 ]
             ],
-            "description": "quote_attr_values=true with irrelevant",
+            "description": "quote_attr_values='always' with irrelevant",
             "options": {
-                "quote_attr_values": true
+                "quote_attr_values": "always"
+            }
+        },
+        {
+            "expected": [
+                "<div class=\"foo\">"
+            ],
+            "input": [
+                [
+                    "StartTag",
+                    "http://www.w3.org/1999/xhtml",
+                    "div",
+                    [
+                        {
+                            "namespace": null,
+                            "name": "class",
+                            "value": "foo"
+                        }
+                    ]
+                ]
+            ],
+            "description": "non-minimized quote_attr_values='always'",
+            "options": {
+                "quote_attr_values": "always"
+            }
+        },
+        {
+            "expected": [
+                "<div class=foo>"
+            ],
+            "input": [
+                [
+                    "StartTag",
+                    "http://www.w3.org/1999/xhtml",
+                    "div",
+                    [
+                        {
+                            "namespace": null,
+                            "name": "class",
+                            "value": "foo"
+                        }
+                    ]
+                ]
+            ],
+            "description": "non-minimized quote_attr_values='legacy'",
+            "options": {
+                "quote_attr_values": "legacy"
+            }
+        },
+        {
+            "expected": [
+                "<div class=foo>"
+            ],
+            "input": [
+                [
+                    "StartTag",
+                    "http://www.w3.org/1999/xhtml",
+                    "div",
+                    [
+                        {
+                            "namespace": null,
+                            "name": "class",
+                            "value": "foo"
+                        }
+                    ]
+                ]
+            ],
+            "description": "non-minimized quote_attr_values='spec'",
+            "options": {
+                "quote_attr_values": "spec"
             }
         },
         {
             "expected": [
                 "<img />"
diff --git a/html5lib/tests/test_serializer.py b/html5lib/tests/test_serializer.py
index 33f2a3b..9327626 100644
--- a/html5lib/tests/test_serializer.py
+++ b/html5lib/tests/test_serializer.py
@@ -144,10 +144,41 @@ def testEndTagName():
 
 def testComment():
     throwsWithLatin1([["Comment", "\u0101"]])
 
 
+@pytest.mark.parametrize("c", list("\t\n\u000C\x20\r\"'=<>`"))
+def testSpecQuoteAttribute(c):
+    input_ = [["StartTag", "http://www.w3.org/1999/xhtml", "span",
+               [{"namespace": None, "name": "foo", "value": c}]]]
+    if c == '"':
+        output_ = ["<span foo='%s'>" % c]
+    else:
+        output_ = ['<span foo="%s">' % c]
+    options_ = {"quote_attr_values": "spec"}
+    runSerializerTest(input_, output_, options_)
+
+
+@pytest.mark.parametrize("c", list("\t\n\u000C\x20\r\"'=<>`"
+                                   "\x00\x01\x02\x03\x04\x05\x06\x07\x08\t\n"
+                                   "\x0b\x0c\r\x0e\x0f\x10\x11\x12\x13\x14\x15"
+                                   "\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f"
+                                   "\x20\x2f\x60\xa0\u1680\u180e\u180f\u2000"
+                                   "\u2001\u2002\u2003\u2004\u2005\u2006\u2007"
+                                   "\u2008\u2009\u200a\u2028\u2029\u202f\u205f"
+                                   "\u3000"))
+def testLegacyQuoteAttribute(c):
+    input_ = [["StartTag", "http://www.w3.org/1999/xhtml", "span",
+               [{"namespace": None, "name": "foo", "value": c}]]]
+    if c == '"':
+        output_ = ["<span foo='%s'>" % c]
+    else:
+        output_ = ['<span foo="%s">' % c]
+    options_ = {"quote_attr_values": "legacy"}
+    runSerializerTest(input_, output_, options_)
+
+
 @pytest.fixture
 def lxml_parser():
     return etree.XMLParser(resolve_entities=False)
 
 

Vulnerable code: quoteAttributeSpec = re.compile("[" + spaceCharacters + "\"'=<>`]")
    quote_attr_values = False
        quote_attr_values=True|False
          per HTML5 parsing rules.
                        if self.quote_attr_values:
                            quote_attr = len(v) == 0 or quoteAttributeSpec.search(v)
                "<span title=foo\u000bbar>"
            "description": "quote_attr_values=true",
                "quote_attr_values": true
            "description": "quote_attr_values=true with irrelevant",
                "quote_attr_values": true
Fixed code: * **Fix #11, avoiding the XSS bug potentially caused by serializer
  allowing attribute values to be escaped out of in old browser versions,
  changing the quote_attr_values option on serializer to take one of
  three values, "always" (the old True value), "legacy" (the new option,
  and the new default), and "spec" (the old False value, and the old
  default).**

quoteAttributeSpecChars = spaceCharacters + "\"'=<>`"
quoteAttributeSpec = re.compile("[" + quoteAttributeSpecChars + "]")
quoteAttributeLegacy = re.compile("[" + quoteAttributeSpecChars +
                                  "\x00\x01\x02\x03\x04\x05\x06\x07\x08\t\n"
                                  "\x0b\x0c\r\x0e\x0f\x10\x11\x12\x13\x14\x15"
                                  "\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f"
                                  "\x20\x2f\x60\xa0\u1680\u180e\u180f\u2000"
                                  "\u2001\u2002\u2003\u2004\u2005\u2006\u2007"
                                  "\u2008\u2009\u200a\u2028\u2029\u202f\u205f"
                                  "\u3000]")
    quote_attr_values = "legacy"  # be secure by default
        quote_attr_values="legacy"|"spec"|"always"
          per legacy browser behaviour, when required by the standard, or always.
                        if self.quote_attr_values == "always" or len(v) == 0:
                        elif self.quote_attr_values == "spec":
                            quote_attr = quoteAttributeSpec.search(v) is not None
                        elif self.quote_attr_values == "legacy":
                            quote_attr = quoteAttributeLegacy.search(v) is not None
                            raise ValueError("quote_attr_values must be one of: "
                                             "'always', 'spec', or 'legacy'")
                "<span title=\"foo\u000bbar\">"
            "description": "quote_attr_values='always'",
                "quote_attr_values": "always"
            "description": "quote_attr_values='always' with irrelevant",
                "quote_attr_values": "always"
            }
        },
        {
            "expected": [
                "<div class=\"foo\">"
            ],
            "input": [
                [
                    "StartTag",
                    "http://www.w3.org/1999/xhtml",
                    "div",
                    [
                        {
                            "namespace": null,
                            "name": "class",
                            "value": "foo"
                        }
                    ]
                ]
            ],
            "description": "non-minimized quote_attr_values='always'",
            "options": {
                "quote_attr_values": "always"
            }
        },
        {
            "expected": [
                "<div class=foo>"
            ],
            "input": [
                [
                    "StartTag",
                    "http://www.w3.org/1999/xhtml",
                    "div",
                    [
                        {
                            "namespace": null,
                            "name": "class",
                            "value": "foo"
                        }
                    ]
                ]
            ],
            "description": "non-minimized quote_attr_values='legacy'",
            "options": {
                "quote_attr_values": "legacy"
            }
        },
        {
            "expected": [
                "<div class=foo>"
            ],
            "input": [
                [
                    "StartTag",
                    "http://www.w3.org/1999/xhtml",
                    "div",
                    [
                        {
                            "namespace": null,
                            "name": "class",
                            "value": "foo"
                        }
                    ]
                ]
            ],
            "description": "non-minimized quote_attr_values='spec'",
            "options": {
                "quote_attr_values": "spec"
@pytest.mark.parametrize("c", list("\t\n\u000C\x20\r\"'=<>`"))
def testSpecQuoteAttribute(c):
    input_ = [["StartTag", "http://www.w3.org/1999/xhtml", "span",
               [{"namespace": None, "name": "foo", "value": c}]]]
    if c == '"':
        output_ = ["<span foo='%s'>" % c]
    else:
        output_ = ['<span foo="%s">' % c]
    options_ = {"quote_attr_values": "spec"}
    runSerializerTest(input_, output_, options_)


@pytest.mark.parametrize("c", list("\t\n\u000C\x20\r\"'=<>`"
                                   "\x00\x01\x02\x03\x04\x05\x06\x07\x08\t\n"
                                   "\x0b\x0c\r\x0e\x0f\x10\x11\x12\x13\x14\x15"
                                   "\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f"
                                   "\x20\x2f\x60\xa0\u1680\u180e\u180f\u2000"
                                   "\u2001\u2002\u2003\u2004\u2005\u2006\u2007"
                                   "\u2008\u2009\u200a\u2028\u2029\u202f\u205f"
                                   "\u3000"))
def testLegacyQuoteAttribute(c):
    input_ = [["StartTag", "http://www.w3.org/1999/xhtml", "span",
               [{"namespace": None, "name": "foo", "value": c}]]]
    if c == '"':
        output_ = ["<span foo='%s'>" % c]
    else:
        output_ = ['<span foo="%s">' % c]
    options_ = {"quote_attr_values": "legacy"}
    runSerializerTest(input_, output_, options_)