From 0eefe9e93646ef5f7915a4b8962d50f0fed8e54f Mon Sep 17 00:00:00 2001 From: Zach Parks Date: Tue, 12 Dec 2023 20:12:16 -0600 Subject: [PATCH] WebHost: Some refactors and additional checks when uploading files. (#2549) --- WebHostLib/check.py | 55 +++++++++++++++++++++-------------------- WebHostLib/upload.py | 58 ++++++++++++++++++++++++++------------------ 2 files changed, 63 insertions(+), 50 deletions(-) diff --git a/WebHostLib/check.py b/WebHostLib/check.py index 4db2ec2c..e739dda0 100644 --- a/WebHostLib/check.py +++ b/WebHostLib/check.py @@ -1,3 +1,4 @@ +import os import zipfile import base64 from typing import Union, Dict, Set, Tuple @@ -6,13 +7,7 @@ from flask import request, flash, redirect, url_for, render_template from markupsafe import Markup from WebHostLib import app - -banned_zip_contents = (".sfc",) - - -def allowed_file(filename): - return filename.endswith(('.txt', ".yaml", ".zip")) - +from WebHostLib.upload import allowed_options, allowed_options_extensions, banned_file from Generate import roll_settings, PlandoOptions from Utils import parse_yamls @@ -51,33 +46,41 @@ def mysterycheck(): def get_yaml_data(files) -> Union[Dict[str, str], str, Markup]: options = {} for uploaded_file in files: - # if user does not select file, browser also - # submit an empty part without filename - if uploaded_file.filename == '': - return 'No selected file' + if banned_file(uploaded_file.filename): + return ("Uploaded data contained a rom file, which is likely to contain copyrighted material. " + "Your file was deleted.") + # If the user does not select file, the browser will still submit an empty string without a file name. + elif uploaded_file.filename == "": + return "No selected file." elif uploaded_file.filename in options: - return f'Conflicting files named {uploaded_file.filename} submitted' - elif uploaded_file and allowed_file(uploaded_file.filename): + return f"Conflicting files named {uploaded_file.filename} submitted." + elif uploaded_file and allowed_options(uploaded_file.filename): if uploaded_file.filename.endswith(".zip"): + if not zipfile.is_zipfile(uploaded_file): + return f"Uploaded file {uploaded_file.filename} is not a valid .zip file and cannot be opened." - with zipfile.ZipFile(uploaded_file, 'r') as zfile: - infolist = zfile.infolist() + uploaded_file.seek(0) # offset from is_zipfile check + with zipfile.ZipFile(uploaded_file, "r") as zfile: + for file in zfile.infolist(): + # Remove folder pathing from str (e.g. "__MACOSX/" folder paths from archives created by macOS). + base_filename = os.path.basename(file.filename) - if any(file.filename.endswith(".archipelago") for file in infolist): - return Markup("Error: Your .zip file contains an .archipelago file. " - 'Did you mean to host a game?') - - for file in infolist: - if file.filename.endswith(banned_zip_contents): - return ("Uploaded data contained a rom file, " - "which is likely to contain copyrighted material. " - "Your file was deleted.") - elif file.filename.endswith((".yaml", ".json", ".yml", ".txt")): + if base_filename.endswith(".archipelago"): + return Markup("Error: Your .zip file contains an .archipelago file. " + 'Did you mean to host a game?') + elif base_filename.endswith(".zip"): + return "Nested .zip files inside a .zip are not supported." + elif banned_file(base_filename): + return ("Uploaded data contained a rom file, which is likely to contain copyrighted " + "material. Your file was deleted.") + # Ignore dot-files. + elif not base_filename.startswith(".") and allowed_options(base_filename): options[file.filename] = zfile.open(file, "r").read() else: options[uploaded_file.filename] = uploaded_file.read() + if not options: - return "Did not find a .yaml file to process." + return f"Did not find any valid files to process. Accepted formats: {allowed_options_extensions}" return options diff --git a/WebHostLib/upload.py b/WebHostLib/upload.py index e7ac0339..8f01294e 100644 --- a/WebHostLib/upload.py +++ b/WebHostLib/upload.py @@ -19,7 +19,22 @@ from worlds.Files import AutoPatchRegister from . import app from .models import Seed, Room, Slot, GameDataPackage -banned_zip_contents = (".sfc", ".z64", ".n64", ".sms", ".gb") +banned_extensions = (".sfc", ".z64", ".n64", ".nes", ".smc", ".sms", ".gb", ".gbc", ".gba") +allowed_options_extensions = (".yaml", ".json", ".yml", ".txt", ".zip") +allowed_generation_extensions = (".archipelago", ".zip") + + +def allowed_options(filename: str) -> bool: + return filename.endswith(allowed_options_extensions) + + +def allowed_generation(filename: str) -> bool: + return filename.endswith(allowed_generation_extensions) + + +def banned_file(filename: str) -> bool: + return filename.endswith(banned_extensions) + def process_multidata(compressed_multidata, files={}): decompressed_multidata = MultiServer.Context.decompress(compressed_multidata) @@ -61,8 +76,8 @@ def upload_zip_to_db(zfile: zipfile.ZipFile, owner=None, meta={"race": False}, s if not owner: owner = session["_id"] infolist = zfile.infolist() - if all(file.filename.endswith((".yaml", ".yml")) or file.is_dir() for file in infolist): - flash(Markup("Error: Your .zip file only contains .yaml files. " + if all(allowed_options(file.filename) or file.is_dir() for file in infolist): + flash(Markup("Error: Your .zip file only contains options files. " 'Did you mean to generate a game?')) return @@ -73,7 +88,7 @@ def upload_zip_to_db(zfile: zipfile.ZipFile, owner=None, meta={"race": False}, s # Load files. for file in infolist: handler = AutoPatchRegister.get_handler(file.filename) - if file.filename.endswith(banned_zip_contents): + if banned_file(file.filename): return "Uploaded data contained a rom file, which is likely to contain copyrighted material. " \ "Your file was deleted." @@ -136,35 +151,34 @@ def upload_zip_to_db(zfile: zipfile.ZipFile, owner=None, meta={"race": False}, s flash("No multidata was found in the zip file, which is required.") -@app.route('/uploads', methods=['GET', 'POST']) +@app.route("/uploads", methods=["GET", "POST"]) def uploads(): - if request.method == 'POST': - # check if the post request has the file part - if 'file' not in request.files: - flash('No file part') + if request.method == "POST": + # check if the POST request has a file part. + if "file" not in request.files: + flash("No file part in POST request.") else: - file = request.files['file'] - # if user does not select file, browser also - # submit an empty part without filename - if file.filename == '': - flash('No selected file') - elif file and allowed_file(file.filename): - if zipfile.is_zipfile(file): - with zipfile.ZipFile(file, 'r') as zfile: + uploaded_file = request.files["file"] + # If the user does not select file, the browser will still submit an empty string without a file name. + if uploaded_file.filename == "": + flash("No selected file.") + elif uploaded_file and allowed_generation(uploaded_file.filename): + if zipfile.is_zipfile(uploaded_file): + with zipfile.ZipFile(uploaded_file, "r") as zfile: try: res = upload_zip_to_db(zfile) except VersionException: flash(f"Could not load multidata. Wrong Version detected.") else: - if type(res) == str: + if res is str: return res elif res: return redirect(url_for("view_seed", seed=res.id)) else: - file.seek(0) # offset from is_zipfile check + uploaded_file.seek(0) # offset from is_zipfile check # noinspection PyBroadException try: - multidata = file.read() + multidata = uploaded_file.read() slots, multidata = process_multidata(multidata) except Exception as e: flash(f"Could not load multidata. File may be corrupted or incompatible. ({e})") @@ -182,7 +196,3 @@ def user_content(): rooms = select(room for room in Room if room.owner == session["_id"]) seeds = select(seed for seed in Seed if seed.owner == session["_id"]) return render_template("userContent.html", rooms=rooms, seeds=seeds) - - -def allowed_file(filename): - return filename.endswith(('.archipelago', ".zip"))