- 
          
- 
                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 16 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