From 8208bf577acc4c098a82a770fc7e70e727d2ef32 Mon Sep 17 00:00:00 2001 From: Daniel Markstedt Date: Sat, 11 Nov 2023 14:31:58 +0900 Subject: [PATCH 1/3] Correct upload and download dir path validation logic --- python/web/src/web.py | 17 ++++++++++++----- python/web/src/web_utils.py | 28 +++++++++------------------- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/python/web/src/web.py b/python/web/src/web.py index 2e85722c56..2c611b1d16 100644 --- a/python/web/src/web.py +++ b/python/web/src/web.py @@ -58,7 +58,6 @@ 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) diff --git a/python/web/src/web_utils.py b/python/web/src/web_utils.py index 69af618bae..bc779836ac 100644 --- a/python/web/src/web_utils.py +++ b/python/web/src/web_utils.py @@ -8,7 +8,7 @@ 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. From b5ec4e7f9c91302c65f4874753c55f871fd98120 Mon Sep 17 00:00:00 2001 From: Daniel Markstedt Date: Sat, 11 Nov 2023 14:45:15 +0900 Subject: [PATCH 2/3] Improve file download labels --- python/web/src/templates/index.html | 4 ++-- python/web/src/templates/upload.html | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/python/web/src/templates/index.html b/python/web/src/templates/index.html index d1baf4d789..01e4967b2a 100644 --- a/python/web/src/templates/index.html +++ b/python/web/src/templates/index.html @@ -395,8 +395,8 @@ + - - + - + - +