Correct upload and download dir path validation logic

python/web/src/web.py

@@ -58,7 +58,6 @@ from web_utils import (
     auth_active,
     is_bridge_configured,
     is_safe_path,
-    validate_target_dir,
     browser_supports_modern_themes,
 )
 from settings import (
@@ -996,15 +995,19 @@ def download_file():
     images_subdir = request.form.get("images_subdir")
     shared_subdir = request.form.get("shared_subdir")
     if destination == "disk_images":
+        safe_path = is_safe_path(Path(images_subdir))
+        if not safe_path["status"]:
+            return response(error=True, message=safe_path["msg"])
         server_info = piscsi_cmd.get_server_info()
         destination_dir = Path(server_info["image_dir"]) / images_subdir
     elif destination == "shared_files":
+        safe_path = is_safe_path(Path(shared_subdir))
+        if not safe_path["status"]:
+            return response(error=True, message=safe_path["msg"])
         destination_dir = Path(FILE_SERVER_DIR) / shared_subdir
     else:
         return response(error=True, message=_("Unknown destination"))
 
-    validate_target_dir(destination_dir)
-
     process = file_cmd.download_to_dir(url, Path(destination_dir) / Path(url).name)
     process = ReturnCodeMapper.add_msg(process)
     if process["status"]:
@@ -1034,17 +1037,21 @@ def upload_file():
     images_subdir = request.form.get("images_subdir")
     shared_subdir = request.form.get("shared_subdir")
     if destination == "disk_images":
+        safe_path = is_safe_path(Path(images_subdir))
+        if not safe_path["status"]:
+            return make_response(safe_path["msg"], 403)
         server_info = piscsi_cmd.get_server_info()
         destination_dir = Path(server_info["image_dir"]) / images_subdir
     elif destination == "shared_files":
+        safe_path = is_safe_path(Path(shared_subdir))
+        if not safe_path["status"]:
+            return make_response(safe_path["msg"], 403)
         destination_dir = Path(FILE_SERVER_DIR) / shared_subdir
     elif destination == "piscsi_config":
         destination_dir = Path(CFG_DIR)
     else:
         return make_response(_("Unknown destination"), 403)
 
-    validate_target_dir(destination_dir)
-
     log = logging.getLogger("pydrop")
     file_object = request.files["file"]
     file_name = secure_filename(file_object.filename)

python/web/src/web_utils.py

@@ -8,7 +8,7 @@ from pathlib import Path
 from ua_parser import user_agent_parser
 from re import findall
 
-from flask import request, abort, make_response
+from flask import request, abort
 from flask_babel import _
 from werkzeug.utils import secure_filename
 
@@ -302,37 +302,27 @@ def is_bridge_configured(interface):
     return {"status": True, "msg": return_msg + ", ".join(to_configure)}
 
 
-def is_safe_path(file_name):
+def is_safe_path(path_name):
     """
-    Takes (Path) file_name with the path to a file on the file system
-    Returns True if the path is safe
-    Returns False if the path is either absolute, or tries to traverse the file system
+    Takes (Path) path_name with the relative path to a file on the file system.
+    Returns False if the path is either absolute, or tries to traverse the file system.
+    Returns True if neither of the above criteria are met.
     """
     error_message = ""
-    if file_name.is_absolute():
+    if path_name.is_absolute():
         error_message = _("Path must not be absolute")
-    elif "../" in str(file_name):
+    elif "../" in str(path_name):
         error_message = _("Path must not traverse the file system")
-    elif str(file_name)[0] == "~":
+    elif str(path_name)[0] == "~":
         error_message = _("Path must not start in the home directory")
 
     if error_message:
-        logging.error("Not an allowed path: %s", str(file_name))
+        logging.error("Not an allowed path: %s", str(path_name))
         return {"status": False, "msg": error_message}
 
     return {"status": True, "msg": ""}
 
 
-def validate_target_dir(target_dir):
-    """
-    Takes (Path) target_dir on the host and validates that it is a valid dir for uploading.
-    """
-    safe_path = is_safe_path(Path(".") / target_dir)
-    if not safe_path["status"]:
-        return make_response(safe_path["msg"], 403)
-    return True
-
-
 def browser_supports_modern_themes():
     """
     Determines if the browser supports the HTML/CSS/JS features used in non-legacy themes.