fix(security): validate port in CSRF origin check (#7212) - #7355
Conversation
- Add normalizePort() helper that resolves empty URL.port to the protocol default (443 for https, 80 for http) - Compare normalized ports in isValidOrigin so requests from mismatched ports (e.g. :9999) are rejected even when hostname and protocol match - Add 15 unit tests covering: port mismatch, default port normalization, env-based custom ports, referer fallback, malformed origin, and combined origin+referer scenarios
📦 Next.js Bundle Size Report (Gzipped Sizes)✨ No significant bundle size changes detected. 📊 Summary of Totals
|
Aamod007
left a comment
There was a problem hiding this comment.
Hey! Thanks for addressing the CSRF origin validation vulnerability. Restricting origin checks down to the precise port blocks a major attack vector in local or shared-hosting environments.
I'm assigning level:advanced, type:security, and quality:clean.
Fantastic catch, approving!
|
🎉 Congratulations @atul-upadhyay-7! Your PR has been successfully merged. 🚀 Thank you for contributing to CommitPulse. Your work helps us build a better tool for the community.
Keep building! 💻✨ |
Fixes #7212
Problem
The
isValidOriginfunction inlib/security/csrf.tsonly checkedprotocolandhostnamewhen validating theOriginheader, ignoring the port entirely. This allowed CSRF bypass from any server sharing the same hostname on a different port (e.g.Origin: https://commitpulse.vercel.app:9999).Solution
normalizePort()helper that resolves emptyURL.portto the protocol default (443 for https, 80 for http), sinceURL.portreturns empty string for default portsisValidOriginto compare normalized ports alongside protocol and hostnamelib/security/csrf.test.tscovering::443matches no-port for HTTPS)NEXT_PUBLIC_SITE_URLenv varChanges
lib/security/csrf.ts: AddednormalizePort(), updatedisValidOrigin()port comparisonlib/security/csrf.test.ts: New test file with 15 test cases