Skip to content

Conversation

@geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Aug 14, 2025

Update StartswithCall to use getCanonicalPath. The solution was written by @hvitved in #20224 . I've added test coverage (in particular the annotation path-injection-checked), and this shows that the new solution sometimes works - suggesting that we have the right expression but probably type inference is lost in many cases (e.g. at a prior canonicalize).

@geoffw0 geoffw0 requested a review from a team as a code owner August 14, 2025 11:22
Copilot AI review requested due to automatic review settings August 14, 2025 11:22
@geoffw0 geoffw0 added no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code labels Aug 14, 2025
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 updates the StartswithCall class in Rust CodeQL to use getCanonicalPath for method resolution and adds comprehensive test coverage for path injection scenarios. The change improves the reliability of detecting safe path access checks by using a more robust method resolution approach.

Key changes:

  • Updates method resolution in StartswithCall from getResolvedPath() to getStaticTarget().getCanonicalPath()
  • Adds extensive test cases covering various path injection scenarios including canonicalization patterns
  • Enhances test infrastructure to track barriers, normalizers, and safe access checks

Reviewed Changes

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

File Description
rust/ql/lib/codeql/rust/frameworks/stdlib/Stdlib.qll Updates StartswithCall to use getCanonicalPath for more reliable method resolution
rust/ql/test/query-tests/security/CWE-022/src/main.rs Adds comprehensive test cases with annotations for path injection scenarios
rust/ql/test/query-tests/security/CWE-022/TaintedPathSinks.ql Expands test infrastructure to track additional path injection elements

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

No need to run DCA; #20224 already did that.

@geoffw0 geoffw0 merged commit 6951f58 into github:main Aug 14, 2025
23 checks passed
@geoffw0
Copy link
Contributor Author

geoffw0 commented Aug 14, 2025

Thanks.

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