From 7393d87bd354e43120937789956175064e4610a0 Mon Sep 17 00:00:00 2001
From: Jeff Dairiki <dairiki@dairiki.org>
Date: Tue, 27 Feb 2024 10:40:28 -0800
Subject: [PATCH] Sanitize DB path traversal (#1179)

* 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     | 13 +++++++++++++
 lektor/utils.py      |  5 +++--
 tests/test_editor.py |  1 +
 tests/test_utils.py  | 16 ++++++++++++++++
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/lektor/editor.py b/lektor/editor.py
index 8f14e93f9..e5ea69261 100644
--- a/lektor/editor.py
+++ b/lektor/editor.py
@@ -16,8 +16,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
 
 
@@ -33,8 +35,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)
 
diff --git a/lektor/utils.py b/lektor/utils.py
index e21470d61..87748aa93 100644
--- a/lektor/utils.py
+++ b/lektor/utils.py
@@ -149,10 +149,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):
diff --git a/tests/test_editor.py b/tests/test_editor.py
index 1d844d376..d490ffbe5 100644
--- a/tests/test_editor.py
+++ b/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):
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 5f92b7be2..de26ecf63 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -1,3 +1,4 @@
+import os
 import warnings
 from contextlib import contextmanager
 from dataclasses import dataclass
@@ -16,6 +17,7 @@
 from lektor.utils import secure_url
 from lektor.utils import slugify
 from lektor.utils import unique_everseen
+from lektor.utils import untrusted_to_os_path
 from lektor.utils import Url
 
 
@@ -336,3 +338,17 @@ def _warning_line(warning: warnings.WarningMessage) -> str:
     """Get the text of the line for which warning was issued."""
     with open(warning.filename, encoding="utf-8") as fp:
         return next(islice(fp, warning.lineno - 1, None), None)
+
+
+@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("/")