# 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.