Skip to content

Conversation

@MathiasVP
Copy link
Collaborator

This PR fixes two FPs on the powershell/microsoft/public/sql-injection query:

  1. We were missing calls to Invoke-Sqlcmd with an InputFile named argument, and because the logic of the sink is "if we cannot find the right argument to the call to Invoke-Sqlcmd then just use the first argument" then we incorrectly marked the first argument of Invoke-Sqlcmd as the sink when it should have been the InputFile named argument. See the FP in the first commit for an example of this.
    This has been fixed in c18db91.

  2. The next FP fix requires some explanation. Consider this example (in some simplified language):

x.f = source();
sink(x);

normally there wouldn't be any path from source() to sink(x) in the above since we never read f off the x object before passing it to sink. However, sometimes we want to have a result in those scenarios. So in taint-tracking we specify certain kinds of "implicit reads" that happen at sinks.
For PowerShell I decided at some point that any kind of field could be read off implicitly at sinks. However, this gives a FP when a specific field of an object is tainted, and we only want an alert when that specific field is tainted. See 25d94fa for an example FP from this.

In 1486200 I've fixed this FP by only allowing implicit reads by default on "array index"-like values. This matches what other CodeQL languages.

Now, this means that we have to add such implicit reads at sinks specifically where we need them. So in 1ff04d9 I added a false negative which requires these implicit reads, and in 1486200 I've added support for such implicit reads.

@LWSimpkins LWSimpkins merged commit 6ab05cd into main Jun 20, 2025
3 checks passed
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.

4 participants