Skip to content

Rust: Update DotDotCheck to use getCanonicalPath #19804

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

Merged
merged 3 commits into from
Jul 25, 2025
Merged

Conversation

geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Jun 17, 2025

Update DotDotCheck to use getCanonicalPath rather than getResolvedPath.

This is not working because (1) we're not finding the static targets of these method calls (presumably because type inference is not quite getting there) and (2) I've been unable to test effectively due to (1), so I'm not even sure the model I've written is correct. I should be able to finish this when type inference is working better in other places, e.g. #19802 . --- merged in latest main, fixed the paths, and it works now. :)

@geoffw0 geoffw0 added the Rust Pull requests that update Rust code label Jun 17, 2025
@geoffw0 geoffw0 added the no-change-note-required This PR does not need a change note label Jun 25, 2025
@geoffw0 geoffw0 marked this pull request as ready for review July 24, 2025 16:25
@Copilot Copilot AI review requested due to automatic review settings July 24, 2025 16:25
@geoffw0 geoffw0 requested a review from a team as a code owner July 24, 2025 16:25
Copy link
Contributor

@Copilot 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 updates the DotDotCheck class in the Rust security library to use getCanonicalPath instead of getResolvedPath for identifying string contains method calls. This change aims to improve the detection of path traversal sanitization checks by using a more reliable method resolution approach.

  • Updates method resolution from getResolvedPath() to getStaticTarget().getCanonicalPath()
  • Changes the target string matching to include both <alloc::string::String>::contains and <core::str>::contains
  • Modifies the type casting from Resolvable to CallExprBase to support the new resolution method

Copy link
Contributor

@paldepind paldepind left a comment

Choose a reason for hiding this comment

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

Looks good. Nice that the QL libraries are now good enough that this works.

@geoffw0
Copy link
Contributor Author

geoffw0 commented Jul 25, 2025

Nice that the QL libraries are now good enough that this works.

100%

@geoffw0 geoffw0 merged commit 4b947db into github:main Jul 25, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants