security(P0): close unauthenticated RCE chain
The local FastAPI server previously exposed macro create/execute over the
network with no authentication while bound to 0.0.0.0, and macro `app`
commands ran via subprocess with shell=True — an unauthenticated RCE for
anyone on the LAN (and, via the relay, the internet).
- Mandatory per-install auth token (secrets.token_urlsafe) required on all
/api/* routes and the /ws handshake; timing-safe hmac.compare_digest.
PWA shell + /static assets remain public to bootstrap.
- Token auto-generated and persisted in settings.json (now chmod 0600).
- Bind host is configurable (web.allow_lan); token is the security boundary.
- macro `app` launch no longer uses shell=True — commands are tokenized with
shlex and exec'd directly, killing ;/&&/|/$() shell chaining.
- Relay client injects the local token and only forwards /api/* paths, and no
longer forwards attacker-supplied headers to the local server.
- Fixed path traversal in /api/image/{path} (realpath-confined to macro_images).
- GUI embeds the token in the LAN URL/QR; PWA reads it and sends it on API,
WebSocket, and image requests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+14
-5
@@ -146,7 +146,12 @@ class MainWindow(QMainWindow):
|
||||
self.macro_manager = MacroManager(data_file, images_dir, app_dir)
|
||||
|
||||
# Initialize web server
|
||||
self.web_server = WebServer(self.macro_manager, app_dir, DEFAULT_PORT)
|
||||
token = self.settings_manager.get_web_auth_token()
|
||||
host = "0.0.0.0" if self.settings_manager.get_web_allow_lan() else "127.0.0.1"
|
||||
self.web_server = WebServer(
|
||||
self.macro_manager, app_dir, DEFAULT_PORT,
|
||||
auth_token=token, host=host
|
||||
)
|
||||
self.server_thread = None
|
||||
|
||||
# Relay client (initialized later if enabled)
|
||||
@@ -450,7 +455,10 @@ class MainWindow(QMainWindow):
|
||||
self.ip_label.setText(full_url)
|
||||
return
|
||||
|
||||
# Fall back to local IP
|
||||
# Fall back to local IP. Append the auth token as a query param so the
|
||||
# QR/copied URL lets a LAN device authenticate against the API.
|
||||
token = self.settings_manager.get_web_auth_token()
|
||||
token_qs = f"?token={token}" if token else ""
|
||||
try:
|
||||
import netifaces
|
||||
for iface in netifaces.interfaces():
|
||||
@@ -459,11 +467,11 @@ class MainWindow(QMainWindow):
|
||||
for addr in addrs[netifaces.AF_INET]:
|
||||
ip = addr.get('addr', '')
|
||||
if ip and not ip.startswith('127.'):
|
||||
self.ip_label.setText(f"http://{ip}:{DEFAULT_PORT}")
|
||||
self.ip_label.setText(f"http://{ip}:{DEFAULT_PORT}{token_qs}")
|
||||
return
|
||||
except Exception:
|
||||
pass
|
||||
self.ip_label.setText(f"http://localhost:{DEFAULT_PORT}")
|
||||
self.ip_label.setText(f"http://localhost:{DEFAULT_PORT}{token_qs}")
|
||||
|
||||
def copy_url_to_clipboard(self):
|
||||
"""Copy the web interface URL to clipboard."""
|
||||
@@ -703,7 +711,8 @@ class MainWindow(QMainWindow):
|
||||
local_port=DEFAULT_PORT,
|
||||
on_connected=self.on_relay_connected,
|
||||
on_disconnected=self.on_relay_disconnected,
|
||||
on_session_id=self.on_relay_session_id
|
||||
on_session_id=self.on_relay_session_id,
|
||||
local_auth_token=self.settings_manager.get_web_auth_token()
|
||||
)
|
||||
self.relay_client.start()
|
||||
self.status_bar.showMessage("Connecting to relay server...")
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import os
|
||||
import json
|
||||
import secrets
|
||||
from typing import Any, Optional
|
||||
|
||||
|
||||
@@ -12,6 +13,10 @@ DEFAULT_SETTINGS = {
|
||||
"session_id": None,
|
||||
"password": ""
|
||||
},
|
||||
"web": {
|
||||
"auth_token": "",
|
||||
"allow_lan": True
|
||||
},
|
||||
"minimize_to_tray": True
|
||||
}
|
||||
|
||||
@@ -36,12 +41,23 @@ class SettingsManager:
|
||||
# Merge with defaults to ensure all keys exist
|
||||
self.settings = self._merge_defaults(DEFAULT_SETTINGS, self.settings)
|
||||
|
||||
# Ensure a web auth token exists (this is the security boundary for API access)
|
||||
if not self.get('web.auth_token'):
|
||||
token = secrets.token_urlsafe(32)
|
||||
self.set('web.auth_token', token)
|
||||
self.save()
|
||||
|
||||
def save(self):
|
||||
"""Save settings to file."""
|
||||
try:
|
||||
os.makedirs(os.path.dirname(self.settings_file), exist_ok=True)
|
||||
with open(self.settings_file, 'w', encoding='utf-8') as f:
|
||||
json.dump(self.settings, f, indent=2)
|
||||
# Best-effort restrict permissions (protects stored relay password + token)
|
||||
try:
|
||||
os.chmod(self.settings_file, 0o600)
|
||||
except OSError:
|
||||
pass
|
||||
return True
|
||||
except IOError:
|
||||
return False
|
||||
@@ -101,6 +117,14 @@ class SettingsManager:
|
||||
self.set('relay.session_id', session_id)
|
||||
self.save()
|
||||
|
||||
def get_web_auth_token(self) -> str:
|
||||
"""Get the web API auth token."""
|
||||
return self.get('web.auth_token', '')
|
||||
|
||||
def get_web_allow_lan(self) -> bool:
|
||||
"""Check if LAN (0.0.0.0) binding is allowed."""
|
||||
return self.get('web.allow_lan', True)
|
||||
|
||||
def get_minimize_to_tray(self) -> bool:
|
||||
"""Check if minimize to tray is enabled."""
|
||||
return self.get('minimize_to_tray', True)
|
||||
|
||||
+11
-2
@@ -3,6 +3,7 @@
|
||||
import copy
|
||||
import json
|
||||
import os
|
||||
import shlex
|
||||
import uuid
|
||||
import pyautogui
|
||||
import subprocess
|
||||
@@ -316,10 +317,18 @@ class MacroManager:
|
||||
time.sleep(ms / 1000.0)
|
||||
|
||||
elif cmd_type == "app":
|
||||
# Launch application
|
||||
# Launch application.
|
||||
# SECURITY: shell=True was removed to kill shell-metacharacter
|
||||
# injection (no ; && | $() chaining). We tokenize the command
|
||||
# and exec the program directly without a shell.
|
||||
command = cmd.get("command", "")
|
||||
if command:
|
||||
subprocess.Popen(command, shell=True)
|
||||
try:
|
||||
args = shlex.split(command, posix=(os.name != "nt"))
|
||||
if args:
|
||||
subprocess.Popen(args)
|
||||
except Exception as e:
|
||||
print(f"Error launching app command: {e}")
|
||||
|
||||
# Legacy API compatibility methods
|
||||
def add_macro_legacy(
|
||||
|
||||
+22
-3
@@ -20,7 +20,8 @@ class RelayClient:
|
||||
local_port: int = 40000,
|
||||
on_connected: Optional[Callable] = None,
|
||||
on_disconnected: Optional[Callable] = None,
|
||||
on_session_id: Optional[Callable[[str], None]] = None
|
||||
on_session_id: Optional[Callable[[str], None]] = None,
|
||||
local_auth_token: str = ""
|
||||
):
|
||||
self.relay_url = relay_url.rstrip('/')
|
||||
if not self.relay_url.endswith('/desktop'):
|
||||
@@ -28,6 +29,7 @@ class RelayClient:
|
||||
self.password = password
|
||||
self.session_id = session_id
|
||||
self.local_url = f"http://localhost:{local_port}"
|
||||
self.local_auth_token = local_auth_token
|
||||
|
||||
# Callbacks
|
||||
self.on_connected = on_connected
|
||||
@@ -174,16 +176,33 @@ class RelayClient:
|
||||
method = msg.get("method", "GET").upper()
|
||||
path = msg.get("path", "/")
|
||||
body = msg.get("body")
|
||||
headers = msg.get("headers", {})
|
||||
|
||||
# Restrict the relay to API routes only. This prevents the relay from
|
||||
# reaching static/other paths on the local server.
|
||||
if not path.startswith("/api/"):
|
||||
await self._ws.send_json({
|
||||
"type": "api_response",
|
||||
"requestId": request_id,
|
||||
"status": 403,
|
||||
"body": {"error": "forbidden path"}
|
||||
})
|
||||
return
|
||||
|
||||
url = f"{self.local_url}{path}"
|
||||
|
||||
# Build a fresh headers dict. We do NOT forward attacker-supplied
|
||||
# headers from the relay; we inject the local auth token ourselves.
|
||||
sending_body = bool(body) and method in ("POST", "PUT", "PATCH")
|
||||
headers = {"X-MacroPad-Token": self.local_auth_token}
|
||||
if sending_body:
|
||||
headers["Content-Type"] = "application/json"
|
||||
|
||||
try:
|
||||
# Forward request to local server
|
||||
async with self._session.request(
|
||||
method,
|
||||
url,
|
||||
json=body if body and method in ("POST", "PUT", "PATCH") else None,
|
||||
json=body if sending_body else None,
|
||||
headers=headers
|
||||
) as response:
|
||||
# Handle binary responses (images)
|
||||
|
||||
+26
-1
@@ -12,11 +12,14 @@ class MacroPadApp {
|
||||
this.relayMode = this.detectRelayMode();
|
||||
this.sessionId = null;
|
||||
this.password = null;
|
||||
this.localToken = null;
|
||||
this.desktopConnected = true;
|
||||
this.wsAuthenticated = false;
|
||||
|
||||
if (this.relayMode) {
|
||||
this.initRelayMode();
|
||||
} else {
|
||||
this.initLocalMode();
|
||||
}
|
||||
|
||||
this.init();
|
||||
@@ -51,6 +54,20 @@ class MacroPadApp {
|
||||
console.log('Relay mode enabled, session:', this.sessionId);
|
||||
}
|
||||
|
||||
initLocalMode() {
|
||||
// Get token from URL query param or localStorage (local/LAN mode)
|
||||
const urlParams = new URLSearchParams(window.location.search);
|
||||
this.localToken = urlParams.get('token') || localStorage.getItem('macropad_local_token');
|
||||
|
||||
if (this.localToken) {
|
||||
// Persist token and strip it from the URL for cleanliness
|
||||
localStorage.setItem('macropad_local_token', this.localToken);
|
||||
if (urlParams.has('token')) {
|
||||
window.history.replaceState({}, '', window.location.pathname);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
getApiUrl(path) {
|
||||
if (this.relayMode && this.sessionId) {
|
||||
return `/${this.sessionId}${path}`;
|
||||
@@ -62,6 +79,8 @@ class MacroPadApp {
|
||||
const headers = { 'Content-Type': 'application/json' };
|
||||
if (this.relayMode && this.password) {
|
||||
headers['X-MacroPad-Password'] = this.password;
|
||||
} else if (!this.relayMode && this.localToken) {
|
||||
headers['X-MacroPad-Token'] = this.localToken;
|
||||
}
|
||||
return headers;
|
||||
}
|
||||
@@ -176,6 +195,10 @@ class MacroPadApp {
|
||||
wsUrl = `${protocol}//${window.location.host}/${this.sessionId}/ws`;
|
||||
} else {
|
||||
wsUrl = `${protocol}//${window.location.host}/ws`;
|
||||
// Append token as query param (WebSocket can't set headers)
|
||||
if (this.localToken) {
|
||||
wsUrl += `?token=${encodeURIComponent(this.localToken)}`;
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
@@ -311,9 +334,11 @@ class MacroPadApp {
|
||||
let imageSrc = null;
|
||||
if (macro.image_path) {
|
||||
const basePath = this.getApiUrl(`/api/image/${macro.image_path}`);
|
||||
// Add password as query param for relay mode (img tags can't use headers)
|
||||
// Add credential as query param since img tags can't use headers
|
||||
if (this.relayMode && this.password) {
|
||||
imageSrc = `${basePath}?password=${encodeURIComponent(this.password)}`;
|
||||
} else if (!this.relayMode && this.localToken) {
|
||||
imageSrc = `${basePath}?token=${encodeURIComponent(this.localToken)}`;
|
||||
} else {
|
||||
imageSrc = basePath;
|
||||
}
|
||||
|
||||
+42
-9
@@ -2,13 +2,14 @@
|
||||
|
||||
import os
|
||||
import sys
|
||||
import hmac
|
||||
import asyncio
|
||||
from typing import List, Optional
|
||||
from contextlib import asynccontextmanager
|
||||
|
||||
from fastapi import FastAPI, WebSocket, WebSocketDisconnect, UploadFile, File, HTTPException
|
||||
from fastapi import FastAPI, WebSocket, WebSocketDisconnect, UploadFile, File, HTTPException, Request
|
||||
from fastapi.staticfiles import StaticFiles
|
||||
from fastapi.responses import FileResponse, HTMLResponse
|
||||
from fastapi.responses import FileResponse, HTMLResponse, JSONResponse
|
||||
from pydantic import BaseModel
|
||||
import uvicorn
|
||||
|
||||
@@ -83,10 +84,13 @@ class ConnectionManager:
|
||||
class WebServer:
|
||||
"""FastAPI-based web server for MacroPad."""
|
||||
|
||||
def __init__(self, macro_manager, app_dir: str, port: int = DEFAULT_PORT):
|
||||
def __init__(self, macro_manager, app_dir: str, port: int = DEFAULT_PORT,
|
||||
auth_token: str = "", host: str = "127.0.0.1"):
|
||||
self.macro_manager = macro_manager
|
||||
self.app_dir = app_dir
|
||||
self.port = port
|
||||
self.auth_token = auth_token
|
||||
self.host = host
|
||||
self.app = None
|
||||
self.manager = ConnectionManager()
|
||||
self.server = None
|
||||
@@ -114,6 +118,25 @@ class WebServer:
|
||||
if os.path.exists(images_dir):
|
||||
app.mount("/images", StaticFiles(directory=images_dir), name="images")
|
||||
|
||||
# Paths served without a token (PWA shell + static assets)
|
||||
public_paths = {"/", "/manifest.json", "/service-worker.js"}
|
||||
|
||||
@app.middleware("http")
|
||||
async def auth_middleware(request: Request, call_next):
|
||||
"""Require a valid auth token for all non-public paths."""
|
||||
path = request.url.path
|
||||
if path in public_paths or path.startswith("/static/"):
|
||||
return await call_next(request)
|
||||
|
||||
# Fail-open only when no token is configured (shouldn't happen in prod)
|
||||
if self.auth_token:
|
||||
provided = request.headers.get("X-MacroPad-Token") \
|
||||
or request.query_params.get("token") or ""
|
||||
if not hmac.compare_digest(provided, self.auth_token):
|
||||
return JSONResponse({"detail": "Unauthorized"}, status_code=401)
|
||||
|
||||
return await call_next(request)
|
||||
|
||||
@app.get("/", response_class=HTMLResponse)
|
||||
async def index():
|
||||
"""Serve the main PWA page."""
|
||||
@@ -231,15 +254,25 @@ class WebServer:
|
||||
|
||||
@app.get("/api/image/{image_path:path}")
|
||||
async def get_image(image_path: str):
|
||||
"""Get macro image (legacy compatibility)."""
|
||||
full_path = os.path.join(self.app_dir, image_path)
|
||||
if os.path.exists(full_path):
|
||||
return FileResponse(full_path)
|
||||
"""Get macro image (legacy compatibility, path-traversal safe)."""
|
||||
images_dir = os.path.join(self.app_dir, "macro_images")
|
||||
requested = os.path.realpath(os.path.join(self.app_dir, image_path))
|
||||
images_real = os.path.realpath(images_dir)
|
||||
# Only serve files that resolve to inside the macro_images directory
|
||||
if (os.path.commonpath([requested, images_real]) == images_real
|
||||
and os.path.isfile(requested)):
|
||||
return FileResponse(requested)
|
||||
raise HTTPException(status_code=404, detail="Image not found")
|
||||
|
||||
@app.websocket("/ws")
|
||||
async def websocket_endpoint(websocket: WebSocket):
|
||||
"""WebSocket for real-time updates."""
|
||||
# Require a valid token before accepting the connection
|
||||
if self.auth_token:
|
||||
provided = websocket.query_params.get("token") or ""
|
||||
if not hmac.compare_digest(provided, self.auth_token):
|
||||
await websocket.close(code=1008)
|
||||
return
|
||||
await self.manager.connect(websocket)
|
||||
try:
|
||||
while True:
|
||||
@@ -260,7 +293,7 @@ class WebServer:
|
||||
|
||||
config = uvicorn.Config(
|
||||
self.app,
|
||||
host="0.0.0.0",
|
||||
host=self.host,
|
||||
port=self.port,
|
||||
log_level="warning",
|
||||
log_config=None # Disable default logging config for PyInstaller compatibility
|
||||
@@ -280,7 +313,7 @@ class WebServer:
|
||||
|
||||
config = uvicorn.Config(
|
||||
self.app,
|
||||
host="0.0.0.0",
|
||||
host=self.host,
|
||||
port=self.port,
|
||||
log_level="warning",
|
||||
log_config=None # Disable default logging config for PyInstaller compatibility
|
||||
|
||||
Reference in New Issue
Block a user