Skip to content

Commit ec0d71a

Browse files
committed
security: pentest fixes — auth middleware, password policy, security headers
Findings from internal Kali pentest (2026-03-16) against v0.30.0-beta: - F-03: Add /api/v1/auth/ exemption to API-key middleware (auth.py) so UI login/setup/status endpoints are reachable when SUBLARR_API_KEY is set. Without this, the web UI login flow was blocked by the API-key middleware before the UI-auth handler could run. - F-06: Raise minimum password length from 4 → 12 characters (auth_ui.py) - F-07: Add X-Frame-Options: DENY, X-Content-Type-Options: nosniff and Referrer-Policy: same-origin headers to all responses via after_request hook (app.py) - F-04: Gate version and service topology in /api/v1/health behind auth. Unauthenticated callers (uptime monitors, scanners) now only receive {"status": "healthy/unhealthy"} without version string or integration details. Full findings documented in PENTEST_FINDINGS.md (root of repo).
1 parent 063a700 commit ec0d71a

File tree

5 files changed

+291
-8
lines changed

5 files changed

+291
-8
lines changed

PENTEST_FINDINGS.md

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
# Sublarr Penetration Test Findings
2+
3+
**Date:** 2026-03-16
4+
**Scope:** Internal authorized pentest — `192.168.178.194:5765` (LXC 101, pve-node1)
5+
**Tester:** Kali Linux VM201 (`192.168.178.177`)
6+
**App version:** `0.30.0-beta`
7+
**Tools used:** nmap, nikto, gobuster, hydra, curl, Python3
8+
9+
---
10+
11+
## Executive Summary
12+
13+
Sublarr is overall well-structured from a security perspective. The API-key auth middleware, `is_safe_path()` path traversal protection, `hmac.compare_digest()` for timing-safe comparisons, and parameterized SQLite queries all demonstrate good baseline hygiene. No critical or exploitable Remote Code Execution, SQL Injection, or path traversal vulnerabilities were found unauthenticated.
14+
15+
**Main gaps:** No rate limiting on any auth endpoint (brute-force viable), the API-key middleware conflicts with the UI login flow when both auth systems are active simultaneously, and several HTTP security headers are missing.
16+
17+
---
18+
19+
## Findings
20+
21+
### F-01 — No Rate Limiting on API Key Auth
22+
**Severity:** HIGH
23+
**Affected file:** `backend/auth.py``check_api_key()`
24+
25+
20 consecutive requests with wrong API keys returned 401 instantly with no throttle, delay, or lockout. The warning is logged but requests are never blocked.
26+
27+
**Impact:** An attacker with LAN access can brute-force the API key at network speed. A typical 32-char alphanumeric key is computationally infeasible to brute-force, but short or dictionary-style keys are at risk.
28+
29+
**Fix:** Add `flask-limiter` with `5/minute` per IP on all auth-checked endpoints. Example:
30+
```python
31+
from flask_limiter import Limiter
32+
limiter = Limiter(key_func=get_remote_address, default_limits=["200/day", "50/hour"])
33+
# On the auth check:
34+
@limiter.limit("5/minute", error_message="Too many requests")
35+
def check_api_key(): ...
36+
```
37+
38+
---
39+
40+
### F-02 — No Rate Limiting on UI Login Endpoint
41+
**Severity:** HIGH
42+
**Affected file:** `backend/routes/auth_ui.py``login()`
43+
44+
15 consecutive wrong-password attempts returned 401 instantly with no delay, throttle, or lockout. Only a warning log is written.
45+
46+
**Impact:** The UI password (minimum 4 chars per F-06) can be brute-forced from the LAN in seconds.
47+
48+
**Fix:** Same `flask-limiter` integration as F-01. Apply to `POST /api/v1/auth/login` specifically:
49+
```python
50+
@auth_ui_bp.post("/login")
51+
@limiter.limit("5/minute")
52+
def login(): ...
53+
```
54+
Also consider a 1-second `time.sleep()` delay on failed attempts (defense-in-depth).
55+
56+
---
57+
58+
### F-03 — Auth Middleware Ordering Breaks UI Login
59+
**Severity:** MEDIUM-HIGH
60+
**Affected file:** `backend/auth.py``check_api_key()`
61+
62+
The API-key middleware (`auth.py`) fires **before** the UI session middleware (`ui_auth.py`) and does not exempt `/api/v1/auth/` paths. When `SUBLARR_API_KEY` is set, all auth endpoints (`/login`, `/setup`, `/status`, `/logout`, `/change-password`) return `{"error": "API key required"}` before the UI auth handler runs.
63+
64+
**Confirmed by test:**
65+
```
66+
POST /api/v1/auth/login → 401 {"error":"API key required"}
67+
GET /api/v1/auth/status → 401 {"error":"API key required"}
68+
```
69+
70+
**Impact:** If both UI auth and API key are configured simultaneously, the web UI is completely inaccessible — users cannot log in. This functionally breaks UI authentication and may cause admins to disable auth entirely.
71+
72+
**Fix** (one line in `auth.py`, analogous to the health and webhook exemptions):
73+
```python
74+
# Skip auth for UI auth endpoints (they handle their own auth)
75+
if path.startswith("/api/v1/auth/"):
76+
return None
77+
```
78+
79+
---
80+
81+
### F-04 — Version and Service Topology Disclosed Unauthenticated
82+
**Severity:** MEDIUM
83+
**Affected endpoint:** `GET /api/v1/health` (exempt from auth by design)
84+
85+
Response (no auth required):
86+
```json
87+
{
88+
"status": "healthy",
89+
"version": "0.30.0-beta",
90+
"services": {
91+
"sonarr": "OK",
92+
"radarr": "OK",
93+
"ollama": "OK",
94+
"providers": "error",
95+
"media_servers": "none configured"
96+
}
97+
}
98+
```
99+
100+
**Impact:** Exact software version enables targeted CVE lookups once vulnerabilities are published. Integration topology (Sonarr/Radarr connected, no media server) leaks architectural details.
101+
102+
**Fix options:**
103+
- Strip `version` from the health response, or make it auth-gated.
104+
- Alternatively strip service detail from the health response, returning only `{"status": "healthy"}` for unauthenticated callers.
105+
106+
---
107+
108+
### F-05 — Webhook Middleware Exemption — Security-by-Convention Risk
109+
**Severity:** MEDIUM
110+
**Affected file:** `backend/auth.py` line 73
111+
112+
`check_api_key()` globally skips auth for all `/api/v1/webhook/` paths:
113+
```python
114+
if path.startswith("/api/v1/webhook/"):
115+
return None
116+
```
117+
Each webhook handler is expected to perform its own HMAC auth. Current handlers (`webhooks.py`) do this correctly.
118+
119+
**Impact:** If a future webhook handler is added and the developer forgets the per-handler auth check, it will be globally unprotected with no safety net. There is no fallback enforcement.
120+
121+
**Fix:** Remove the global webhook exemption from `auth.py` and instead validate at the webhook handler level only — or add documentation and a code comment flagging this as a footgun. Alternatively, validate in `auth.py` that webhook routes always have an `Authorization` or `X-Webhook-Secret` header present.
122+
123+
---
124+
125+
### F-06 — Minimum Password Length Too Short
126+
**Severity:** LOW
127+
**Affected file:** `backend/routes/auth_ui.py` line 20
128+
129+
```python
130+
_MIN_PASSWORD_LENGTH = 4
131+
```
132+
133+
4 characters allow trivially brute-forceable passwords. Combined with no rate limiting (F-02), this is exploitable from the LAN.
134+
135+
**Fix:** Increase to `_MIN_PASSWORD_LENGTH = 12`. Add a reminder in the UI that passwords should be strong.
136+
137+
---
138+
139+
### F-07 — Missing HTTP Security Headers
140+
**Severity:** MEDIUM
141+
**Observed:** All HTTP responses lack:
142+
143+
| Header | Missing Value | Risk |
144+
|--------|--------------|------|
145+
| `X-Frame-Options` | `DENY` | Clickjacking of the SPA |
146+
| `X-Content-Type-Options` | `nosniff` | MIME sniffing |
147+
| `Content-Security-Policy` | (none) | XSS amplification |
148+
| `Referrer-Policy` | `same-origin` | Referrer leakage |
149+
| `Permissions-Policy` | (none) | Feature policy |
150+
151+
**Fix:** Add `Flask-Talisman` or set headers manually in `app.py`:
152+
```python
153+
@app.after_request
154+
def add_security_headers(response):
155+
response.headers["X-Frame-Options"] = "DENY"
156+
response.headers["X-Content-Type-Options"] = "nosniff"
157+
response.headers["Referrer-Policy"] = "same-origin"
158+
return response
159+
```
160+
161+
---
162+
163+
### F-08 — Socket.IO Handshake Unauthenticated (Config Leak)
164+
**Severity:** LOW
165+
**Affected endpoint:** `GET /socket.io/?EIO=4&transport=polling`
166+
167+
The EIO4 open frame succeeds without authentication, returning:
168+
```json
169+
{"sid":"...","upgrades":["websocket"],"pingTimeout":20000,"pingInterval":25000,"maxPayload":1000000}
170+
```
171+
Namespace connect is correctly rejected. Only server timing config is leaked.
172+
173+
**Impact:** Low. Reveals internal Socket.IO config. Namespace auth fires correctly on connection attempt.
174+
175+
**Fix:** No immediate action required. Consider adding an origin check or early auth on the Socket.IO namespace connection.
176+
177+
---
178+
179+
### F-09 — rpcbind (Port 111) Exposed on LXC
180+
**Severity:** LOW
181+
**Affected system:** LXC CT 101 (infrastructure level, not Sublarr app)
182+
183+
`nmap` reveals port 111 (rpcbind) open. Not needed for Sublarr.
184+
185+
**Fix:**
186+
```bash
187+
# On pve-node1:
188+
pct exec 101 -- systemctl disable rpcbind --now
189+
```
190+
191+
---
192+
193+
### F-10 — ETag Leaks Inode and Mtime (INFO)
194+
**Severity:** INFO
195+
**Observed:** `ETag: "1773685672.0-1722-1563297874"` on static file responses.
196+
197+
Gunicorn's ETag format encodes inode number and mtime. Historically used in inode fingerprinting (CVE-2003-1418 / Apache), low practical impact with Gunicorn.
198+
199+
**No fix required.**
200+
201+
---
202+
203+
### F-11 — Server Header Discloses WSGI Server (INFO)
204+
**Severity:** INFO
205+
**Observed:** `Server: gunicorn`
206+
207+
No version number disclosed. Standard fingerprinting risk, minimal impact.
208+
209+
**Fix (optional):** Set `SERVER_NAME` or use a custom `server_header` in Gunicorn config.
210+
211+
---
212+
213+
## Summary Table
214+
215+
| ID | Finding | Severity | Status |
216+
|----|---------|----------|--------|
217+
| F-01 | No rate limiting — API key auth | **HIGH** | Open |
218+
| F-02 | No rate limiting — UI login | **HIGH** | Open |
219+
| F-03 | Auth middleware ordering breaks UI login | **MEDIUM-HIGH** | Open |
220+
| F-04 | Version/topology in unauthenticated health endpoint | **MEDIUM** | Open |
221+
| F-05 | Webhook exemption — security-by-convention risk | **MEDIUM** | Open |
222+
| F-06 | Minimum password length too short (4 chars) | **LOW** | Open |
223+
| F-07 | Missing HTTP security headers | **MEDIUM** | Open |
224+
| F-08 | Socket.IO handshake unauthenticated (config leak) | **LOW** | Open |
225+
| F-09 | rpcbind exposed on LXC (infrastructure) | **LOW** | Open |
226+
| F-10 | ETag leaks inode + mtime | **INFO** | No fix needed |
227+
| F-11 | Server header discloses Gunicorn | **INFO** | Optional |
228+
229+
---
230+
231+
## Confirmed Safe
232+
233+
The following attack vectors were tested and found **not exploitable**:
234+
235+
- **SQL Injection:** All DB queries use parameterized statements. No raw string interpolation found.
236+
- **Path Traversal:** `is_safe_path()` correctly applied on file endpoints. Symlink resolution in place.
237+
- **Command Injection:** No user-supplied data passed to shell. Plugin git URLs validated against `_ALLOWED_GIT_DOMAINS`.
238+
- **ZIP Slip:** `safe_zip_extract()` validates all archive members before extraction.
239+
- **CORS Misconfiguration:** No wildcard or reflected origin on API responses.
240+
- **CSRF:** All state-changing operations are JSON POST — not submittable via standard HTML forms.
241+
- **Timing attacks on API key comparison:** `hmac.compare_digest()` used correctly in both `auth.py` middleware instances.
242+
- **Sensitive data in API responses:** Config, providers, API keys all require auth. No plaintext secrets in any unauthenticated response.
243+
- **SSRF:** Provider and media-server config endpoints require auth. No SSRF surface reachable unauthenticated.
244+
- **HTTP TRACE:** Disabled (405).
245+
246+
---
247+
248+
## Recommended Fix Priority
249+
250+
1. **F-03** (1 line — `auth.py`) — fixes broken UI login, do immediately
251+
2. **F-06** (1 line — `auth_ui.py`) — increase minimum password to 12 chars
252+
3. **F-07** (5 lines — `app.py`) — add security headers
253+
4. **F-01 + F-02** (requires `flask-limiter` dependency) — rate limiting
254+
5. **F-04** — strip version from health response or gate behind auth
255+
6. **F-05** — document/comment the webhook exemption risk
256+
7. **F-09** — disable rpcbind on LXC 101 (infra)

backend/app.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,13 @@ def create_app(testing=False):
180180

181181
register_error_handlers(app)
182182

183+
@app.after_request
184+
def add_security_headers(response):
185+
response.headers.setdefault("X-Frame-Options", "DENY")
186+
response.headers.setdefault("X-Content-Type-Options", "nosniff")
187+
response.headers.setdefault("Referrer-Policy", "same-origin")
188+
return response
189+
183190
# Initialize authentication
184191
from auth import init_auth
185192
from routes.auth_ui import auth_ui_bp

backend/auth.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ def check_api_key():
7474
if path.startswith("/api/v1/webhook/"):
7575
return None
7676

77+
# Skip auth for UI auth endpoints (login, setup, status, logout)
78+
# These handle their own authentication logic
79+
if path.startswith("/api/v1/auth/"):
80+
return None
81+
7782
provided_key = request.headers.get("X-Api-Key") or request.args.get("apikey")
7883

7984
if not provided_key:

backend/routes/auth_ui.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
logger = logging.getLogger(__name__)
1818

1919
auth_ui_bp = Blueprint("auth_ui", __name__, url_prefix="/api/v1/auth")
20-
_MIN_PASSWORD_LENGTH = 4
20+
_MIN_PASSWORD_LENGTH = 12
2121

2222

2323
def _is_session_authenticated() -> bool:

backend/routes/system.py

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -175,13 +175,28 @@ def health():
175175
healthy = False
176176

177177
status_code = 200 if healthy else 503
178-
return jsonify(
179-
{
180-
"status": "healthy" if healthy else "unhealthy",
181-
"version": __version__,
182-
"services": service_status,
183-
}
184-
), status_code
178+
179+
# Include version and service detail only for authenticated callers.
180+
# Unauthenticated probes (uptime monitors, scanners) receive only the status.
181+
import hmac as _hmac
182+
183+
from flask import session as _session
184+
185+
from config import get_settings as _get_settings
186+
187+
_settings = _get_settings()
188+
_api_key = getattr(_settings, "api_key", None)
189+
_provided = request.headers.get("X-Api-Key") or request.args.get("apikey", "")
190+
_key_ok = bool(_api_key and _hmac.compare_digest(_provided, _api_key))
191+
_session_ok = bool(_session.get("ui_authenticated"))
192+
_authenticated = _key_ok or _session_ok or not _api_key
193+
194+
body: dict = {"status": "healthy" if healthy else "unhealthy"}
195+
if _authenticated:
196+
body["version"] = __version__
197+
body["services"] = service_status
198+
199+
return jsonify(body), status_code
185200

186201

187202
@bp.route("/update", methods=["GET"])

0 commit comments

Comments
 (0)