From 0943aeaa016a2eb4c893e5cad7c5aa819181806f Mon Sep 17 00:00:00 2001 From: Domingo Dirutigliano Date: Thu, 23 Jul 2026 18:38:22 +0200 Subject: [PATCH] enforced security over import backup, minor changes --- backend/app.py | 72 ++++++++++++++++++++++++++++++++--------- backend/utils/sqlite.py | 16 +++++++-- 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/backend/app.py b/backend/app.py index 11823d9..b934522 100644 --- a/backend/app.py +++ b/backend/app.py @@ -9,7 +9,7 @@ from fastapi import FastAPI, HTTPException, Depends, APIRouter from fastapi.security import OAuth2PasswordBearer, OAuth2PasswordRequestForm from jose import jwt from utils.sqlite import SQLite -from utils import API_VERSION, FIREGEX_PORT, FIREGEX_HOST, FIREGEX_SOCKET, JWT_ALGORITHM, get_interfaces, socketio_emit, DEBUG, SysctlManager, NORELOAD +from utils import API_VERSION, FIREGEX_PORT, FIREGEX_HOST, FIREGEX_SOCKET, JWT_ALGORITHM, get_interfaces, socketio_emit, DEBUG, SysctlManager, NORELOAD, safe_join from utils.loader import frontend_deploy, load_routers from utils.models import ChangePasswordModel, IpInterface, PasswordChangeForm, PasswordForm, ResetRequest, StatusModel, StatusMessageModel from contextlib import asynccontextmanager @@ -17,6 +17,8 @@ from fastapi.middleware.cors import CORSMiddleware import socketio from socketio.exceptions import ConnectionRefusedError import hashlib +import hmac +import re from ipaddress import ip_network, ip_address # DB init db = SQLite('db/firegex.db') @@ -125,9 +127,13 @@ async def hash_psw(psw: str) -> str: return await asyncio.to_thread(_hash_psw_sync, psw) def _verify_psw_sync(psw: str, hashed: str) -> bool: - psw_hash, salt = hashed.split("-") + try: + psw_hash, salt = hashed.split("-") + except (ValueError, AttributeError): + return False new_hashed = hashlib.pbkdf2_hmac("sha256", psw.encode(), salt.encode(), 500_000).hex() - return new_hashed == psw_hash + # Constant-time comparison to avoid leaking hash-match progress via timing. + return hmac.compare_digest(new_hashed, psw_hash) async def verify_psw(psw: str, hashed: str) -> bool: return await asyncio.to_thread(_verify_psw_sync, psw, hashed) @@ -297,9 +303,45 @@ async def export_db(): dbs['nfproxy_filters'][f] = base64.b64encode(script_file.read()).decode('utf-8') return dbs +# Backup entries are limited to plain basenames of the two shapes export_db() +# produces: ".db" databases and ".py" nfproxy filter files. The charset +# forbids path separators and "..", so a crafted key can't escape the target dir. +_SAFE_DB_NAME = re.compile(r'^[A-Za-z0-9_-]+\.db$') +_SAFE_PY_NAME = re.compile(r'^[A-Za-z0-9_-]+\.py$') + @api.post('/import', response_model=StatusMessageModel) async def import_db(data: dict): """Import all configuration databases from JSON""" + # Validate the ENTIRE payload before touching any file, so a malformed or + # malicious backup is rejected atomically instead of being half-applied. + db_imports = [] # (destination path, dump dict) + filter_imports = [] # (destination path, decoded bytes) + + for key, value in data.items(): + if not isinstance(key, str): + raise HTTPException(status_code=400, detail="Invalid backup: keys must be strings") + if _SAFE_DB_NAME.match(key): + if not isinstance(value, dict): + raise HTTPException(status_code=400, detail=f"Invalid backup: '{key}' must be an object") + # safe_join is defense-in-depth on top of the regex: it rejects any + # path that would resolve outside the db/ directory. + db_imports.append((safe_join('db', key), value)) + elif key == 'nfproxy_filters': + if not isinstance(value, dict): + raise HTTPException(status_code=400, detail="Invalid backup: 'nfproxy_filters' must be an object") + for fname, script_content in value.items(): + if not isinstance(fname, str) or not _SAFE_PY_NAME.match(fname): + raise HTTPException(status_code=400, detail=f"Invalid backup: illegal filter filename '{fname}'") + if not isinstance(script_content, str): + raise HTTPException(status_code=400, detail=f"Invalid backup: filter '{fname}' must be a base64 string") + try: + decoded = base64.b64decode(script_content, validate=True) + except ValueError: + raise HTTPException(status_code=400, detail=f"Invalid backup: filter '{fname}' is not valid base64") + filter_imports.append((safe_join('db/nfproxy_filters', fname), decoded)) + else: + raise HTTPException(status_code=400, detail=f"Invalid backup: unexpected entry '{key}'") + if not os.path.exists('db'): os.makedirs('db') @@ -308,19 +350,17 @@ async def import_db(data: dict): current_password = db.get("password") current_secret = db.get("secret") - for db_file, db_data in data.items(): - if db_file.endswith('.db'): - temp_db = SQLite(os.path.join('db', db_file)) - # Just load the data, we assume schemas exist or will be matched - # We don't delete the whole file to preserve DB_VERSION and schemas - temp_db.load(db_data) - elif db_file == 'nfproxy_filters': - if not os.path.exists('db/nfproxy_filters'): - os.makedirs('db/nfproxy_filters') - for f, script_content in db_data.items(): - if f.endswith('.py'): - with open(os.path.join('db/nfproxy_filters', f), 'wb') as script_file: - script_file.write(base64.b64decode(script_content)) + for db_path, db_data in db_imports: + temp_db = SQLite(str(db_path)) + # Load into the existing schema; SQLite.load only writes to tables/columns + # that actually exist, so unknown fields in the backup are ignored. + temp_db.load(db_data) + + if filter_imports: + os.makedirs('db/nfproxy_filters', exist_ok=True) + for filter_path, decoded in filter_imports: + with open(filter_path, 'wb') as script_file: + script_file.write(decoded) if current_password is not None: db.put("password", current_password) diff --git a/backend/utils/sqlite.py b/backend/utils/sqlite.py index 6c03ca2..a8a10ff 100644 --- a/backend/utils/sqlite.py +++ b/backend/utils/sqlite.py @@ -144,18 +144,28 @@ class SQLite(): try: cur.execute("BEGIN") for table_name, rows in data.items(): - # Verify table exists to prevent SQL injection or schema mismatches + # Verify the table exists to prevent SQL injection or schema mismatches cur.execute("SELECT name FROM sqlite_master WHERE type='table' AND name=?;", (table_name,)) if len(cur.fetchall()) == 0: continue + # Ignore anything that isn't a proper list of row objects + if not isinstance(rows, list) or not all(isinstance(r, dict) for r in rows): + continue cur.execute(f"DELETE FROM {table_name};") if rows: - cols = list(rows[0].keys()) + # Column names are identifiers and can't be parameterized, so only + # keep the ones that actually exist on the table - this stops a + # crafted backup from injecting arbitrary identifiers into the query. + cur.execute(f'PRAGMA table_info("{table_name}");') + valid_cols = {r["name"] for r in cur.fetchall()} + cols = [c for c in rows[0].keys() if c in valid_cols] + if not cols: + continue placeholders = ", ".join(["?"] * len(cols)) col_names = ", ".join(cols) query = f"INSERT INTO {table_name} ({col_names}) VALUES ({placeholders})" for row in rows: - cur.execute(query, [row[c] for c in cols]) + cur.execute(query, [row.get(c) for c in cols]) cur.execute("COMMIT") except Exception as e: cur.execute("ROLLBACK")