Skip to content

Fix P1/P2 security findings from adversarial review#7

Open
hashd wants to merge 6 commits into
masterfrom
fix/adversarial-review-improvements
Open

Fix P1/P2 security findings from adversarial review#7
hashd wants to merge 6 commits into
masterfrom
fix/adversarial-review-improvements

Conversation

@hashd
Copy link
Copy Markdown
Owner

@hashd hashd commented Apr 13, 2026

Summary

Addresses 23 findings (P1 critical + P2 significant) from the adversarial security review.

P1 Critical fixes

  • Cross-ticket prize claim fraud — struck set now filtered per-ticket during claims and progress computation
  • join_secret leaked in state API — stripped from sanitize_state
  • Full game state returned to any user — REST show endpoint now filters to requesting user's tickets/struck/progress
  • Ticket broadcast to all subscribers — player_tickets_updated only sent to the owning player

P2 Significant fixes

  • Magic link auto-registers unknown emails — now returns error, controller hides email existence
  • Magic link replay race — atomic delete replaces update-used_at
  • Prize progress leaks opponent data — filtered per-ticket
  • Game code space (~25K) — expanded to ~250K with 3-digit numbers
  • No player/game limits — max 100 players/game, max 5 active games/user
  • list_public_games O(n) GenServer calls — reads Registry metadata instead
  • Chat text unsanitized/unbounded — 500 char limit + validation
  • Reaction emoji unvalidated — allowlist enforced
  • create_game race condition — wrapped in DB transaction
  • Rate limit TOCTOU — atomic ETS increment
  • Monitor calls :state every minute — uses Registry metadata
  • CORSPlug with no config — explicit origins configured
  • Logout doesn't revoke token — bearer token deleted on logout
  • inspect(reason) in error response — replaced with generic message
  • No validation on game create params — name length, visibility, join_secret validated

Test plan

  • mix test — 127 tests pass (2 properties + 125 tests, 0 failures)
  • Updated code format tests for WORD-NNN format
  • Updated magic link tests for new return type
  • Manual test: verify game creation, join, and prize claiming flows
  • Manual test: verify magic link and OTP login still work

hashd added 6 commits April 13, 2026 16:45
…validation

- Filter struck set per-ticket in prize claims to prevent cross-ticket fraud (#1)
- Filter prize progress per-ticket to prevent cross-ticket info leak (#9)
- Strip join_secret from sanitized state (#2)
- Filter REST show endpoint to only return requesting user's data (#3)
- Filter player_tickets_updated broadcast to target user only (#4)
- Add chat text length limit (500 chars) and validation (#13)
- Add reaction emoji allowlist (#14)
- Add max players per game limit (100) (#11)
- Add input validation for game creation params (#23)
- Remove inspect(reason) from error responses (#22)
- Store rich metadata in Registry for O(1) game listing
- Don't auto-register unknown emails on magic link request (#7)
- Atomic token delete in verify_magic_link to prevent replay (#8)
- Add delete_api_token/1 and revoke bearer token on logout (#21)
- Return opaque message on magic link request (don't reveal email existence)
- Wrap create_game in transaction for atomicity (#16)
- Limit active games per user to 5 (#11)
- Optimize list_public_games to read Registry metadata instead of O(n) GenServer calls (#12)
- Expand code space from ~25K to ~250K (3-digit numbers) (#10)
- Optimize monitor to use Registry metadata instead of GenServer.call (#18)
- Fix rate limit TOCTOU with atomic ETS increment (#17)
- Configure CORS with explicit origins (#19)
- Update code format tests for WORD-NNN (3-digit) format
- Update magic link tests for new {:ok, token, record} return type
…imer

- Move all Repo calls in GenServer to Task.start to avoid blocking (#15)
- Cancel existing host disconnect timer before setting a new one (#25)
- Reject start_game with zero players (#27)
- Use deterministic sort instead of shuffle in Board.from_snapshot (#28)
- Move token from localStorage to sessionStorage to limit exposure (#5)
- Enforce requiresHost route guard with server-side check (#20)
- Compensate for client/server clock skew in countdown timer (#34)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant