Sanitize DB path traversal (3.3 branch) (#1180)

* test: check that make_editor_session rejects funky paths

* fix: validate path in make_editor_session

* test: check that untrusted_to_os_path prevents traversal to parent

* fix[untrusted_to_os_path]: prevent traversal to parent directories

lektor/editor.py

@@ -15,8 +15,10 @@
 from lektor.constants import PRIMARY_ALT
 from lektor.metaformat import serialize
 from lektor.utils import atomic_open
+from lektor.utils import cleanup_path
 from lektor.utils import increment_filename
 from lektor.utils import is_valid_id
+from lektor.utils import parse_path
 from lektor.utils import secure_filename
 
 
@@ -32,8 +34,19 @@ class BadDelete(BadEdit):
     pass
 
 
+def _is_valid_path(path: str) -> bool:
+    split_path = path.strip("/").split("/")
+    if split_path == [""]:
+        split_path = []
+    return parse_path(path) == split_path
+
+
 def make_editor_session(pad, path, is_attachment=None, alt=PRIMARY_ALT, datamodel=None):
     """Creates an editor session for the given path object."""
+    if not _is_valid_path(path):
+        raise BadEdit("Invalid path")
+    path = cleanup_path(path)
+
     if alt != PRIMARY_ALT and not pad.db.config.is_valid_alternative(alt):
         raise BadEdit("Attempted to edit an invalid alternative (%s)" % alt)
 

lektor/utils.py

@@ -33,7 +33,7 @@
 
 _slash_escape = "\\/" not in json.dumps("/")
 
-_slashes_re = re.compile(r"(/\.{1,2}(/|$))|/")
+_slashes_re = re.compile(r"(?:/(?:\.{1,2}(?=/|$))?)+")
 _last_num_re = re.compile(r"^(.*)(\d+)(.*?)$")
 _list_marker = object()
 _value_marker = object()
@@ -109,10 +109,11 @@ def is_path_child_of(a, b, strict=True):
 
 
 def untrusted_to_os_path(path):
-    path = path.strip("/").replace("/", os.path.sep)
     if not isinstance(path, str):
         path = path.decode(fs_enc, "replace")
-    return path
+    clean_path = cleanup_path(path)
+    assert clean_path.startswith("/")
+    return clean_path[1:].replace("/", os.path.sep)
 
 
 def is_path(path):

tests/test_editor.py

@@ -43,6 +43,7 @@ def test_make_editor_session(pad, path, kwargs, expect):
             "conflicting",
             marks=pytest.mark.xfail(reason="buglet that should be fixed"),
         ),
+        ("/../../templates", {}, "Invalid path"),
     ],
 )
 def test_make_editor_session_raises_bad_edit(pad, path, kwargs, expect):

tests/test_utils.py

@@ -1,17 +1,20 @@
 # coding: utf-8
+import os
 from dataclasses import dataclass
 from urllib.parse import urlsplit
 
 import pytest
 
 from lektor.utils import build_url
+from lektor.utils import cleanup_path
 from lektor.utils import is_path_child_of
 from lektor.utils import join_path
 from lektor.utils import magic_split_ext
 from lektor.utils import make_relative_url
 from lektor.utils import parse_path
 from lektor.utils import secure_url
 from lektor.utils import slugify
+from lektor.utils import untrusted_to_os_path
 from lektor.utils import Url
 
 
@@ -229,3 +232,30 @@ def test_make_relative_url(source, target, expected):
 def test_make_relative_url_relative_source_absolute_target():
     with pytest.raises(ValueError):
         make_relative_url("rel/a/tive/", "/abs/o/lute")
+
+
+@pytest.mark.parametrize(
+    "db_path, expected",
+    [
+        ("a/b", "/a/b"),
+        ("//a//./b//", "/a/b"),
+        ("//a//../b//", "/a/b"),
+        ("//a//..x/b//", "/a/..x/b"),
+    ],
+)
+def test_cleanup_path(db_path, expected):
+    assert cleanup_path(db_path) == expected
+
+
+@pytest.mark.parametrize(
+    "db_path, expected",
+    [
+        ("a/b", "a/b"),
+        ("/a/b", "a/b"),
+        ("a/b/", "a/b"),
+        ("/../../a", "a"),
+    ],
+)
+def test_untrusted_to_os_path(db_path, expected):
+    os_path = untrusted_to_os_path(db_path)
+    assert os_path.split(os.sep) == expected.split("/")