johan
/
dealspace
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
dealspace/docs/SECURITY-AUDIT-2026-02-28.md

8.0 KiB
Raw Blame History

Security Audit Report - Dealspace Go Codebase

Date: 2026-02-28
Auditor: James (automated security review)
Scope: All Go source files in lib/, api/, cmd/, migrations/

Executive Summary

Security audit of the Dealspace M&A workflow platform codebase. Found 2 CRITICAL, 2 HIGH, 4 MEDIUM, and 3 LOW/INFO issues. Critical and high severity issues have been fixed in this commit.

Findings by Severity

  • CRITICAL: 2 (fixed)
  • HIGH: 2 (fixed)
  • MEDIUM: 4 (documented, recommended for next sprint)
  • LOW/INFO: 3 (acceptable risk)

CRITICAL Findings (Fixed)

[CRITICAL] OTP Code Comparison Timing Attack

Location: lib/dbcore.go:605 (ChallengeVerify)
Issue: OTP code comparison used c.Code != code which is vulnerable to timing attacks. An attacker could measure response times to progressively guess the correct OTP, significantly reducing the brute-force space from 1M combinations.
Fix: Changed to subtle.ConstantTimeCompare([]byte(c.Code), []byte(code)) != 1 which executes in constant time regardless of how many characters match.
Status: ✅ FIXED

[CRITICAL] Backdoor Code Comparison Timing Attack

Location: api/handlers.go:327-333 (Verify endpoint)
Issue: Backdoor code comparison used req.Code == h.Cfg.BackdoorCode which is timing-vulnerable. Additionally, the original logic was confusing with redundant nested checks.
Fix: Simplified logic and changed to subtle.ConstantTimeCompare([]byte(h.Cfg.BackdoorCode), []byte(req.Code)) == 1. Also added len(req.Code) > 0 check to prevent length leakage.
Status: ✅ FIXED

HIGH Findings (Fixed)

[HIGH] CORS Policy Wildcard

Location: api/middleware.go:135
Issue: CORS policy was Access-Control-Allow-Origin: * allowing any website to make authenticated cross-origin requests. This enables CSRF-like attacks if a user visits a malicious site while logged in.
Fix: Replaced with origin allowlist containing only trusted domains (muskepo.com, dealspace.io, localhost for development). Added Vary: Origin header for proper caching.
Status: ✅ FIXED

[HIGH] Missing Security Headers

Location: api/middleware.go (missing)
Issue: No security headers were set on responses:

  • No X-Frame-Options (clickjacking risk)
  • No X-Content-Type-Options (MIME sniffing risk)
  • No Content-Security-Policy (XSS risk)
    Fix: Added SecurityHeadersMiddleware with:
  • X-Frame-Options: DENY
  • X-Content-Type-Options: nosniff
  • X-XSS-Protection: 1; mode=block
  • Referrer-Policy: strict-origin-when-cross-origin
  • Restrictive CSP policy
    Status: ✅ FIXED

MEDIUM Findings (Documented)

[MEDIUM] No File Type Validation on Upload

Location: api/handlers.go:640-675 (UploadObject)
Issue: Any file type can be uploaded. While files are served with Content-Disposition: attachment and Content-Type: application/octet-stream, this could be a vector for abuse (e.g., uploading malware, hosting phishing pages if served differently in future).
Mitigation in place: Files are served as downloads only, not rendered in browser.
Recommendation: Add allowlist of permitted MIME types (PDF, DOCX, XLSX, images). Validate magic bytes, not just extension.

[MEDIUM] OTP Challenge Rate Limit Too Permissive

Location: api/routes.go:19
Issue: OTP challenge endpoint uses global rate limit of 120 req/min. This is too high for an OTP endpoint. At 120 req/min, an attacker could try all 1M OTP combinations in ~139 hours. With timing attack fixed, this is less critical but still concerning.
Recommendation: Add specific rate limit for /api/auth/challenge and /api/auth/verify (e.g., 5 requests per 10 minutes per email, 10 requests per IP per hour).

[MEDIUM] Session Token Not Bound to IP/Fingerprint

Location: lib/dbcore.go (SessionCreate/SessionByID)
Issue: Sessions store fingerprint (User-Agent) but don't validate it on subsequent requests. A stolen session token can be used from any device/IP.
Recommendation: Add optional strict mode that validates User-Agent and/or IP hasn't changed dramatically since session creation.

[MEDIUM] No Account Lockout After Failed OTP Attempts

Location: api/handlers.go (Verify endpoint)
Issue: Failed OTP verification attempts are not tracked or limited per user. An attacker can make unlimited OTP guesses for a specific email (limited only by global rate limit).
Recommendation: Implement exponential backoff or temporary lockout after 3-5 failed attempts per email address.

LOW/INFO Findings

[LOW] Audit Log Actions Not Encrypted

Location: migrations/001_initial.sql:99, lib/dbcore.go (AuditLog)
Issue: Audit log stores action and details as BLOB (encrypted) but action field is currently stored as plaintext in the handlers. This could leak operation types.
Recommendation: Ensure audit entries use Pack() for all sensitive fields.

[INFO] HSTS Not Set by Application

Location: api/middleware.go
Issue: Application doesn't set HSTS header.
Status: Acceptable - documented that Caddy handles HSTS at the proxy layer. Verify Caddy config includes Strict-Transport-Security: max-age=31536000; includeSubDomains.

[INFO] Empty Responses on Challenge Don't Reveal User Existence

Location: api/handlers.go:265-273
Issue: N/A - This is correctly implemented. Challenge endpoint returns same response whether email exists or not, preventing user enumeration.
Status: ✅ GOOD PRACTICE

Positive Security Findings

The codebase demonstrates good security practices in several areas:

  1. Cryptography:

    • ✅ AES-256-GCM for encryption (FIPS 140-3 compliant)
    • ✅ HKDF-SHA256 for key derivation with proper context separation
    • ✅ Fresh random nonce for each encryption (crypto/rand)
    • ✅ No hardcoded keys (MASTER_KEY from environment)
    • ✅ Per-project encryption keys (compromise isolation)

  2. SQL Injection:

    • ✅ All queries use parameterized statements
    • ✅ No string concatenation in SQL queries

  3. Authentication:

    • ✅ Session tokens generated with crypto/rand (32 bytes, 256-bit)
    • ✅ JWT signature comparison uses hmac.Equal (constant-time)
    • ✅ Passwordless auth reduces password-related attack surface

  4. Authorization:

    • ✅ RBAC consistently enforced via CheckAccess* functions
    • ✅ Project access is explicit (no implicit org-based access)
    • ✅ super_admin bypass is correctly implemented

  5. File Storage:

    • ✅ Content-addressable storage (SHA-256 hash as filename)
    • ✅ No path traversal risk (filenames are hashes)
    • ✅ Files encrypted at rest with project-specific keys

  6. Watermarking:

    • ✅ Applied at serve time (cannot be bypassed)
    • ✅ Tied to authenticated user identity
    • ✅ Original files stored clean (can re-watermark)

  7. Chat/AI Endpoint:

    • ✅ Specific rate limit (20 req/IP/hour)
    • ✅ Zero retention noted for AI provider (Fireworks)

Recommendations for Future Work

  1. Add File Type Validation - Implement MIME type allowlist with magic byte verification
  2. Implement Per-Email Rate Limiting - Add specific rate limits for auth endpoints
  3. Add Failed Attempt Tracking - Implement account lockout after failed OTP attempts
  4. Security Headers Tuning - Review CSP policy after deployment to ensure no breakage
  5. Penetration Testing - Schedule professional pentest before production launch
  6. Dependency Audit - Run go mod audit periodically for known vulnerabilities

Changes Made in This Commit

  1. lib/dbcore.go - Added crypto/subtle import, changed OTP comparison to constant-time
  2. api/handlers.go - Added crypto/subtle import, fixed backdoor code comparison
  3. api/middleware.go - Replaced wildcard CORS with origin allowlist, added SecurityHeadersMiddleware
  4. api/routes.go - Added SecurityHeadersMiddleware to middleware chain
  5. api/middleware_test.go - Updated CORS test for new allowlist behavior

All code compiles and passes updated tests.