From 9ebb77fe1d5feb808d596a45ddd85c6a554cd229 Mon Sep 17 00:00:00 2001
From: Daniel Markstedt <markstedt@gmail.com>
Date: Wed, 1 Nov 2023 00:06:11 +0900
Subject: [PATCH] Safer handling of file download paths

---
 python/common/src/piscsi/file_cmds.py | 12 ++++++------
 python/web/src/return_code_mapper.py  |  2 +-
 python/web/src/web.py                 |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/python/common/src/piscsi/file_cmds.py b/python/common/src/piscsi/file_cmds.py
index 9f696c59..1b08dada 100644
--- a/python/common/src/piscsi/file_cmds.py
+++ b/python/common/src/piscsi/file_cmds.py
@@ -486,14 +486,14 @@ class FileCmds:
 
         file_name = PurePath(url).name
         iso_filename = Path(server_info["image_dir"]) / f"{file_name}.iso"
+        tmp_full_path = Path(tmp_dir) / file_name
 
         with TemporaryDirectory() as tmp_dir:
-            req_proc = self.download_to_dir(quote(url, safe=URL_SAFE), tmp_dir, file_name)
+            req_proc = self.download_to_dir(quote(url, safe=URL_SAFE), tmp_full_path)
             logging.info("Downloaded %s to %s", file_name, tmp_dir)
             if not req_proc["status"]:
                 return {"status": False, "msg": req_proc["msg"]}
 
-            tmp_full_path = Path(tmp_dir) / file_name
             if is_zipfile(tmp_full_path):
                 if "XtraStuf.mac" in str(ZipFile(str(tmp_full_path)).namelist()):
                     logging.info(
@@ -565,9 +565,9 @@ class FileCmds:
         }
 
     # noinspection PyMethodMayBeStatic
-    def download_to_dir(self, url, save_dir, file_name):
+    def download_to_dir(self, url, target_path):
         """
-        Takes (str) url, (str) save_dir, (str) file_name
+        Takes (str) url, (Path) target_path
         Returns (dict) with (bool) status and (str) msg
         """
         logging.info("Making a request to download %s", url)
@@ -580,7 +580,7 @@ class FileCmds:
             ) as req:
                 req.raise_for_status()
                 try:
-                    with open(f"{save_dir}/{file_name}", "wb") as download:
+                    with open(str(target_path), "wb") as download:
                         for chunk in req.iter_content(chunk_size=8192):
                             download.write(chunk)
                 except FileNotFoundError as error:
@@ -593,7 +593,7 @@ class FileCmds:
         logging.info("Response content-type: %s", req.headers["content-type"])
         logging.info("Response status code: %s", req.status_code)
 
-        parameters = {"file_name": file_name, "save_dir": save_dir}
+        parameters = {"target_path": str(target_path)}
         return {
             "status": True,
             "return_code": ReturnCodes.DOWNLOADTODIR_SUCCESS,
diff --git a/python/web/src/return_code_mapper.py b/python/web/src/return_code_mapper.py
index 95025d9b..dd5e43b9 100644
--- a/python/web/src/return_code_mapper.py
+++ b/python/web/src/return_code_mapper.py
@@ -23,7 +23,7 @@ class ReturnCodeMapper:
         ReturnCodes.DOWNLOADFILETOISO_SUCCESS:
             _("Created CD-ROM ISO image with arguments \"%(value)s\""),
         ReturnCodes.DOWNLOADTODIR_SUCCESS:
-            _("%(file_name)s downloaded to %(save_dir)s"),
+            _("Downloaded file to %(target_path)s"),
         ReturnCodes.WRITEFILE_SUCCESS:
             _("File created: %(target_path)s"),
         ReturnCodes.WRITEFILE_COULD_NOT_WRITE:
diff --git a/python/web/src/web.py b/python/web/src/web.py
index 865ef0d2..7310a068 100644
--- a/python/web/src/web.py
+++ b/python/web/src/web.py
@@ -991,7 +991,7 @@ def download_file():
     else:
         return response(error=True, message=_("Unknown destination"))
 
-    process = file_cmd.download_to_dir(url, destination_dir, Path(url).name)
+    process = file_cmd.download_to_dir(url, Path(destination_dir) / Path(url).name)
     process = ReturnCodeMapper.add_msg(process)
     if process["status"]:
         return response(message=process["msg"])