fix: URL validation in downloadFileFromURL for CodeQL SSRF recognition #406
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… CodeQL Addresses CodeQL SSRF alert (java/ssrf) by: - Validating URL immediately before network request (CodeQL data flow recognition) - Reconstructing URL using whitelist host instead of user-provided host - Using validated URL for openStream() to ensure sanitized data is used This defense-in-depth approach ensures CodeQL recognizes the URL is validated and sanitized before being used in the network request. Fixes CodeQL alert: Potential server-side request forgery
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances SSRF protection in the Maven wrapper downloader by adding URL validation and reconstruction directly in the downloadFileFromURL() method. The changes are designed to help CodeQL's static analysis recognize the security guard by placing validation immediately before the network operation and reconstructing the URL from trusted whitelist data rather than using user-provided input directly.
Key Changes:
- Added validation call to
isAllowedUrl()at the start ofdownloadFileFromURL()method - Implemented URL reconstruction logic that finds the matching allowlisted host and builds a new URL using trusted data
- Modified the network operation to use the reconstructed URL instead of the user-provided URL
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses three Copilot code review suggestions: 1. Port validation: Only allow default HTTPS port (443), reject non-standard ports to prevent SSRF attacks via non-standard ports. Always use -1 when constructing URL. 2. Optimize host lookup: Use pre-computed reverse mapping (CANONICALIZED_TO_ORIGINAL) instead of loop with repeated canonicalization calls. 3. Defensive null check: Kept for code clarity and safety, with explanatory comment. These changes improve security (port validation) and performance (optimized lookup).
Remove the null check for allowedHost since isAllowedUrl() validation guarantees the canonicalized host exists in CANONICALIZED_TO_ORIGINAL map. The check was technically unreachable code. This simplifies the code while maintaining the same security guarantees.
Remove the null check for allowedHost since isAllowedUrl() validation guarantees the canonicalized host exists in CANONICALIZED_TO_ORIGINAL map. The check was technically unreachable code. This simplifies the code while maintaining the same security guarantees.
DerekRoberts
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes a CodeQL SSRF alert by adding URL validation and reconstruction in the
downloadFileFromURL()method. CodeQL's data flow analysis requires validation to occur immediately before the network request to recognize the security guard.Changes
downloadFileFromURL()methodopenStream()to ensure sanitized data is usedType of Change
Security
Fixes CodeQL alert: Potential server-side request forgery (java/ssrf)
This is a defense-in-depth approach that ensures CodeQL's static analysis recognizes:
Testing
isAllowedUrl()remains intactmain()and again indownloadFileFromURL()for CodeQL recognitionThanks for the PR!
Deployments, as required, will be available below:
Please create PRs in draft mode. Mark as ready to enable:
After merge, new images are deployed in: