Skip to content

fix: patch XSS vulnerability in input fields (OWASP A03 - Injection)#1319

Open
IshitaSingh0822 wants to merge 1 commit into
Priyanshu-byte-coder:mainfrom
IshitaSingh0822:fix/xss-sanitization
Open

fix: patch XSS vulnerability in input fields (OWASP A03 - Injection)#1319
IshitaSingh0822 wants to merge 1 commit into
Priyanshu-byte-coder:mainfrom
IshitaSingh0822:fix/xss-sanitization

Conversation

@IshitaSingh0822
Copy link
Copy Markdown
Contributor

Summary

Patches a critical XSS vulnerability (OWASP A03: Injection) across DevTrack's input fields. User-supplied text was being stored and rendered without sanitization, allowing script injection via fields like the goal label. This fix adds a shared sanitization utility, backend validation, frontend defence-in-depth, and CSP headers.

Closes #1316

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Refactor / code cleanup

Changes Made

  • Added src/lib/sanitize.ts — shared stripHtml() and validateTextInput() helpers that work in both Node and browser
  • Updated src/app/api/goals/route.ts — validates and strips HTML from label before writing to DB; returns 400 if input is empty or contains tags
  • Updated src/components/GoalTracker.tsx — calls stripHtml() before sending to API and again when rendering goal.label from DB
  • Updated next.config.mjs — added CSP, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, and Permissions-Policy headers on all routes

How to Test

  1. Start the dev server with npm run dev
  2. Enter <script>alert(1)</script> in the goal label field and submit — no alert should fire
  3. Try <img src=x onerror=alert('XSS')> in the same field — same result expected
  4. Run curl -I http://localhost:3000 and confirm CSP headers are present in the response
  5. Open DevTools → Network → reload → check Response Headers for all security headers

Screenshots (if UI change)

N/A — no visual changes, security fix only.

Checklist

  • Linked issue in summary
  • npm run lint passes locally
  • No TypeScript errors (npm run type-check)
  • Self-reviewed the diff
  • Added/updated tests if applicable

- Add src/lib/sanitize.ts with stripHtml and validateTextInput helpers
- Sanitize goal label on both frontend (GoalTracker) and backend (goals API)
- Strip HTML tags from all text inputs before storing to DB
- Add CSP, X-Frame-Options, X-Content-Type-Options headers in next.config.mjs
@vercel
Copy link
Copy Markdown

vercel Bot commented May 27, 2026

@IshitaSingh0822 is attempting to deploy a commit to the PRIYANSHU DOSHI's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added gssoc26 GSSoC 2026 contribution type:bug GSSoC type bonus: bug fix type:security GSSoC type bonus: security (+20 pts) labels May 27, 2026
@github-actions
Copy link
Copy Markdown

GSSoC Label Checklist 🏷️

@Priyanshu-byte-coder — please apply the appropriate labels before merging:

Difficulty (pick one):

  • level:beginner — 20 pts
  • level:intermediate — 35 pts
  • level:advanced — 55 pts
  • level:critical — 80 pts

Quality (optional):

  • quality:clean — ×1.2 multiplier
  • quality:exceptional — ×1.5 multiplier

Validation (required to score):

  • gssoc:approved — counts for points
  • gssoc:invalid / gssoc:spam / gssoc:ai-slop — does not score

Type labels (type:*) are auto-detected from files and title. Review and adjust if needed.
Points formula: (difficulty × quality_multiplier) + type_bonus

@Priyanshu-byte-coder
Copy link
Copy Markdown
Owner

Thanks for the XSS sanitization work @IshitaSingh0822! The sanitize.ts utility and validateTextInput function are genuinely useful. However, there are some significant issues with the scope of changes:

Problems that need fixing:

  1. Removes output: "standalone" from next.config.mjs — This is required for Docker deployments. Please restore it.

  2. CSP headers are too restrictive — The script-src 'self' will block Next.js's inline hydration scripts. At minimum you need 'unsafe-inline' or a nonce-based approach. Also missing WakaTime, GitHub avatar CDN sources for images/connect.

  3. Gutted goals/route.ts — The PR removes recurrence logic (none/weekly/monthly), deadline support, MAX_GOALS_PER_USER limit, resolveAppUser, and the full Goal interface. These features need to be preserved while still applying sanitization.

  4. Heavily simplified GoalTracker.tsx — Removes recurrence picker, deadline input, confetti burst, and per-field error display that currently exist.

  5. Playwright smoke test failing

What to keep:

  • src/lib/sanitize.ts (looks good ✓)
  • The idea of calling validateTextInput on user inputs

Please scope the PR to only: (1) add sanitize.ts, (2) apply validateTextInput to goal title/unit inputs in the existing route without removing any logic. Revert the next.config.mjs changes entirely.

Copy link
Copy Markdown
Owner

@Priyanshu-byte-coder Priyanshu-byte-coder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR has 4 merge conflicts with current main. The XSS sanitization approach also needs discussion — blanket DOMPurify sanitization on all inputs can break legitimate use cases. Please rebase and scope the fix more narrowly (only apply sanitization at render/output points, not at input change handlers).

@Priyanshu-byte-coder Priyanshu-byte-coder added gssoc:approved GSSoC: PR approved for scoring level:advanced GSSoC: Advanced difficulty (55 pts) labels May 28, 2026
@Priyanshu-byte-coder
Copy link
Copy Markdown
Owner

Your PR has conflicts with the current main branch. Please rebase on main to resolve conflicts and this can be reviewed for merging. Labels have been added ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gssoc:approved GSSoC: PR approved for scoring gssoc26 GSSoC 2026 contribution level:advanced GSSoC: Advanced difficulty (55 pts) type:bug GSSoC type bonus: bug fix type:security GSSoC type bonus: security (+20 pts)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants