-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix(isURL): fix CVE-2025-56200 #2610
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
rollup didn't work anymore locally so I've updated it
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2610 +/- ##
===========================================
- Coverage 100.00% 99.14% -0.86%
===========================================
Files 114 114
Lines 2536 2583 +47
Branches 642 657 +15
===========================================
+ Hits 2536 2561 +25
- Misses 0 13 +13
- Partials 0 9 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
if ( | ||
!parsedUrl.hostname && | ||
hasProtocol && | ||
originalUrl.indexOf('://') === originalUrl.length - 3 && |
Check failure
Code scanning / CodeQL
Incorrect suffix check High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
To fix the problem in line 222, ensure that the result of indexOf('://')
is not -1
before making a position comparison. The best practice would be:
- Store
originalUrl.indexOf('://')
in a variable (e.g.,protoIdx
). - Check that
protoIdx !== -1 && protoIdx === originalUrl.length - 3
This avoids the case where both sides are -1
due to missing substring, and thus avoids a false positive. Restrict the change to just the affected region around line 222.
No new methods or complex imports are needed—just a minor code change to add the variable and condition.
-
Copy modified line R219 -
Copy modified lines R223-R224
@@ -216,10 +216,12 @@ | ||
|
||
// Handle special case for URLs ending with just protocol:// (should always fail) | ||
// But allow URLs like file:/// that have paths | ||
const protoIdx = originalUrl.indexOf('://'); | ||
if ( | ||
!parsedUrl.hostname && | ||
hasProtocol && | ||
originalUrl.indexOf('://') === originalUrl.length - 3 && | ||
protoIdx !== -1 && | ||
protoIdx === originalUrl.length - 3 && | ||
(!parsedUrl.pathname || parsedUrl.pathname === '/') | ||
) { | ||
return false; |
This is a fix for CVE-2025-56200. Additional test cases are partially generated by generative AI.
The rewrite was done by a different generative AI process, and needs to be properly reviewed before this PR is ready
Checklist