Skip to content

Security: harden auth, CSRF, SSRF, XSS, and env race conditions - #171

Closed
betamod wants to merge 2 commits into
nesquena:masterfrom
betamod:security/hardening-12-fixes
Closed

betamod wants to merge 2 commits into
nesquena:masterfrom
betamod:security/hardening-12-fixes

Conversation

@betamod

@betamod betamod commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Security Audit -- 12 Fixes

Full security review of the Hermes WebUI codebase. All fixes are backward-compatible with no breaking changes.

CRITICAL

  • CSRF protection -- All POST endpoints now validate Origin/Referer against Host. Prevents cross-origin abuse of self-update, settings changes, file operations, and password changes.

HIGH

  • Password hashing unified -- save_settings() was using single-iteration SHA-256. Now calls auth._hash_password() (PBKDF2-HMAC-SHA256, 600k iterations).
  • Login rate limiting -- 5 attempts per 60 seconds per IP. Returns 429 on excess.

MEDIUM

  • Session ID validation -- Session.load() rejects non-hex chars before filesystem operations. Prevents path traversal via crafted session ID.
  • SSRF DNS resolution -- get_available_models() now resolves DNS before checking private IPs. Prevents DNS rebinding attacks.
  • Non-loopback warning -- Startup warning when binding to 0.0.0.0 without a password set (common Docker footgun).
  • ENV LOCK consistency -- _ENV_LOCK now wraps all os.environ mutations in both sync chat and streaming restore blocks.
  • Stored XSS prevention -- Forces Content-Disposition: attachment for HTML/XHTML/SVG MIME types.

LOW

  • HMAC signature -- Extended from 64 to 128 bits.
  • Skills path validation -- resolve().relative_to(SKILLS_DIR) check after path construction.
  • Secure cookie flag -- Auto-set when TLS or X-Forwarded-Proto: https detected.
  • Error message sanitization -- _sanitize_error() strips absolute filesystem paths from exception messages.

Files Changed

  • api/auth.py (+27/-2) -- rate limiter, HMAC, Secure flag
  • api/config.py (+26/-4) -- PBKDF2, SSRF DNS check
  • api/helpers.py (+9) -- error sanitizer
  • api/models.py (+3) -- session ID validation
  • api/routes.py (+85/-38) -- CSRF, ENV LOCK, XSS, skills validation, sanitized errors
  • api/streaming.py (+17/-7) -- ENV LOCK on restore
  • server.py (+8) -- non-loopback warning

betamod and others added 2 commits April 8, 2026 00:49
Twelve fixes from a full security audit:

CRITICAL
- Add CSRF Origin/Referer validation on all POST endpoints
  (prevents cross-origin abuse of self-update, settings, file ops)

HIGH
- Unify password hashing: config.py now uses PBKDF2 (600k iters)
  instead of single-iteration SHA-256
- Add per-IP rate limiting on login (5 attempts/60s, 429 on excess)

MEDIUM
- Validate session IDs as hex-only before filesystem operations
  (prevents path traversal via crafted session ID)
- SSRF: resolve DNS before private-IP check in model fetching
  (prevents DNS rebinding to internal services)
- Warn loudly when binding non-loopback without password set
- SSE env var mutations: wrap sync chat + streaming restore in _ENV_LOCK
- Force Content-Disposition:attachment for HTML/XHTML/SVG uploads
  (prevents stored XSS via uploaded files)

LOW
- Extend HMAC session signature from 64 to 128 bits
- Add resolve()+relative_to() check on skills path construction
- Set Secure flag on session cookie when connection is HTTPS
- Sanitize exception messages to strip filesystem paths

No breaking changes. All fixes are backward-compatible.
handler.request.getpeercert raises AttributeError on plain sockets
(non-SSL). Use getattr(..., None) to safely check for SSL.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nesquena

nesquena commented Apr 8, 2026

Copy link
Copy Markdown
Owner

Independent Review: PR #171 — Security Hardening (12 fixes)

Thorough security PR from @betamod. Reviewed all 12 fixes individually.

All 12 fixes are legitimate and well-implemented:

# Fix Verdict
1 CSRF Origin/Referer validation Good — standard pattern, allows non-browser clients
2 PBKDF2 password hashing (600k iterations) Good — replaces weak single-iteration SHA-256
3 Login rate limiting (5/60s per IP) Good — clean implementation
4 Session ID hex-only validation Good — prevents path traversal via crafted IDs
5 SSRF DNS resolution check Good — prevents DNS rebinding, allows known local providers
6 Non-loopback startup warning Good — catches common Docker footgun
7 ENV_LOCK on sync chat path Good — matches existing streaming path pattern
8 Content-Disposition for HTML/SVG Good — prevents stored XSS via uploads
9 HMAC signature 64→128 bits Good — minor improvement
10 Skills path resolve()+relative_to() Good — prevents traversal
11 Secure cookie flag Had a bug — fixed (see below)
12 Error path sanitization Good — regex strips filesystem paths

Bug found and fixed

Secure cookie detection (auth.py line 181): handler.request.getpeercert raises AttributeError on plain sockets — non-SSL socket.socket objects don't have that method. Pushed a fix using getattr(handler.request, 'getpeercert', None) which safely returns None for plain sockets and the method object for SSL sockets.

Tests

442 passed, 0 failed, 24 skipped. No regressions.

nesquena-hermes pushed a commit that referenced this pull request Apr 8, 2026
33 tests covering all 12 security fixes:
- CSRF origin/referer validation
- Login rate limiting (5 attempts/60s)
- Session ID hex validation (path traversal prevention)
- Error path sanitization (_sanitize_error)
- Secure cookie getattr safety
- HMAC signature length (64->128 bit)
- Skills path traversal prevention
- Content-Disposition for HTML/SVG/XHTML
- PBKDF2 password hashing verification
- Non-loopback startup warning
- SSRF DNS guard code presence
- _ENV_LOCK export from streaming module
@betamod

betamod commented Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

Thanks very much!

nesquena-hermes added a commit that referenced this pull request Apr 8, 2026
* Security: harden auth, CSRF, SSRF, XSS, and env race conditions

Twelve fixes from a full security audit:

CRITICAL
- Add CSRF Origin/Referer validation on all POST endpoints
  (prevents cross-origin abuse of self-update, settings, file ops)

HIGH
- Unify password hashing: config.py now uses PBKDF2 (600k iters)
  instead of single-iteration SHA-256
- Add per-IP rate limiting on login (5 attempts/60s, 429 on excess)

MEDIUM
- Validate session IDs as hex-only before filesystem operations
  (prevents path traversal via crafted session ID)
- SSRF: resolve DNS before private-IP check in model fetching
  (prevents DNS rebinding to internal services)
- Warn loudly when binding non-loopback without password set
- SSE env var mutations: wrap sync chat + streaming restore in _ENV_LOCK
- Force Content-Disposition:attachment for HTML/XHTML/SVG uploads
  (prevents stored XSS via uploaded files)

LOW
- Extend HMAC session signature from 64 to 128 bits
- Add resolve()+relative_to() check on skills path construction
- Set Secure flag on session cookie when connection is HTTPS
- Sanitize exception messages to strip filesystem paths

No breaking changes. All fixes are backward-compatible.

* fix: use getattr for Secure cookie SSL detection

handler.request.getpeercert raises AttributeError on plain sockets
(non-SSL). Use getattr(..., None) to safely check for SSL.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* tests: add sprint 29 security hardening coverage (PR #171)

33 tests covering all 12 security fixes:
- CSRF origin/referer validation
- Login rate limiting (5 attempts/60s)
- Session ID hex validation (path traversal prevention)
- Error path sanitization (_sanitize_error)
- Secure cookie getattr safety
- HMAC signature length (64->128 bit)
- Skills path traversal prevention
- Content-Disposition for HTML/SVG/XHTML
- PBKDF2 password hashing verification
- Non-loopback startup warning
- SSRF DNS guard code presence
- _ENV_LOCK export from streaming module

* release: v0.39.0 — security hardening, 12 fixes (#171)

---------

Co-authored-by: betamod <matthew.sloly@gmail.com>
Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nesquena-hermes

nesquena-hermes commented Apr 8, 2026

Copy link
Copy Markdown
Collaborator

Review complete — merged as v0.39.0 via PR #172

Reviewed all 12 fixes end-to-end. All are legitimate, well-implemented, and backward-compatible. Summary:

# Fix Verdict
1 CSRF Origin/Referer validation ✅ Clean implementation — non-browser clients unaffected
2 PBKDF2 password hashing (600k iterations) ✅ Replaces weak single-iteration SHA-256
3 Login rate limiting (5/60s per IP) ✅ In-memory rate limiter, correct sliding window
4 Session ID hex-only validation ✅ Prevents path traversal via crafted session IDs
5 SSRF DNS resolution check ✅ Prevents DNS rebinding; known local providers whitelisted
6 Non-loopback startup warning ✅ Catches common Docker footgun
7 ENV_LOCK on sync chat path ✅ Consistent with streaming path pattern
8 Content-Disposition for HTML/SVG ✅ Prevents stored XSS via uploads
9 HMAC signature 64→128 bits ✅ Clean improvement
10 Skills path resolve()+relative_to() ✅ Prevents traversal
11 Secure cookie flag (getattr fix applied) ✅ Safe on plain sockets
12 Error path sanitization ✅ Regex strips filesystem paths

What was added before merge

  • Rebased on master (branch was behind by 5 commits from v0.38.5; cherry-picked cleanly, no conflicts)
  • 33 tests in tests/test_sprint29.py covering all 12 fixes — all green
  • Version bump: v0.38.6 → v0.39.0 with full CHANGELOG entry

Final results

499 tests passed, 0 failed.

Thanks @betamod — solid security work.

Ola-Turmo pushed a commit to Ola-Turmo/hermes-webui that referenced this pull request Apr 9, 2026
* Security: harden auth, CSRF, SSRF, XSS, and env race conditions

Twelve fixes from a full security audit:

CRITICAL
- Add CSRF Origin/Referer validation on all POST endpoints
  (prevents cross-origin abuse of self-update, settings, file ops)

HIGH
- Unify password hashing: config.py now uses PBKDF2 (600k iters)
  instead of single-iteration SHA-256
- Add per-IP rate limiting on login (5 attempts/60s, 429 on excess)

MEDIUM
- Validate session IDs as hex-only before filesystem operations
  (prevents path traversal via crafted session ID)
- SSRF: resolve DNS before private-IP check in model fetching
  (prevents DNS rebinding to internal services)
- Warn loudly when binding non-loopback without password set
- SSE env var mutations: wrap sync chat + streaming restore in _ENV_LOCK
- Force Content-Disposition:attachment for HTML/XHTML/SVG uploads
  (prevents stored XSS via uploaded files)

LOW
- Extend HMAC session signature from 64 to 128 bits
- Add resolve()+relative_to() check on skills path construction
- Set Secure flag on session cookie when connection is HTTPS
- Sanitize exception messages to strip filesystem paths

No breaking changes. All fixes are backward-compatible.

* fix: use getattr for Secure cookie SSL detection

handler.request.getpeercert raises AttributeError on plain sockets
(non-SSL). Use getattr(..., None) to safely check for SSL.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* tests: add sprint 29 security hardening coverage (PR nesquena#171)

33 tests covering all 12 security fixes:
- CSRF origin/referer validation
- Login rate limiting (5 attempts/60s)
- Session ID hex validation (path traversal prevention)
- Error path sanitization (_sanitize_error)
- Secure cookie getattr safety
- HMAC signature length (64->128 bit)
- Skills path traversal prevention
- Content-Disposition for HTML/SVG/XHTML
- PBKDF2 password hashing verification
- Non-loopback startup warning
- SSRF DNS guard code presence
- _ENV_LOCK export from streaming module

* release: v0.39.0 — security hardening, 12 fixes (nesquena#171)

---------

Co-authored-by: betamod <matthew.sloly@gmail.com>
Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
JKJameson pushed a commit to JKJameson/hermes-webui that referenced this pull request Apr 25, 2026
* Security: harden auth, CSRF, SSRF, XSS, and env race conditions

Twelve fixes from a full security audit:

CRITICAL
- Add CSRF Origin/Referer validation on all POST endpoints
  (prevents cross-origin abuse of self-update, settings, file ops)

HIGH
- Unify password hashing: config.py now uses PBKDF2 (600k iters)
  instead of single-iteration SHA-256
- Add per-IP rate limiting on login (5 attempts/60s, 429 on excess)

MEDIUM
- Validate session IDs as hex-only before filesystem operations
  (prevents path traversal via crafted session ID)
- SSRF: resolve DNS before private-IP check in model fetching
  (prevents DNS rebinding to internal services)
- Warn loudly when binding non-loopback without password set
- SSE env var mutations: wrap sync chat + streaming restore in _ENV_LOCK
- Force Content-Disposition:attachment for HTML/XHTML/SVG uploads
  (prevents stored XSS via uploaded files)

LOW
- Extend HMAC session signature from 64 to 128 bits
- Add resolve()+relative_to() check on skills path construction
- Set Secure flag on session cookie when connection is HTTPS
- Sanitize exception messages to strip filesystem paths

No breaking changes. All fixes are backward-compatible.

* fix: use getattr for Secure cookie SSL detection

handler.request.getpeercert raises AttributeError on plain sockets
(non-SSL). Use getattr(..., None) to safely check for SSL.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* tests: add sprint 29 security hardening coverage (PR nesquena#171)

33 tests covering all 12 security fixes:
- CSRF origin/referer validation
- Login rate limiting (5 attempts/60s)
- Session ID hex validation (path traversal prevention)
- Error path sanitization (_sanitize_error)
- Secure cookie getattr safety
- HMAC signature length (64->128 bit)
- Skills path traversal prevention
- Content-Disposition for HTML/SVG/XHTML
- PBKDF2 password hashing verification
- Non-loopback startup warning
- SSRF DNS guard code presence
- _ENV_LOCK export from streaming module

* release: v0.39.0 — security hardening, 12 fixes (nesquena#171)

---------

Co-authored-by: betamod <matthew.sloly@gmail.com>
Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
* Security: harden auth, CSRF, SSRF, XSS, and env race conditions

Twelve fixes from a full security audit:

CRITICAL
- Add CSRF Origin/Referer validation on all POST endpoints
  (prevents cross-origin abuse of self-update, settings, file ops)

HIGH
- Unify password hashing: config.py now uses PBKDF2 (600k iters)
  instead of single-iteration SHA-256
- Add per-IP rate limiting on login (5 attempts/60s, 429 on excess)

MEDIUM
- Validate session IDs as hex-only before filesystem operations
  (prevents path traversal via crafted session ID)
- SSRF: resolve DNS before private-IP check in model fetching
  (prevents DNS rebinding to internal services)
- Warn loudly when binding non-loopback without password set
- SSE env var mutations: wrap sync chat + streaming restore in _ENV_LOCK
- Force Content-Disposition:attachment for HTML/XHTML/SVG uploads
  (prevents stored XSS via uploaded files)

LOW
- Extend HMAC session signature from 64 to 128 bits
- Add resolve()+relative_to() check on skills path construction
- Set Secure flag on session cookie when connection is HTTPS
- Sanitize exception messages to strip filesystem paths

No breaking changes. All fixes are backward-compatible.

* fix: use getattr for Secure cookie SSL detection

handler.request.getpeercert raises AttributeError on plain sockets
(non-SSL). Use getattr(..., None) to safely check for SSL.


* tests: add sprint 29 security hardening coverage (PR nesquena#171)

33 tests covering all 12 security fixes:
- CSRF origin/referer validation
- Login rate limiting (5 attempts/60s)
- Session ID hex validation (path traversal prevention)
- Error path sanitization (_sanitize_error)
- Secure cookie getattr safety
- HMAC signature length (64->128 bit)
- Skills path traversal prevention
- Content-Disposition for HTML/SVG/XHTML
- PBKDF2 password hashing verification
- Non-loopback startup warning
- SSRF DNS guard code presence
- _ENV_LOCK export from streaming module

* release: v0.39.0 — security hardening, 12 fixes (nesquena#171)

---------

Co-authored-by: betamod <matthew.sloly@gmail.com>
Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants