Skip to content

Conversation

@Napalys
Copy link
Contributor

@Napalys Napalys commented Mar 14, 2025

Included escape and unescape functions as additional flow steps.

@github-actions github-actions bot added the JS label Mar 14, 2025
@Napalys Napalys marked this pull request as ready for review March 14, 2025 13:32
Copilot AI review requested due to automatic review settings March 14, 2025 13:32
@Napalys Napalys requested a review from a team as a code owner March 14, 2025 13:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for handling the escape and unescape functions in test cases for tainted paths.

  • Added test cases in TaintedPath.js to validate flows involving escape() and unescape()
  • Updated change notes to document the new flow steps

Reviewed Changes

Copilot reviewed 2 out of 4 changed files in this pull request and generated no comments.

File Description
javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.js Added test cases using escape() and unescape() to trigger security alerts
javascript/ql/lib/change-notes/2025-03-14-escape.md Documented the support added for escape() and unescape() flows
Files not reviewed (2)
  • javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll: Language not supported
  • javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected: Language not supported

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

@asgerf
Copy link
Contributor

asgerf commented Mar 14, 2025

Could you also add escape as a StringManipulationTaintStep like unescape, and then add it as a sanitizer for XSS?

You can look at UriEncodingSanitizer in Xss.qll as an example of how to add sanitisers to all the XSS queries.

…athCustomizations.qll

Co-authored-by: Asger F <asgerf@github.com>
@Napalys Napalys merged commit 749a056 into github:main Mar 17, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants