Skip to content

Security hardening bundle (medium): prod error leak, session cookie Secure/CSRF, SESSION_SECRET default, SMTP plaintext AUTH, cc/bcc validation, dashboard rate-limit #133

Description

@mohamedboukari

Severity: Medium (defense-in-depth). A basket of lower-severity findings from the v0.8.0 security sweep. Each is small; grouped to track together. Split out if any grows.

1. Global error handler leaks raw error.message in production

src/index.ts:158 — stacks are dev-only (good), but the raw exception message is returned on every unhandled 500 in prod. Postgres/driver errors embed table/column/query fragments → schema disclosure. Fix: return a generic "Internal server error" for the 500 branch; keep mapped errors (SuppressedRecipientError, UnauthorizedSenderError); log detail server-side only.

2. Session cookie missing Secure; CSRF relies solely on SameSite=Lax

src/pages/pages.plugin.tsx:266bm_session=…; HttpOnly; SameSite=Lax; Path=/dashboardno Secure (cookie sent over plaintext HTTP if TLS is ever missing → MITM session theft). Every state-mutating dashboard action is a POST with no CSRF token and no Origin/Referer check; SameSite=Lax is the only defense. Fix: add Secure (at least off-localhost) + an Origin/Referer allowlist check or per-session CSRF token on POST handlers.

3. SESSION_SECRET defaults to a fresh random UUID per process

src/config.ts:266 — regenerated every restart, differs per replica (sessions silently invalidate; multi-replica rejects each other's cookies → operators "fix" it with a weak hardcoded value). Session payload is <ts>.<hmac> with no session id → no revocation, password change doesn't invalidate cookies. Fix: require SESSION_SECRET in production (mirror the DASHBOARD_PASSWORD prod guard), fail fast if unset.

4. SMTP submission allows plaintext AUTH by default

src/modules/smtp-submission/services/smtp-submission.service.ts:190allowInsecureAuth: true when TLS unset (default), and the password is a bm_live_ key. Documented "trusted network only," but on by default → one exposed port from leaking full-privilege keys in cleartext. Fix: default to false unless SMTP_SUBMISSION_ALLOW_INSECURE=true; recommend STARTTLS.

5. cc/bcc unvalidated (CRLF header-injection defense-in-depth)

src/modules/emails/dtos/send-email.dto.ts:22cc/bcc are bare t.String() (no format: "email", no length cap), unlike to/from. Injection is currently neutralized only by nodemailer's address parser. Fix: apply format: "email" + length caps to cc/bcc; strip \r/\n from subject and the List-Unsubscribe url/mailto as defense-in-depth.

6. No rate limit on dashboard actions that do expensive work

POST /dashboard/domains/:id/verify (pages.plugin.tsx:969) runs live DNS lookups with no throttle; the HTTP limiter is per-API-key and only on /api/v1/*. In-memory limiter is also per-replica. Fix: throttle the DNS-verify action; consider a shared-store limiter for multi-replica.

Informational (no action needed)

  • Outbound MX TLS rejectUnauthorized: false (mailer.service.ts:265) — industry norm for opportunistic direct-to-MX; only disabled-TLS site in the codebase. Document as accepted.
  • Unsalted SHA-256 API-key hashing — fine for 128-bit random tokens (consider 256-bit for margin).
  • Logger has no generic secret redaction; adequate by convention today but fragile — consider a denylist redactor in formatLog.

Acceptance

  • Items 1–6 addressed (or explicitly deferred with rationale).
  • SECURITY.md / THREAT_MODEL.md updated where relevant.

Metadata

Metadata

Assignees

No one assigned

    Labels

    securitySecurity issue or hardening

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions