JS: remove encodeURI from sanitizer list of request forgery#19750
Merged
Napalys merged 4 commits intogithub:mainfrom Jun 19, 2025
Merged
JS: remove encodeURI from sanitizer list of request forgery#19750Napalys merged 4 commits intogithub:mainfrom
encodeURI from sanitizer list of request forgery#19750Napalys merged 4 commits intogithub:mainfrom
Conversation
dbaa5bd to
c2b9a5c
Compare
encodeURI from sanitizer list for xss and request forgeryencodeURI from sanitizer list of request forgery
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR removes encodeURI from the list of recognized sanitizers in request-forgery analysis, so calls to encodeURI no longer suppress alerts.
- Added a test case in
serverSide.jsto showencodeURI(input)is now flagged. - Updated the expected alerts in
RequestForgery.expectedto include the new alert forencodedUrl. - Modified
RequestForgeryCustomizations.qllto filter outencodeURIfrom being treated as a sanitizer. - Added a change note documenting the removal of
encodeURI.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| javascript/ql/test/query-tests/Security/CWE-918/serverSide.js | Added encodeURI(input) and its flagged axios.get call. |
| javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected | Updated expected alerts to include the new encodedUrl case. |
| javascript/ql/lib/semmle/javascript/security/dataflow/RequestForgeryCustomizations.qll | Excluded encodeURI calls from UriEncodingSanitizer. |
| javascript/ql/lib/change-notes/2025-06-13-remove-encodeuri.md | Documented the removal of encodeURI from the sanitizer list. |
Comments suppressed due to low confidence (2)
javascript/ql/lib/change-notes/2025-06-13-remove-encodeuri.md:4
- [nitpick] Since the analysis comment mentions both
encodeURIandencodeURIComponent, update this change note to also mention removal ofencodeURIComponentif that exclusion is intended.
* Removed `encodeURI` function from the sanitizer list for request forgery.
javascript/ql/lib/semmle/javascript/security/dataflow/RequestForgeryCustomizations.qll:114
- The comment above mentions both
encodeURIandencodeURIComponentas not valid URL sanitizers, but onlyencodeURIis excluded. Consider adding a similar exclusion forencodeURIComponentto match the documentation.
class UriEncodingSanitizer extends Sanitizer instanceof Xss::Shared::UriEncodingSanitizer {
c2b9a5c to
7a7e879
Compare
8aca630 to
0d5f510
Compare
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
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.
No description provided.