Harden URL safety checks and sanitization in safe mode (#721)

CHANGES.md

@@ -12,6 +12,7 @@
 - [pull #713] Fix `header-ids` extra generating duplicate ids when a suffixed id collides with another header (#661)
 - [pull #705] XSS fixes in links, images, and more
 - [pull #720] Add `wiki-links` extra for `[[Page Name]]` style links (#221)
+- [pull #722] Harden URL safety checks and sanitization in safe mode (#721)
 
 
 ## python-markdown2 2.5.5

lib/markdown2.py

@@ -1397,9 +1397,12 @@ def _escape_special_chars(self, text: str) -> str:
         return ''.join(escaped)
 
     def _is_auto_link(self, text):
-        if ':' in text and self._auto_link_re.match(text):
-            return True
-        elif '@' in text and self._auto_email_link_re.match(text):
+        if ':' in text:
+            autolink_match = self._auto_link_re.match(text)
+            if autolink_match:
+                return self.safe_mode is None or self._safe_href.match(autolink_match.group(1))
+
+        if '@' in text and self._auto_email_link_re.match(text):
             return True
         return False
 
@@ -1451,7 +1454,10 @@ def _is_comment(token):
                     tokens.append(self._hash_span(self._sanitize_html(is_comment.group(3))))
                 elif self._is_unescaped_re.match(token) is None:
                     # if the HTML is escaped then escape any special chars and add the token as-is
-                    tokens.append(self._escape_special_chars(token))
+                    tokens.append(
+                        # HTML can be snuck into escaped comment bodies - #721
+                        self._sanitize_html(self._escape_special_chars(token))
+                    )
                 else:
                     tokens.append(self._hash_span(self._sanitize_html(token)))
             elif is_html_markup and is_code:
@@ -1493,10 +1499,11 @@ def _sanitize_html(self, s: str) -> str:
             return self.html_removed_text
         elif self.safe_mode == "escape":
             replacements = [
-                ('&', '&'),
                 ('<', '&lt;'),
                 ('>', '&gt;'),
             ]
+            # use a smart ampersand sub to avoid re-sanitizing stuff like `&lt;`
+            s = _AMPERSAND_RE.sub('&amp;', s)
             for before, after in replacements:
                 s = s.replace(before, after)
             return s
@@ -1602,7 +1609,8 @@ def _safe_href(self):
         # omitted ['"<>] for XSS reasons
         less_safe = r'#/\.!#$%&\(\)\+,/:;=\?@\[\]^`\{\}\|~'
         # html encoded colon in a URL still functions as a normal colon, so need to detect those
-        protocol_seperators = [':', '&#x3a;', '&#58;', '&colon;']
+        # semicolon at the end is optional in browsers - see #721
+        protocol_seperators = [':', r'&#x3a;?', r'&#58;?', r'&colon;?']
         # dot seperated hostname, optional port number, not followed by protocol seperator
         domain = r'(?:[{}]+(?:\.[{}]+)*)(?:(?<!tel)(?<!javascript):\d+/?)?(?![^:/]*(?:{})/*)'.format(safe, safe, '|'.join(protocol_seperators))
         fragment = r'[%s]*' % (safe + less_safe)

test/tm-cases/escaped_html_in_safe_mode.html

@@ -1,3 +1,3 @@
 <p>&lt;abc&gt;
-&lt;abc>
+&lt;abc&gt;
 &lt;why?</p>

test/tm-cases/hash_html_blocks_orphaned_close_tags.html

@@ -6,6 +6,6 @@
 
 <p></ul></p>
 
-<p><a href="http:/onmouseover=alert(origin)">http:/onmouseover=alert(origin)</a></p>
+<p>&lt;http:/onmouseover=alert(origin)&gt;</p>
 
 <p>-</p>

test/tm-cases/xss_issue721.html

@@ -0,0 +1,5 @@
+<p><a href="#">Click me</a></p>
+
+<p>&lt;http:|&gt;&lt;x| oncontentvisibilityautostatechange=alert(origin) style=display:block;content-visibility:auto></p>
+
+<p>&lt;!--&lt;img src onerror=alert(origin)//&gt;--&gt;</p>

test/tm-cases/xss_issue721.opts

@@ -0,0 +1 @@
+{"safe_mode": "escape"}

test/tm-cases/xss_issue721.text

@@ -0,0 +1,5 @@
+[Click me](javascript&#58alert(origin))
+
+<http:|><x| oncontentvisibilityautostatechange=alert(origin) style=display:block;content-visibility:auto>
+
+\<!--<img src onerror=alert(origin)//>-->