Skip to content

fix: address 7 findings from security review#30

Merged
olivierlambert merged 1 commit intomainfrom
security/fix-review-findings
Apr 4, 2026
Merged

fix: address 7 findings from security review#30
olivierlambert merged 1 commit intomainfrom
security/fix-review-findings

Conversation

@olivierlambert
Copy link
Copy Markdown
Owner

Summary

Fixes 7 vulnerabilities (4 High, 2 Medium, 1 template-level) identified during a full codebase security review.

High severity

  • Stored XSS via innerHTML — User-controlled data (name, email, username) was rendered via innerHTML in team/event-type search dropdowns. Minijinja's auto-escaping produces HTML entities (<) which innerHTML decodes back to live HTML. Replaced all instances with safe DOM APIs (textContent, createElement). Affected: team_settings.html, team_form.html, event_type_form.html.

  • IDOR: invite deletionPOST /dashboard/invites/{id}/delete had no ownership check. Any authenticated user could delete any invite. Added ownership verification via accounts.user_id or team_members membership.

  • IDOR: invite creation for private event typesPOST /dashboard/invites/{id}/send and POST /dashboard/invites/{id}/quick-link did not enforce the documented rule that private event types restrict invite creation to the owner. Added ownership check for private visibility; internal remains open to all authenticated users by design.

  • SSRF via CalDAV source URLs — No validation on user-supplied CalDAV URLs allowed probing internal networks (cloud metadata, localhost services). Added validate_caldav_url() that rejects non-HTTP(S) schemes, private/reserved IP ranges (RFC 1918, loopback, link-local, CGNAT, IPv6 ULA), and resolves hostnames to check all IPs before connecting.

Medium severity

  • IDOR: invite management pageGET /dashboard/invites/{id} leaked private event type details (title, guest PII, active invite tokens) to any authenticated user. Added ownership check for private visibility.

  • Booking approval via GETGET /booking/approve/{token} performed irreversible side effects (confirm booking, CalDAV write-back, send emails). Email security scanners (Safe Links, Proofpoint) auto-follow links, causing unintended approvals. Changed to two-step flow matching existing decline/cancel pattern: GET renders confirmation form, POST performs mutation.

Files changed

File Change
src/caldav/mod.rs validate_caldav_url(), is_private_ip()
src/web/mod.rs Ownership checks on 4 invite handlers, approve GET→POST split, SSRF validation call, updated tests
templates/event_type_form.html innerHTML → textContent
templates/team_form.html innerHTML → textContent
templates/team_settings.html innerHTML → textContent
templates/booking_approve_form.html New confirmation form template

Test plan

  • All 508 existing tests pass
  • Approve test updated: verifies GET shows form without approving, POST performs approval
  • Manual: register user with <img src=x onerror=alert(1)> as name, verify no XSS in team search
  • Manual: verify CalDAV source with http://127.0.0.1 or http://169.254.169.254 is rejected
  • Manual: verify non-owner cannot access private event type invite pages
  • Manual: verify booking approval email link shows form, requires click to approve

🤖 Generated with Claude Code

Fixes XSS, IDOR, SSRF, and CSRF vulnerabilities identified during
a full codebase security review.

- Stored XSS: replace innerHTML with textContent/createElement for
  user-controlled data in team and event type search results
- IDOR: add ownership checks on invite delete, create, quick-link,
  and invite management page for private event types
- SSRF: validate CalDAV source URLs against private IP ranges and
  non-HTTP schemes before making requests
- CSRF: change booking approval from GET to GET(form)+POST(action)
  to prevent auto-approval by email security scanners

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@olivierlambert olivierlambert merged commit eaf4680 into main Apr 4, 2026
4 checks passed
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