Add some sanitization for paths, switch QRCode renderer to SvgPathImage

cgi-bin/pyupload.cgi

@@ -4,15 +4,17 @@ import cgitb
 cgitb.enable()
 
 import cgi
+import html
 import logging
 import os
+import pathlib
 import socket
 
 REQUEST_METHOD = os.environ.get('REQUEST_METHOD', 'GET')
 REQUEST_PORT = os.environ.get("SERVER_PORT", '8000')
 
 
-def render_form(success_filenames=None):
+def render_form(uploaded=None, not_uploaded=None):
     print("Content-Type: text/html")
     print()
     print("""
@@ -21,13 +23,22 @@ def render_form(success_filenames=None):
         </head>
     """)
 
-    if success_filenames:
+    if uploaded:
         print("""
             <h3>Files uploaded successfully:</h3>
             <ul>
         """)
-        for fname in success_filenames:
-            print(f"<li>{fname}</li>")
+        for fname in uploaded:
+            print(f"<li>{html.escape(fname)}</li>")
+        print("</ul><hr>")
+
+    if not_uploaded:
+        print("""
+            <h3>Files not uploaded successfully:</h3>
+            <ul>
+        """)
+        for fname, reason in not_uploaded:
+            print(f"<li>{html.escape(fname)} - {html.escape(reason)}</li>")
         print("</ul><hr>")
 
     print(f"""
@@ -47,7 +58,7 @@ def render_form(success_filenames=None):
         import qrcode.image.svg
 
         url = f'http://{socket.getfqdn()}.local:{REQUEST_PORT}/cgi-bin/pyupload.cgi'
-        qr = qrcode.make(url, image_factory=qrcode.image.svg.SvgImage)
+        qr = qrcode.make(url, image_factory=qrcode.image.svg.SvgPathImage)
         print(f"""
             <hr>
             <p>Scan to access on another device: <a href="{url}">{url}</a></p>
@@ -58,18 +69,52 @@ def render_form(success_filenames=None):
         logging.warning('Skipping generating address QR code, qrcode library not installed or not in python path.')
 
 
-if REQUEST_METHOD == 'POST':
+def sanitize_path(filename):
+    if not filename:
+        raise ValueError("No filename")
+
+    required_parent_dir = f"{os.path.dirname(__file__)}/../"
+    disallowed_dirs = [
+        f"{required_parent_dir}cgi-bin",
+        f"{required_parent_dir}htbin",
+        f"{required_parent_dir}.git",
+    ]
+
+    test_results = []
+    target = (pathlib.Path(required_parent_dir) / filename).resolve()
+    for test_dir in (required_parent_dir, *disallowed_dirs):
+        base = pathlib.Path(test_dir).resolve()
+        test_results.append(target != base and target.is_relative_to(base))
+
+    if not test_results[0] or any(test_results[1:]):
+        raise ValueError("Invalid filename")
+
+    return target.absolute()
+
+
+if REQUEST_METHOD == "POST":
     form = cgi.FieldStorage()
     files = form['uploadedfile'] if isinstance(form['uploadedfile'], list) else [form['uploadedfile']]
 
-    filenames = []
+    uploaded = []
+    not_uploaded = []
     for file in files:
         filename = file.filename
-        filenames.append(filename)
-        with open(f'{os.path.dirname(__file__)}/../{filename}', 'wb+') as f:
-            f.write(file.file.read())
+        try:
+            upload_path = sanitize_path(filename)
+            logging.info(f"Uploading to '{upload_path}'")
+
+            with open(upload_path, "wb+") as f:
+                f.write(file.file.read())
+
+            uploaded.append(filename)
+        except ValueError as e:
+            not_uploaded.append((filename, e.args[0]))
+        except Exception as e:
+            logging.error("Failed to upload", exc_info=True)
+            not_uploaded.append((filename, "Internal error"))
 
-    render_form(success_filenames=filenames)
+    render_form(uploaded=uploaded, not_uploaded=not_uploaded)
 
 else:
     render_form()