Repository: Pylons/colander

Issue:
Unclosed parenthesis in URL causes infinite loop
When there is an unclosed parenthesis in URL and we use _url_ validator, it causes an infinite loop. What's more interesting is that it only happens when the unclosed parenthesis is followed by many characters (check test case number 3 and 4).
```
from colander import MappingSchema, SchemaNode, Str, url


class MySchema(MappingSchema):
    url = SchemaNode(Str(encoding='utf-8'), validator=url)

print MySchema().deserialize({"url": "http://www.mysite.com/tttttttttttttttttttttt.jpg"})  # it works
print MySchema().deserialize({"url": "http://www.mysite.com/(tttttttttttttttttttttt).jpg"})  # it works
print MySchema().deserialize({"url": "http://www.mysite.com/(ttttttttttt.jpg"})  # it works
print MySchema().deserialize({"url": "http://www.mysite.com/(tttttttttttttttttttttt.jpg"})  # infinite loop
```


In addition, if you check it in an online regex checker (https://regex101.com/) it also fails. Try this regex, it's used for URL validation in colander. It's taken from _colander.\_\_init\_\_.py:438_, I only escaped two slashes here.
```
(?i)\b((?:[a-z][\w-]+:(?:\/{1,3}|[a-z0-9%])|www\d{0,3}[.]|[a-z0-9.\-]+[.][a-z]{2,4}\/)(?:[^\s()<>]+|\(([^\s()<>]+|(\([^\s()<>]+\)))*\))+(?:\(([^\s()<>]+|(\([^\s()<>]+\)))*\)|[^\s`!()\[\]{};:'".,<>?«»“”‘’]))
```
Use this URL: _http://www.mysite.com/(tttttttttttttttttttttt.jpg_ and you'll get _catastrophic backtracking_. You can use debugger on that site to check which group falls in infinite loop.


Discussion:
bertjwregeer: I have requested a CVE for this vulnerability as a potential Denial of Service, will update CHANGES.rst and this ticket if it gets assigned.
bertjwregeer: This has been assigned CVE-ID: CVE-2017-18361

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index c40160b..f3f5d14 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -1,6 +1,23 @@
 Unreleased
 ==========
 
+- The URL validator regex has been updated to no longer be vulnerable to a
+  catastrophic backtracking that would have led to an infinite loop. See
+  https://github.com/Pylons/colander/pull/323 and
+  https://github.com/Pylons/colander/issues/290. With thanks to Przemek
+  (https://github.com/p-m-k).
+  
+  This does change the behaviour of the URL validator and it no longer supports
+  ``file://`` URI scheme (https://tools.ietf.org/html/rfc8089). Users that
+  wish to validate ``file://`` URI's should change their validator to use
+  ``colander.file_uri`` instead.
+
+  It has also dropped support for alternate schemes outside of http/ftp (and
+  their secure equivelants). Please let us know if we need to relax this
+  requirement.
+
+  CVE-ID: CVE-2017-18361
+
 - The Email validator has been updated to use the same regular expression that
   is used by the WhatWG HTML specification, thereby increasing the email
   addresses that will validate correctly from web forms submitted. See
diff --git a/colander/__init__.py b/colander/__init__.py
index 6f0dff5..6eac447 100644
--- a/colander/__init__.py
+++ b/colander/__init__.py
@@ -607,14 +607,40 @@ def _luhnok(value):
     return checksum
 
 
+# Gingerly lifted from Django 1.3.x:
+# https://github.com/django/django/blob/stable/1.3.x/django/core/validators.py#L45
+# <3 y'all!
 URL_REGEX = (
-    r'(?i)\b((?:[a-z][\w-]+:(?:/{1,3}|[a-z0-9%])|www\d{0,3}[.]|'
-    r'[a-z0-9.\-]+[.][a-z]{2,4}/)(?:[^\s()<>]+|\(([^\s()<>]+|'
-    r'(\([^\s()<>]+\)))*\))+(?:\(([^\s()<>]+|(\([^\s()<>]+\)))*\)|'
-    r'[^\s`!()\[\]{};:\'".,<>?«»“”‘’]))'  # "emacs!
+    # {http,ftp}s:// (not required)
+    r'^((?:http|ftp)s?://)?'
+    # Domain
+    r'(?:(?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\.)+'
+    r'(?:[A-Z]{2,6}\.?|[A-Z0-9-]{2,}\.?)|'
+    # Localhost
+    r'localhost|'
+    # IPv6 address
+    r'\[[a-f0-9:]+\]|'
+    # IPv4 address
+    r'\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})'
+    # Optional port
+    r'(?::\d+)?'
+    # Path
+    r'(?:/?|[/?]\S+)$'
 )
 
-url = Regex(URL_REGEX, _('Must be a URL'))
+url = Regex(URL_REGEX, msg=_('Must be a URL'), flags=re.IGNORECASE)
+
+
+URI_REGEX = (
+    # file:// (required)
+    r'^file://'
+    # Path
+    r'(?:/|[/?]\S+)$'
+)
+
+file_uri = Regex(
+    URI_REGEX, msg=_('Must be a file:// URI scheme'), flags=re.IGNORECASE
+)
 
 UUID_REGEX = (
     r'^(?:urn:uuid:)?\{?[a-f0-9]{8}(?:-?[a-f0-9]{4}){3}-?[a-f0-9]{12}\}?$'


Test patch:
diff --git a/colander/tests/test_colander.py b/colander/tests/test_colander.py
index 917a719..5411c2b 100644
--- a/colander/tests/test_colander.py
+++ b/colander/tests/test_colander.py
@@ -646,6 +646,76 @@ class Test_url_validator(unittest.TestCase):
 
         self.assertRaises(Invalid, self._callFUT, val)
 
+    def test_add_sample_dos(self):
+        # In the old regex (colander <=1.6) this would cause a catastrophic
+        # backtracking that would cause the regex engine to go into an infinite
+        # loop.
+        val = "http://www.mysite.com/(tttttttttttttttttttttt.jpg"
+
+        result = self._callFUT(val)
+        self.assertEqual(result, None)
+
+    def test_website_no_scheme(self):
+        val = "www.mysite.com"
+
+        result = self._callFUT(val)
+        self.assertEqual(result, None)
+
+    def test_ipv6(self):
+        val = "http://[2001:db8::0]/"
+
+        result = self._callFUT(val)
+        self.assertEqual(result, None)
+
+    def test_ipv4(self):
+        val = "http://192.0.2.1/"
+
+        result = self._callFUT(val)
+        self.assertEqual(result, None)
+
+    def test_file_raises(self):
+        from colander import Invalid
+
+        val = "file:///this/is/a/file.jpg"
+
+        self.assertRaises(Invalid, self._callFUT, val)
+
+
+class Test_file_uri_validator(unittest.TestCase):
+    def _callFUT(self, val):
+        from colander import file_uri
+
+        return file_uri(None, val)
+
+    def test_it_success(self):
+        val = 'file:///'
+        result = self._callFUT(val)
+        self.assertEqual(result, None)
+
+    def test_it_failure(self):
+        val = 'not-a-uri'
+        from colander import Invalid
+
+        self.assertRaises(Invalid, self._callFUT, val)
+
+    def test_no_path_fails(self):
+        val = 'file://'
+        from colander import Invalid
+
+        self.assertRaises(Invalid, self._callFUT, val)
+
+    def test_file_with_path(self):
+        val = "file:///this/is/a/file.jpg"
+
+        result = self._callFUT(val)
+        self.assertEqual(result, None)
+
+    def test_file_with_path_windows(self):
+        val = "file:///c:/is/a/file.jpg"
+
+        result = self._callFUT(val)
+        self.assertEqual(result, None)
+
 
 class TestUUID(unittest.TestCase):
     def _callFUT(self, val):