From 0c32e6073f4fb4ff85306c678aee3ce3b4719c6b Mon Sep 17 00:00:00 2001 From: mahemium Date: Fri, 20 Feb 2026 17:57:17 +0300 Subject: [PATCH] Fix bugs --- src/dashboard.py | 30 ++--- src/modules/AmneziaConfiguration.py | 21 +++- src/modules/AmneziaPeer.py | 26 ++-- src/modules/Peer.py | 49 +++++--- src/modules/WireguardConfiguration.py | 111 +++++++++++------- .../backupRestoreComponents/backup.vue | 9 +- 6 files changed, 150 insertions(+), 96 deletions(-) diff --git a/src/dashboard.py b/src/dashboard.py index dd3b466b..b13fabe7 100644 --- a/src/dashboard.py +++ b/src/dashboard.py @@ -8,7 +8,7 @@ from datetime import datetime, timedelta import sqlalchemy from jinja2 import Template -from flask import Flask, request, render_template, session, send_file +from flask import Flask, request, render_template, session, send_file, current_app from flask_cors import CORS from icmplib import ping, traceroute from flask.json.provider import DefaultJSONProvider @@ -254,7 +254,6 @@ def auth_req(): whiteList = [ '/static/', 'validateAuthentication', 'authenticate', 'getDashboardConfiguration', 'getDashboardTheme', 'getDashboardVersion', 'sharePeer/get', 'isTotpEnabled', 'locale', - '/fileDownload', '/client', '/assets/', '/img/', '/json/', '/client/assets/', '/client/img/' @@ -608,12 +607,19 @@ def API_deleteWireguardConfigurationBackup(): @app.get(f'{APP_PREFIX}/api/downloadWireguardConfigurationBackup') def API_downloadWireguardConfigurationBackup(): - configurationName = request.args.get('configurationName') - backupFileName = request.args.get('backupFileName') + configurationName = os.path.basename(request.args.get('configurationName')) + backupFileName = os.path.basename(request.args.get('backupFileName')) + if configurationName is None or configurationName not in WireguardConfigurations.keys(): return ResponseObject(False, "Configuration does not exist", status_code=404) + status, zip = WireguardConfigurations[configurationName].downloadBackup(backupFileName) - return ResponseObject(status, data=zip, status_code=(200 if status else 404)) + + if not status: + current_app.logger.error(f"Failed to download a requested backup.\nConfiguration Name: {configurationName}\nBackup File Name: {backupFileName}") + return ResponseObject(False, "Internal server error", status_code=500) + + return send_file(os.path.join('download', zip), as_attachment=True) @app.post(f'{APP_PREFIX}/api/restoreWireguardConfigurationBackup') def API_restoreWireguardConfigurationBackup(): @@ -1241,20 +1247,6 @@ def API_getPeerScheduleJobLogs(configName): requestAll = True return ResponseObject(data=AllPeerJobs.getPeerJobLogs(configName)) -''' -File Download -''' -@app.get(f'{APP_PREFIX}/fileDownload') -def API_download(): - file = request.args.get('file') - if file is None or len(file) == 0: - return ResponseObject(False, "Please specify a file") - if os.path.exists(os.path.join('download', file)): - return send_file(os.path.join('download', file), as_attachment=True) - else: - return ResponseObject(False, "File does not exist") - - ''' Tools ''' diff --git a/src/modules/AmneziaConfiguration.py b/src/modules/AmneziaConfiguration.py index 871369bf..898bb500 100644 --- a/src/modules/AmneziaConfiguration.py +++ b/src/modules/AmneziaConfiguration.py @@ -276,13 +276,22 @@ class AmneziaConfiguration(WireguardConfiguration): with open(uid, "w+") as f: f.write(p['preshared_key']) - subprocess.check_output( - f"{self.Protocol} set {self.Name} peer {p['id']} allowed-ips {p['allowed_ip'].replace(' ', '')}{f' preshared-key {uid}' if presharedKeyExist else ''}", - shell=True, stderr=subprocess.STDOUT) + newAllowedIPs = p['allowed_ip'].replace(" ", "") + if not re.match(r"^[0-9a-fA-F\.\,:/ ]+$", newAllowedIPs): + return False, [], "Allowed IPs entry format is incorrect" + + if not re.match(r"^[A-Za-z0-9+/]{42}[A-Ea-e0-9]=$", p["id"]): + return False, [], "Peer key format is incorrect" + + command = [self.Protocol, "set", self.Name, "peer", p['id'], "allowed-ips", newAllowedIPs, "preshared-key", uid if presharedKeyExist else ""] + subprocess.check_output(command, stderr=subprocess.STDOUT) + if presharedKeyExist: os.remove(uid) - subprocess.check_output( - f"{self.Protocol}-quick save {self.Name}", shell=True, stderr=subprocess.STDOUT) + + command = [f"{self.Protocol}-quick", "save", self.Name] + subprocess.check_output(command, stderr=subprocess.STDOUT) + self.getPeers() for p in peers: p = self.searchPeer(p['id']) @@ -294,7 +303,7 @@ class AmneziaConfiguration(WireguardConfiguration): }) except Exception as e: current_app.logger.error("Add peers error", e) - return False, [], str(e) + return False, [], "Internal server error" return True, result['peers'], "" def getRestrictedPeers(self): diff --git a/src/modules/AmneziaPeer.py b/src/modules/AmneziaPeer.py index 5a99257a..4dae3826 100644 --- a/src/modules/AmneziaPeer.py +++ b/src/modules/AmneziaPeer.py @@ -1,4 +1,5 @@ import os +from flask import current_app import random import re import subprocess @@ -77,18 +78,26 @@ class AmneziaPeer(Peer): f.write(preshared_key) newAllowedIPs = allowed_ip.replace(" ", "") - updateAllowedIp = subprocess.check_output( - f"{self.configuration.Protocol} set {self.configuration.Name} peer {self.id} allowed-ips {newAllowedIPs} {f'preshared-key {uid}' if psk_exist else 'preshared-key /dev/null'}", - shell=True, stderr=subprocess.STDOUT) + + if not re.match(r"^[0-9a-fA-F\.\,:/ ]+$", newAllowedIPs): + return False, "Allowed IPs entry format is incorrect" + + command = [self.configuration.Protocol, "set", self.configuration.Name, "peer", self.id, "allowed-ips", newAllowedIPs, "preshared-key", uid if psk_exist else "/dev/null"] + + updateAllowedIp = subprocess.check_output(command, stderr=subprocess.STDOUT) if psk_exist: os.remove(uid) if len(updateAllowedIp.decode().strip("\n")) != 0: - return False, "Update peer failed when updating Allowed IPs" - saveConfig = subprocess.check_output(f"{self.configuration.Protocol}-quick save {self.configuration.Name}", - shell=True, stderr=subprocess.STDOUT) + current_app.logger.error(f"Update peer failed when updating Allowed IPs.\nInput: {newAllowedIPs}\nOutput: {updateAllowedIp.decode().strip('\n')}") + return False, "Internal server error" + + command = [f"{self.configuration.Protocol}-quick", "save", self.configuration.Name] + saveConfig = subprocess.check_output(command, stderr=subprocess.STDOUT) + if f"wg showconf {self.configuration.Name}" not in saveConfig.decode().strip('\n'): - return False, "Update peer failed when saving the configuration" + current_app.logger.error("Update peer failed when saving the configuration") + return False, "Internal server error" with self.configuration.engine.begin() as conn: conn.execute( @@ -108,4 +117,5 @@ class AmneziaPeer(Peer): self.configuration.getPeers() return True, None except subprocess.CalledProcessError as exc: - return False, exc.output.decode("UTF-8").strip() \ No newline at end of file + current_app.logger.error(f"Subprocess call failed:\n{exc.output.decode("UTF-8")}") + return False, "Internal server error" diff --git a/src/modules/Peer.py b/src/modules/Peer.py index bba16deb..b7d31d9b 100644 --- a/src/modules/Peer.py +++ b/src/modules/Peer.py @@ -10,6 +10,7 @@ from datetime import timedelta import jinja2 import sqlalchemy as db from .PeerJob import PeerJob +from flask import current_app from .PeerShareLink import PeerShareLink from .Utilities import GenerateWireguardPublicKey, CheckAddress, ValidateDNSAddress @@ -114,17 +115,25 @@ class Peer: f.write(preshared_key) newAllowedIPs = allowed_ip.replace(" ", "") - updateAllowedIp = subprocess.check_output( - f"{self.configuration.Protocol} set {self.configuration.Name} peer {self.id} allowed-ips {newAllowedIPs} {f'preshared-key {uid}' if psk_exist else 'preshared-key /dev/null'}", - shell=True, stderr=subprocess.STDOUT) + if not re.match(r"^[0-9a-fA-F\.\,:/ ]+$", newAllowedIPs): + return False, "Allowed IPs entry format is incorrect" + + command = [self.configuration.Protocol, "set", self.configuration.Name, "peer", self.id, "allowed-ips", newAllowedIPs, "preshared-key", uid if psk_exist else "/dev/null"] + updateAllowedIp = subprocess.check_output(command, stderr=subprocess.STDOUT) if psk_exist: os.remove(uid) + if len(updateAllowedIp.decode().strip("\n")) != 0: - return False, "Update peer failed when updating Allowed IPs" - saveConfig = subprocess.check_output(f"{self.configuration.Protocol}-quick save {self.configuration.Name}", - shell=True, stderr=subprocess.STDOUT) + current_app.logger.error("Update peer failed when updating Allowed IPs") + return False, "Internal server error" + + command = [f"{self.configuration.Protocol}-quick", "save", self.configuration.Name] + saveConfig = subprocess.check_output(command, stderr=subprocess.STDOUT) + if f"wg showconf {self.configuration.Name}" not in saveConfig.decode().strip('\n'): - return False, "Update peer failed when saving the configuration" + current_app.logger.error("Update peer failed when saving the configuration") + return False, "Internal server error" + with self.configuration.engine.begin() as conn: conn.execute( self.configuration.peersTable.update().values({ @@ -142,7 +151,8 @@ class Peer: ) return True, None except subprocess.CalledProcessError as exc: - return False, exc.output.decode("UTF-8").strip() + current_app.logger.error(f"Subprocess call failed:\n{exc.output.decode("UTF-8")}") + return False, "Internal server error" def downloadPeer(self) -> dict[str, str]: final = { @@ -153,12 +163,23 @@ class Peer: if len(filename) == 0: filename = "UntitledPeer" filename = "".join(filename.split(' ')) - filename = f"{filename}" - illegal_filename = [".", ",", "/", "?", "<", ">", "\\", ":", "*", '|' '\"', "com1", "com2", "com3", - "com4", "com5", "com6", "com7", "com8", "com9", "lpt1", "lpt2", "lpt3", "lpt4", - "lpt5", "lpt6", "lpt7", "lpt8", "lpt9", "con", "nul", "prn"] - for i in illegal_filename: - filename = filename.replace(i, "") + +# filename = f"{filename}" +# illegal_filename = [".", ",", "/", "?", "<", ">", "\\", ":", "*", '|' '\"', "com1", "com2", "com3", +# "com4", "com5", "com6", "com7", "com8", "com9", "lpt1", "lpt2", "lpt3", "lpt4", +# "lpt5", "lpt6", "lpt7", "lpt8", "lpt9", "con", "nul", "prn"] +# for i in illegal_filename: +# filename = filename.replace(i, "") + + # Previous filtering is flawed, the filter is insufficient. (com?5 -> com5, or comcom5 -> com5) + # If this new filter is not working as expected, use the previous code to resolve it. + + filename = re.sub(r'[.,/?<>\\:*|"]', '', filename).rstrip(". ") + + reserved_pattern = r"^(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])(\..*)?$" + + if re.match(reserved_pattern, filename, re.IGNORECASE): + filename = f"file_{filename}" for i in filename: if re.match("^[a-zA-Z0-9_=+.-]$", i): diff --git a/src/modules/WireguardConfiguration.py b/src/modules/WireguardConfiguration.py index 59785203..939ebfe5 100644 --- a/src/modules/WireguardConfiguration.py +++ b/src/modules/WireguardConfiguration.py @@ -545,12 +545,22 @@ class WireguardConfiguration: with open(uid, "w+") as f: f.write(p['preshared_key']) - subprocess.check_output(f"{self.Protocol} set {self.Name} peer {p['id']} allowed-ips {p['allowed_ip'].replace(' ', '')}{f' preshared-key {uid}' if presharedKeyExist else ''}", - shell=True, stderr=subprocess.STDOUT) + newAllowedIPs = p['allowed_ip'].replace(" ", "") + if not re.match(r"^[0-9a-fA-F\.\,:/ ]+$", newAllowedIPs): + return False, [], "Allowed IPs entry format is incorrect" + + if not re.match(r"^[A-Za-z0-9+/]{42}[A-Ea-e0-9]=$", p["id"]): + return False, [], "Peer key format is incorrect" + + command = [self.Protocol, "set", self.Name, "peer", p['id'], "allowed-ips", newAllowedIPs, "preshared-key", uid if presharedKeyExist else ""] + subprocess.check_output(command, stderr=subprocess.STDOUT) + if presharedKeyExist: os.remove(uid) - subprocess.check_output( - f"{self.Protocol}-quick save {self.Name}", shell=True, stderr=subprocess.STDOUT) + + command = [f"{self.Protocol}-quick", "save", self.Name] + subprocess.check_output(command, stderr=subprocess.STDOUT) + self.getPeers() for p in peers: p = self.searchPeer(p['id']) @@ -562,7 +572,7 @@ class WireguardConfiguration: }) except Exception as e: current_app.logger.error("Add peers error", e) - return False, [], str(e) + return False, [], "Internal server error" return True, result['peers'], "" def searchPeer(self, publicKey): @@ -600,8 +610,16 @@ class WireguardConfiguration: with open(uid, "w+") as f: f.write(restrictedPeer['preshared_key']) - subprocess.check_output(f"{self.Protocol} set {self.Name} peer {restrictedPeer['id']} allowed-ips {restrictedPeer['allowed_ip'].replace(' ', '')}{f' preshared-key {uid}' if presharedKeyExist else ''}", - shell=True, stderr=subprocess.STDOUT) + newAllowedIPs = restrictedPeer['allowed_ip'].replace(" ", "") + if not re.match(r"^[0-9a-fA-F\.\,:/ ]+$", newAllowedIPs): + return False, "Allowed IPs entry format is incorrect" + + if not re.match(r"^[A-Za-z0-9+/]{42}[A-Ea-e0-9]=$", restrictedPeer["id"]): + return False, "Peer key format is incorrect" + + command = [self.Protocol, "set", self.Name, "peer", restrictedPeer["id"], "allowed-ips", newAllowedIPs, "preshared-key", uid if presharedKeyExist else ""] + subprocess.check_output(command, stderr=subprocess.STDOUT) + if presharedKeyExist: os.remove(uid) else: return False, "Failed to allow access of peer " + i @@ -621,8 +639,9 @@ class WireguardConfiguration: found, pf = self.searchPeer(p) if found: try: - subprocess.check_output(f"{self.Protocol} set {self.Name} peer {pf.id} remove", - shell=True, stderr=subprocess.STDOUT) + command = [self.Protocol, "set", self.Name, "peer", pf.id, "remove"] + subprocess.check_output(command, stderr=subprocess.STDOUT) + conn.execute( self.peersRestrictedTable.insert().from_select( [c.name for c in self.peersTable.columns], @@ -703,17 +722,20 @@ class WireguardConfiguration: def __wgSave(self) -> tuple[bool, str] | tuple[bool, None]: try: - subprocess.check_output(f"{self.Protocol}-quick save {self.Name}", shell=True, stderr=subprocess.STDOUT) + command = [f"{self.Protocol}-quick", "save", self.Name] + subprocess.check_output(command, stderr=subprocess.STDOUT) + return True, None except subprocess.CalledProcessError as e: - return False, str(e) + current_app.logger.error(f"Failed to process command:\n{str(e)}") + return False, "Internal server error" def getPeersLatestHandshake(self): if not self.getStatus(): self.toggleConfiguration() try: - latestHandshake = subprocess.check_output(f"{self.Protocol} show {self.Name} latest-handshakes", - shell=True, stderr=subprocess.STDOUT) + command = [self.Protocol, "show", self.Name, "latest-handshakes"] + latestHandshake = subprocess.check_output(command, stderr=subprocess.STDOUT) except subprocess.CalledProcessError: return "stopped" latestHandshake = latestHandshake.decode("UTF-8").split() @@ -752,8 +774,9 @@ class WireguardConfiguration: if not self.getStatus(): self.toggleConfiguration() # try: - data_usage = subprocess.check_output(f"{self.Protocol} show {self.Name} transfer", - shell=True, stderr=subprocess.STDOUT) + command = [self.Protocol, "show", self.Name, "transfer"] + data_usage = subprocess.check_output(command, stderr=subprocess.STDOUT) + data_usage = data_usage.decode("UTF-8").split("\n") data_usage = [p.split("\t") for p in data_usage] @@ -808,10 +831,11 @@ class WireguardConfiguration: if not self.getStatus(): self.toggleConfiguration() try: - data_usage = subprocess.check_output(f"{self.Protocol} show {self.Name} endpoints", - shell=True, stderr=subprocess.STDOUT) + command = [self.Protocol, "show", self.Name, "endpoints"] + data_usage = subprocess.check_output(command, stderr=subprocess.STDOUT) except subprocess.CalledProcessError: return "stopped" + data_usage = data_usage.decode("UTF-8").split() count = 0 with self.engine.begin() as conn: @@ -829,14 +853,17 @@ class WireguardConfiguration: self.getStatus() if self.Status: try: - check = subprocess.check_output(f"{self.Protocol}-quick down {self.Name}", - shell=True, stderr=subprocess.STDOUT) + command = [f"{self.Protocol}-quick", "down", self.Name] + check = subprocess.check_output(command, stderr=subprocess.STDOUT) + self.removeAutostart() except subprocess.CalledProcessError as exc: return False, str(exc.output.strip().decode("utf-8")) else: try: - check = subprocess.check_output(f"{self.Protocol}-quick up {self.Name}", shell=True, stderr=subprocess.STDOUT) + command = [f"{self.Protocol}-quick", "up", self.Name] + check = subprocess.check_output(command, stderr=subprocess.STDOUT) + self.addAutostart() except subprocess.CalledProcessError as exc: return False, str(exc.output.strip().decode("utf-8")) @@ -1015,31 +1042,33 @@ class WireguardConfiguration: return True def renameConfiguration(self, newConfigurationName) -> tuple[bool, str]: + newConfigurationName = os.path.basename(newConfigurationName) + + if len(newConfigurationName) > 15 or not re.match(r'^[a-zA-Z0-9_=\+\.\-]{1,15}$', newConfigurationName): + return False, "Configuration name is either too long or contains an illegal character" + + newConfigurationName = newConfigurationName.replace("`", "") # double check + try: if self.getStatus(): self.toggleConfiguration() self.createDatabase(newConfigurationName) with self.engine.begin() as conn: - conn.execute( - sqlalchemy.text( - f'INSERT INTO "{newConfigurationName}" SELECT * FROM "{self.Name}"' + def doRenameStatement(suffix): + newConfig = f"{newConfigurationName}{suffix}" + oldConfig = f"{self.Name}{suffix}" + + conn.execute( + sqlalchemy.text( + f'INSERT INTO `{newConfig}` SELECT * FROM `{oldConfig}`' + ) ) - ) - conn.execute( - sqlalchemy.text( - f'INSERT INTO "{newConfigurationName}_restrict_access" SELECT * FROM "{self.Name}_restrict_access"' - ) - ) - conn.execute( - sqlalchemy.text( - f'INSERT INTO "{newConfigurationName}_deleted" SELECT * FROM "{self.Name}_deleted"' - ) - ) - conn.execute( - sqlalchemy.text( - f'INSERT INTO "{newConfigurationName}_transfer" SELECT * FROM "{self.Name}_transfer"' - ) - ) + + doRenameStatement("") + doRenameStatement("_restrict_access") + doRenameStatement("_deleted") + doRenameStatement("_transfer") + self.AllPeerJobs.updateJobConfigurationName(self.Name, newConfigurationName) shutil.copy( self.configPath, @@ -1047,8 +1076,8 @@ class WireguardConfiguration: ) self.deleteConfiguration() except Exception as e: - traceback.print_stack() - return False, str(e) + current_app.logger.error(f"Failed to rename configuration.\nNew Configuration Name: {newConfigurationName}\nError: {str(e)}") + return False, "Internal server error" return True, None def getNumberOfAvailableIP(self): diff --git a/src/static/app/src/components/configurationComponents/backupRestoreComponents/backup.vue b/src/static/app/src/components/configurationComponents/backupRestoreComponents/backup.vue index 8aceffef..6cf31e9e 100644 --- a/src/static/app/src/components/configurationComponents/backupRestoreComponents/backup.vue +++ b/src/static/app/src/components/configurationComponents/backupRestoreComponents/backup.vue @@ -46,14 +46,7 @@ const restoreBackup = () => { } const downloadBackup = () => { - fetchGet("/api/downloadWireguardConfigurationBackup", { - configurationName: route.params.id, - backupFileName: props.b.filename - }, (res) => { - if (res.status){ - window.open(getUrl(`/fileDownload?file=${res.data}`), '_blank') - } - }) + window.location.href = getUrl(`/api/downloadWireguardConfigurationBackup?configurationName=${route.params.id}&backupFileName=${props.b.filename}`); } const delaySeconds = computed(() => {