feat(XSS): also scan filenames for XSS #2730
Closed
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.
This PR addresses #2699.
Bug Bounty references: ULYKOFYK-1, EOJQZ6XX.
It will solve false negatives such as
/index.php/%3Csvg/onload=alert()
I've reviewed our XSS rules, and on first sight, the rules seem specific enough to allow us passing the filename to them and not be flooded by false positives.
Added REQUEST_FILENAME to all XSS rules, except for
@detectXSS
, which if I recall correctly, I tried earlier to run on the filename but had to revert because the number of false positives was not tolerable for a default install. It is active in PL2 though.REQUEST_FILENAME (not REQUEST_URI) seems good enough to me to add to the XSS rules, as the additional query parameters in REQUEST_URI are already passed to the rules through ARGS_NAMES and ARGS.
Now there is a chance that we find false positives, so that maybe we have to remove the filename scan from some of the rules. To improve confidence, I can run these rules on some traffic and see how it behaves.