Convert paths safely to file system paths

With the old implementation on Windows a path like
"/c:/file/ignore" got converted to "c:\file" and
allowed access to files outside of FOLDER

radicale/pathutils.py

@@ -20,8 +20,11 @@ Helper functions for working with paths
 
 """
 
+import os
 import posixpath
 
+from . import log
+
 
 def sanitize_path(path):
     """Make absolute (with leading slash) to prevent access to other data.
@@ -35,3 +38,35 @@ def sanitize_path(path):
         new_path = posixpath.join(new_path, part)
     trailing_slash = "" if new_path.endswith("/") else trailing_slash
     return new_path + trailing_slash
+
+
+def is_safe_filesystem_path_component(path):
+    """Checks if path is a single component of a local filesystem path
+       and is safe to join"""
+    if not path:
+        return False
+    drive, _ = os.path.splitdrive(path)
+    if drive:
+        return False
+    head, _ = os.path.split(path)
+    if head:
+        return False
+    if path in (os.curdir, os.pardir):
+        return False
+    return True
+
+
+def path_to_filesystem(path, base_folder):
+    """Converts path to a local filesystem path relative to base_folder
+        in a secure manner or raises ValueError."""
+    sane_path = sanitize_path(path).strip("/")
+    safe_path = base_folder
+    if not sane_path:
+        return safe_path
+    for part in sane_path.split("/"):
+        if not is_safe_filesystem_path_component(part):
+            log.LOGGER.debug("Can't translate path safely to filesystem: %s",
+                             path)
+            raise ValueError("Unsafe path")
+        safe_path = os.path.join(safe_path, part)
+    return safe_path

radicale/storage/filesystem.py

@@ -28,7 +28,8 @@ import json
 import time
 import sys
 from contextlib import contextmanager
-from .. import config, ical
+
+from .. import config, ical, pathutils
 
 
 FOLDER = os.path.expanduser(config.get("storage", "filesystem_folder"))
@@ -63,41 +64,41 @@ def open(path, mode="r"):
 class Collection(ical.Collection):
     """Collection stored in a flat ical file."""
     @property
-    def _path(self):
+    def _filesystem_path(self):
         """Absolute path of the file at local ``path``."""
-        return os.path.join(FOLDER, self.path.replace("/", os.sep))
+        return pathutils.path_to_filesystem(self.path, FOLDER)
 
     @property
     def _props_path(self):
         """Absolute path of the file storing the collection properties."""
-        return self._path + ".props"
+        return self._filesystem_path + ".props"
 
     def _create_dirs(self):
         """Create folder storing the collection if absent."""
-        if not os.path.exists(os.path.dirname(self._path)):
+        if not os.path.exists(os.path.dirname(self._filesystem_path)):
-            os.makedirs(os.path.dirname(self._path))
+            os.makedirs(os.path.dirname(self._filesystem_path))
 
     def save(self, text):
         self._create_dirs()
-        with open(self._path, "w") as fd:
+        with open(self._filesystem_path, "w") as fd:
             fd.write(text)
 
     def delete(self):
-        os.remove(self._path)
+        os.remove(self._filesystem_path)
         os.remove(self._props_path)
 
     @property
     def text(self):
         try:
-            with open(self._path) as fd:
+            with open(self._filesystem_path) as fd:
                 return fd.read()
         except IOError:
             return ""
 
     @classmethod
     def children(cls, path):
-        abs_path = os.path.join(FOLDER, path.replace("/", os.sep))
+        filesystem_path = pathutils.path_to_filesystem(path, FOLDER)
-        _, directories, files = next(os.walk(abs_path))
+        _, directories, files = next(os.walk(filesystem_path))
         for filename in directories + files:
             rel_filename = posixpath.join(path, filename)
             if cls.is_node(rel_filename) or cls.is_leaf(rel_filename):
@@ -105,17 +106,19 @@ class Collection(ical.Collection):
 
     @classmethod
     def is_node(cls, path):
-        abs_path = os.path.join(FOLDER, path.replace("/", os.sep))
+        filesystem_path = pathutils.path_to_filesystem(path, FOLDER)
-        return os.path.isdir(abs_path)
+        return os.path.isdir(filesystem_path)
 
     @classmethod
     def is_leaf(cls, path):
-        abs_path = os.path.join(FOLDER, path.replace("/", os.sep))
+        filesystem_path = pathutils.path_to_filesystem(path, FOLDER)
-        return os.path.isfile(abs_path) and not abs_path.endswith(".props")
+        return (os.path.isfile(filesystem_path) and not
+                filesystem_path.endswith(".props"))
 
     @property
     def last_modified(self):
-        modification_time = time.gmtime(os.path.getmtime(self._path))
+        modification_time = \
+            time.gmtime(os.path.getmtime(self._filesystem_path))
         return time.strftime("%a, %d %b %Y %H:%M:%S +0000", modification_time)
 
     @property

radicale/storage/multifilesystem.py

@@ -30,13 +30,14 @@ import sys
 from . import filesystem
 from .. import ical
 from .. import log
+from .. import pathutils
 
 
 class Collection(filesystem.Collection):
     """Collection stored in several files per calendar."""
     def _create_dirs(self):
-        if not os.path.exists(self._path):
+        if not os.path.exists(self._filesystem_path):
-            os.makedirs(self._path)
+            os.makedirs(self._filesystem_path)
 
     @property
     def headers(self):
@@ -52,17 +53,18 @@ class Collection(filesystem.Collection):
             name = (
                 component.name if sys.version_info[0] >= 3 else
                 component.name.encode(filesystem.FILESYSTEM_ENCODING))
-            path = os.path.join(self._path, name)
+            filesystem_path = os.path.join(self._filesystem_path, name)
-            with filesystem.open(path, "w") as fd:
+            with filesystem.open(filesystem_path, "w") as fd:
                 fd.write(text)
 
     def delete(self):
-        shutil.rmtree(self._path)
+        shutil.rmtree(self._filesystem_path)
         os.remove(self._props_path)
 
     def remove(self, name):
-        if os.path.exists(os.path.join(self._path, name)):
+        filesystem_path = os.path.join(self._filesystem_path, name)
-            os.remove(os.path.join(self._path, name))
+        if os.path.exists(filesystem_path):
+            os.remove(filesystem_path)
 
     @property
     def text(self):
@@ -70,14 +72,14 @@ class Collection(filesystem.Collection):
             ical.Timezone, ical.Event, ical.Todo, ical.Journal, ical.Card)
         items = set()
         try:
-            filenames = os.listdir(self._path)
+            filenames = os.listdir(self._filesystem_path)
         except (OSError, IOError) as e:
             log.LOGGER.info('Error while reading collection %r: %r'
-                            % (self._path, e))
+                            % (self._filesystem_path, e))
             return ""
 
         for filename in filenames:
-            path = os.path.join(self._path, filename)
+            path = os.path.join(self._filesystem_path, filename)
             try:
                 with filesystem.open(path) as fd:
                     items.update(self._parse(fd.read(), components))
@@ -90,17 +92,21 @@ class Collection(filesystem.Collection):
 
     @classmethod
     def is_node(cls, path):
-        path = os.path.join(filesystem.FOLDER, path.replace("/", os.sep))
+        filesystem_path = pathutils.path_to_filesystem(path,
-        return os.path.isdir(path) and not os.path.exists(path + ".props")
+                                                       filesystem.FOLDER)
+        return (os.path.isdir(filesystem_path) and
+                not os.path.exists(filesystem_path + ".props"))
 
     @classmethod
     def is_leaf(cls, path):
-        path = os.path.join(filesystem.FOLDER, path.replace("/", os.sep))
+        filesystem_path = pathutils.path_to_filesystem(path,
-        return os.path.isdir(path) and os.path.exists(path + ".props")
+                                                       filesystem.FOLDER)
+        return (os.path.isdir(filesystem_path) and
+                os.path.exists(path + ".props"))
 
     @property
     def last_modified(self):
         last = max([
-            os.path.getmtime(os.path.join(self._path, filename))
+            os.path.getmtime(os.path.join(self._filesystem_path, filename))
-            for filename in os.listdir(self._path)] or [0])
+            for filename in os.listdir(self._filesystem_path)] or [0])
         return time.strftime("%a, %d %b %Y %H:%M:%S +0000", time.gmtime(last))