Skip to content

Security: Sanitize authentication log fields and harden lockout behavior against account denial of service #4

Description

@fvent01

Finding

The login endpoint accepts any non-empty identifier, interpolates it directly into warning log messages, and records it in login_attempts. The rate limiter blocks when failures match either the supplied identifier or source IP, without clearing or reducing failures after a successful login.

Risk

Control characters in an identifier can forge or split log entries, confusing incident response and automated log processing. Separately, an attacker who knows a username or email can repeatedly submit bad passwords from rotating addresses and keep that account locked out indefinitely because username-based failures alone reach the threshold.

Evidence

  • validate_login() checks only whether identifier/password are empty.
  • Failed and rate-limited identifiers are interpolated into log strings.
  • The limiter counts (username = ? OR ip_address = ?) failures.
  • Successful authentication does not clear or age down prior account-specific failure state beyond the fixed time window.

Recommended remediation

  • Enforce a conservative identifier length and reject control characters.
  • Use structured logging with escaped/encoded fields rather than string interpolation.
  • Normalize identifiers consistently before lookup, rate limiting, and logging.
  • Replace hard account lockout with progressive delays, per-IP controls, and carefully bounded per-account throttling.
  • Clear or reduce appropriate counters after successful authentication while retaining an immutable audit event.
  • Add cleanup/retention for login_attempts to prevent indefinite database growth.
  • Consider generic user notification for unusual failed-login activity.

Acceptance criteria

  • Newlines and control characters cannot forge authentication logs.
  • A distributed attacker cannot indefinitely deny access to a known account using only failed passwords.
  • One abusive IP is still effectively throttled.
  • Successful login updates throttling state safely without deleting audit history.
  • Old attempt records are purged according to a documented retention policy.
  • Tests cover log injection, rotating-IP lockout attempts, successful recovery, and cleanup.

Priority

P1 — authentication availability and audit-log integrity.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions