Merge lp:~jderose/degu/dotdot into lp:degu

[Jason Gerard DeRose]

See https://bugs.launchpad.net/degu/+bug/1856935

[David Jordan (dmj726)]

Great, good security catch!

review: Approve

[David Jordan (dmj726)]

We should be peachy now.

=== modified file 'degu/_base.c'
--- degu/_base.c	2018-01-13 23:29:31 +0000
+++ degu/_base.c	2019-12-19 05:05:49 +0000
@@ -167,6 +167,7 @@
 _DEGU_SRC_CONSTANT(MINUS, "-")
 _DEGU_SRC_CONSTANT(SEMICOLON, ";")
 _DEGU_SRC_CONSTANT(EQUALS, "=")
+_DEGU_SRC_CONSTANT(DOTDOT, "..")
 
 
 /******************************************************************************
@@ -1789,6 +1790,10 @@
 static inline PyObject *
 _parse_path_component(DeguSrc src)
 {
+    if (_equal(src, DOTDOT)) {
+        _value_error("bad path component: %R", src);
+        return NULL;
+    }
     return _decode(src, PATH_MASK, "bad bytes in path component: %R");
 }
 

=== modified file 'degu/_basepy.py'
--- degu/_basepy.py	2016-10-16 18:32:04 +0000
+++ degu/_basepy.py	2019-12-19 05:05:49 +0000
@@ -605,6 +605,8 @@
 
 
 def _parse_path_component(src):
+    if src == b'..':
+        raise ValueError('bad path component: {!r}'.format(src))
     if PATH.issuperset(src):
         return src.decode('ascii')
     raise ValueError('bad bytes in path component: {!r}'.format(src))

=== modified file 'degu/tests/helpers.py'
--- degu/tests/helpers.py	2019-12-17 03:30:29 +0000
+++ degu/tests/helpers.py	2019-12-19 05:05:49 +0000
@@ -118,8 +118,8 @@
     assert isinstance(allowed, bytes)
     not_allowed = tables.invert(allowed)
     for i in range(len(good)):
+        bad = bytearray(good)
         for b in not_allowed:
-            bad = bytearray(good)
             bad[i] = b
             yield bytes(bad)
 

=== modified file 'degu/tests/test_base.py'
--- degu/tests/test_base.py	2019-11-26 15:57:14 +0000
+++ degu/tests/test_base.py	2019-12-19 05:05:49 +0000
@@ -1750,6 +1750,11 @@
         self.assertEqual(r.path, [])
         self.assertIsNone(r.query)
 
+        # Directory traversal:
+        with self.assertRaises(ValueError) as cm:
+            parse_request(b'GET /foo/../bar HTTP/1.1', rfile)
+        self.assertEqual(str(cm.exception), "bad path component: b'..'")
+
         r = parse_request(b'GET / HTTP/1.1\r\nRange: bytes=17-20', rfile)
         self.assertIs(type(r), Request)
         self.assertEqual(r.method, 'GET')
@@ -3144,6 +3149,49 @@
             parse_uri(b'foo')
         self.assertEqual(str(cm.exception), "path[0:1] != b'/': b'foo'")
 
+        # Bad bytes in URI:
+        good = b'/foo/bar?k=v&a=b'
+        for bad in iter_bad(good, bytes(_basepy.URI)):
+            with self.assertRaises(ValueError) as cm:
+                parse_uri(bad)
+            self.assertEqual(str(cm.exception),
+                'bad bytes in uri: {!r}'.format(bad)
+            )
+        with self.assertRaises(ValueError) as cm:
+            parse_uri(b'/hello /world')
+        self.assertEqual(str(cm.exception),
+            "bad bytes in uri: b'/hello /world'"
+        )
+
+        # dot-dot (directory traversal attack):
+        dotdots = (
+            b'/..',
+            b'/../',
+            b'/../foo',
+            b'/../foo/',
+            b'/foo/..',
+            b'/foo/../',
+            b'/foo/../bar',
+            b'/foo/../bar/',
+        )
+        for bad in dotdots:
+            with self.assertRaises(ValueError) as cm:
+                parse_uri(bad)
+            self.assertEqual(str(cm.exception), "bad path component: b'..'")
+        # '..' is allowed, just not as a path component:
+        self.assertEqual(parse_uri(b'/hello..'),
+            ('/hello..', [], ['hello..'], None)
+        )
+        self.assertEqual(parse_uri(b'/..hello'),
+            ('/..hello', [], ['..hello'], None)
+        )
+        self.assertEqual(parse_uri(b'/hello..world'),
+            ('/hello..world', [], ['hello..world'], None)
+        )
+        self.assertEqual(parse_uri(b'/...'),
+            ('/...', [], ['...'], None)
+        )
+
         # Empty path component:
         double_slashers = (
             b'//',

=== modified file 'doc/changelog.rst'
--- doc/changelog.rst	2019-12-16 23:36:50 +0000
+++ doc/changelog.rst	2019-12-19 05:05:49 +0000
@@ -8,6 +8,24 @@
 
 `Download Degu 0.19`_
 
+Bug fixes:
+
+    *   `lp:1856935`_ --- To prevent directory traversal attacks and similar
+        mischief, the Degu server now disallows ``'..'`` in request path
+        components.
+
+        For example, these URI will now be rejected (connection will be
+        closed)::
+
+            /..
+            /foo/../bar
+
+        However, ``'..'`` can occur as a sub-string, just not as the entirety of
+        a path component.  For example, these URI are still fine::
+
+            /hello..world
+            /foo/bar..
+
 
 New API additions:
 
@@ -2011,6 +2029,7 @@
 .. _`Download Degu 0.6`: https://launchpad.net/degu/+milestone/0.6
 .. _`Download Degu 0.5`: https://launchpad.net/degu/+milestone/0.5
 
+.. _`lp:1856935`: https://bugs.launchpad.net/degu/+bug/1856935
 .. _`lp:1590459`: https://bugs.launchpad.net/degu/+bug/1590459
 
 .. _`HTTPConnection.request()`: https://docs.python.org/3/library/http.client.html#http.client.HTTPConnection.request