Skip to content

Conversation

@MathiasVP
Copy link
Collaborator

@MathiasVP MathiasVP commented Oct 1, 2024

The current cs/path-injection query explicitly disables String.StartsWith and String.EndsWith as sanitizers. Presumably because a prefix check such as path.StartsWith("C:/Foo") doesn't gurantee that the path points into the Foo directory if the path contains .. (for example, C:/Foo/../Bar would satisfy the prefix check, but would point to the Bar directory).

However, if the path has been "normalized" (i.e., made absolute) by a call such as Path.GetFullPath(path) then no .. are present in the path, and the EndsWith or StartsWith checks should be sufficient to avoid path injections.

This PR fixes this by introducing a flow state to represent whether the path is normalized, allowing StartsWith and EndsWith to act as barriers when the path is in a normalized state.

Commit-by-commit review recommended:

  • Commit 1 adds a FP testcase
  • Commit 2 adds a flow state to the dataflow configuration that tracks whether the path has gone through a Path.GetFullPath call
  • Commit 3 makes the previous explicitly-banned barriers block flow when the path is normalized
  • Commit 4 accepts the test changes

@MathiasVP MathiasVP merged commit 953bd09 into main Oct 2, 2024
bdrodes pushed a commit that referenced this pull request Jan 9, 2025
Add rule to detect cases where CodeQL default setup could be used instead of advanced setup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants