Skip to content

Comments

Fix critical security vulnerabilities#211

Merged
alxlion merged 5 commits intodevfrom
security/fix-critical-vulnerabilities
Feb 9, 2026
Merged

Fix critical security vulnerabilities#211
alxlion merged 5 commits intodevfrom
security/fix-critical-vulnerabilities

Conversation

@alxlion
Copy link
Contributor

@alxlion alxlion commented Feb 9, 2026

Summary

Addresses 5 critical findings from a comprehensive security audit of the codebase:

  • C1 - Stored XSS via custom embeds: Sanitize custom embed HTML at the changeset level, stripping everything except <iframe> tags with whitelisted attributes (src, width, height, frameborder, allow, allowfullscreen, etc.). Blocks javascript: URIs in src.
  • C2 - Reflected XSS via URL formatting: Escape URLs with Phoenix.HTML.html_escape/1 before interpolating into <a> tags in format_body/1, preventing attribute injection via crafted URLs in post messages.
  • C3 - IDOR on form data export: Add authorize_event_access/2 check to export_form/2 in StatController, matching the authorization pattern already used by export_poll, export_quiz, and export_all_messages.
  • C4 - Atom exhaustion DoS: Replace all 8 instances of String.to_atom/1 on user-controlled input with explicit whitelists or String.to_existing_atom/1 across 6 files (converter, show, manage, dashboard, table actions, form component).
  • C6 - Missing rate limiting: Add hammer dependency with ETS backend and a RateLimitPlug that enforces 10 requests/minute per IP on all authentication routes (login, registration, password reset, confirmation).

C5 (unprotected file serving via Plug.Static) is deferred for a separate PR.

Test plan

  • mix test test/claper/embeds_test.exs — all 11 embed tests pass (including custom embed sanitization)
  • mix format --check-formatted passes
  • Full test suite: 209 tests, 34 pre-existing failures (none introduced by this PR)
  • Manual: create custom embed with <script> tag alongside iframe — verify only iframe is stored
  • Manual: post message with http://x.com" onclick="alert(1)" — verify " is escaped
  • Manual: call POST /export/forms/:id as non-owner — verify 403
  • Manual: send invalid atom values via LiveView events — verify graceful handling
  • Manual: rapid-fire login attempts — verify 429 after 10 requests

alxlion and others added 4 commits February 9, 2026 12:02
Address 5 critical findings from security audit:
- C1: Sanitize custom embed HTML to prevent stored XSS (strip all non-iframe content)
- C2: Escape URLs in format_body/1 to prevent reflected XSS via post messages
- C3: Add authorization check to form export endpoint (IDOR fix)
- C4: Replace String.to_atom/1 on user input with explicit whitelists (8 locations)
- C6: Add IP-based rate limiting on authentication endpoints via Hammer

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Migrate to Hammer 7.0 API: define Claper.RateLimit module with
use Hammer, add to supervision tree, and use hit/3 instead of
check_rate/3. Remove old app-level config.
Move Claper.RateLimit above ClaperWeb.Endpoint so the ETS table
is ready before any requests are accepted.
@alxlion alxlion marked this pull request as ready for review February 9, 2026 17:22
Merge the two separate case blocks into one to stay within Credo's
complexity limit.
@alxlion alxlion merged commit 8f46837 into dev Feb 9, 2026
1 check passed
@alxlion alxlion deleted the security/fix-critical-vulnerabilities branch February 9, 2026 18:19
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