fix(utils.url): require boundary char in withBase prefix match (#564)#576
fix(utils.url): require boundary char in withBase prefix match (#564)#576SAY-5 wants to merge 1 commit into
Conversation
) withBase previously short-circuited and returned the input unchanged whenever the input merely had the base string as a prefix. An attacker controlling the input could craft a URL like http://api.internal.attacker.com/steal that satisfies startsWith for the base http://api.internal, bypassing the join and reaching an arbitrary host (CWE-918 SSRF / authority-override). The prefix match now requires the next character after the base to be a URL boundary (/, ?, #) or end-of-string. Adds 8 unit tests covering the existing behaviour and the new boundary guard. Signed-off-by: SAY-5 <say.apm35@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR fixes a Server-Side Request Forgery (SSRF) vulnerability in ChangesSSRF Boundary Validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Fixes #564.
withBase(input, base)previously returnedinputunchanged wheneverinput.startsWith(base)was true. An attacker-controlled input likehttp://api.internal.attacker.com/stealsatisfiesstartsWith("http://api.internal")and bypasses the base-URL enforcement, reaching an arbitrary host (CWE-918 SSRF / authority-override).This change requires the character after the base to be a URL boundary (
/,?,#) or end-of-string before short-circuiting. Otherwise the input is joined with the base as a path segment, matching what callers usingbaseURLexpect.Adds
test/utils.url.test.tswith 8 unit tests covering existing behaviour (/,?,#, exact match, no-base, base-less input) and the new boundary guard. The regression test fails on the pre-fix code and passes after.Full suite:
pnpm vitest run-> 36/36 passing.Summary by CodeRabbit
Bug Fixes
Tests