Merge branch 'patrik-fix-path-traversal' into 'master'

Mitigate path traversal vulnerability #227

See merge request ikus-soft/rdiffweb!255

README.md

@@ -140,7 +140,13 @@ Breaking changes:
 * `session-dir` is deprecated and should be replace by `rate-limit-dir`. User's session are stored in database.
 * previous `.css` customization are not barkward compatible
 
-## 2.4.9 (2002-09-28)
+## 2.4.10 (2022-10-03)
+
+This releases include a security fix. If you are using an earlier version, you should upgrade to this release immediately.
+
+* Mitigate path traversal vulnerability [CVE-2022-3389](https://nvd.nist.gov/vuln/detail/CVE-2022-3389)
+
+## 2.4.9 (2022-09-28)
 
 This releases include a security fix. If you are using an earlier version, you should upgrade to this release immediately.
 

rdiffweb/controller/dispatch.py

@@ -23,10 +23,8 @@
 
 
 import inspect
-import os
 
 import cherrypy
-from cherrypy.lib.static import mimetypes, serve_file
 
 import rdiffweb.tools.auth_form  # noqa
 import rdiffweb.tools.auth_mfa  # noqa
@@ -146,34 +144,37 @@ def decorated(cls_or_self=None, vpath=None):
     return decorated
 
 
-def static(path):
+def staticdir(path):
     """
-    Create a page handler to serve static files. Disable authentication.
+    Create a page handler to serve static directory.
     """
-    assert isinstance(path, str)
-    assert os.path.exists(path), "%r doesn't exists" % path
-    content_type = None
-    if os.path.isfile(path):
-        # Set content-type based on filename extension
-        ext = ""
-        i = path.rfind('.')
-        if i != -1:
-            ext = path[i:].lower()
-        content_type = mimetypes.types_map.get(ext, None)  # @UndefinedVariable
 
     @cherrypy.expose
     @cherrypy.tools.auth_form(on=False)
     @cherrypy.tools.auth_mfa(on=False)
     @cherrypy.tools.ratelimit(on=False)
     @cherrypy.tools.sessions(on=False)
     @cherrypy.tools.secure_headers(on=False)
+    @cherrypy.tools.staticdir(section="", dir=path)
     def handler(*args, **kwargs):
-        if cherrypy.request.method not in ('GET', 'HEAD'):
-            raise cherrypy.HTTPError(405)
-        filename = os.path.join(path, *args)
-        assert filename.startswith(path)
-        if not os.path.isfile(filename):
-            raise cherrypy.HTTPError(404)
-        return serve_file(filename, content_type)
+        raise cherrypy.HTTPError(400)
+
+    return handler
+
+
+def staticfile(path):
+    """
+    Create a page handler to serve static file.
+    """
+
+    @cherrypy.expose
+    @cherrypy.tools.auth_form(on=False)
+    @cherrypy.tools.auth_mfa(on=False)
+    @cherrypy.tools.ratelimit(on=False)
+    @cherrypy.tools.sessions(on=False)
+    @cherrypy.tools.secure_headers(on=False)
+    @cherrypy.tools.staticfile(filename=path)
+    def handler(*args, **kwargs):
+        raise cherrypy.HTTPError(400)
 
     return handler

rdiffweb/controller/tests/test_controller.py

@@ -20,7 +20,6 @@
 @author: Patrik Dufresne
 """
 
-
 import datetime
 
 from parameterized import parameterized
@@ -78,6 +77,7 @@ def test_static_files(self, path):
         """
         Check if the theme is properly configure.
         """
+        self.getPage('/logout')
         self.getPage(path)
         self.assertStatus(200)
 
@@ -86,14 +86,18 @@ def test_static_invalid_method(self):
         Check if the theme is properly configure.
         """
         self.getPage("/static/default.css", method="POST")
-        self.assertStatus(405)
+        self.assertStatus(400)
 
     def test_static_invalid_file(self):
         """
         Check if the theme is properly configure.
         """
         self.getPage("/static/invalid.css")
-        self.assertStatus(404)
+        self.assertStatus(400)
+
+    def test_path_traversal(self):
+        self.getPage('/static//../../test.txt')
+        self.assertStatus(403)
 
 
 class ControllerOrangeThemeTest(rdiffweb.test.WebCase):
@@ -102,7 +106,7 @@ class ControllerOrangeThemeTest(rdiffweb.test.WebCase):
 
     default_config = {'DefaultTheme': 'orange'}
 
-    def test_theme(self):
+    def test_static(self):
         """
         Check if the theme is properly configure.
         """

rdiffweb/rdw_app.py

@@ -44,7 +44,7 @@
 import rdiffweb.tools.secure_headers
 from rdiffweb.controller import Controller
 from rdiffweb.controller.api import ApiPage
-from rdiffweb.controller.dispatch import static  # noqa
+from rdiffweb.controller.dispatch import staticdir, staticfile
 from rdiffweb.controller.page_admin import AdminPage
 from rdiffweb.controller.page_browse import BrowsePage
 from rdiffweb.controller.page_delete import DeletePage
@@ -106,11 +106,11 @@ def __init__(self):
 
         # Register static dir.
         static_dir = pkg_resources.resource_filename('rdiffweb', 'static')  # @UndefinedVariable
-        self.static = static(static_dir)
+        self.static = staticdir(static_dir)
 
         # Register robots.txt
         robots_txt = pkg_resources.resource_filename('rdiffweb', 'static/robots.txt')  # @UndefinedVariable
-        self.robots_txt = static(robots_txt)
+        self.robots_txt = staticfile(robots_txt)
 
 
 class RdiffwebApp(Application):
@@ -220,11 +220,11 @@ def __init__(self, cfg):
         Application.__init__(self, root=Root(), config=config)
 
         # Register favicon.ico
-        self.root.favicon_ico = static(self._favicon)
+        self.root.favicon_ico = staticfile(self._favicon)
 
         # Register header_logo
         if self._header_logo:
-            self.root.header_logo = static(self._header_logo)
+            self.root.header_logo = staticfile(self._header_logo)
 
         # Define TEMP env
         if self._tempdir: