We changed the test query when the query was changed so that the
comments in the test file would stay the same.
I've reverted the test query and updated the comments in the test file.
This avoids problems in the branch switching to use-use flow.
The extra nodes in .expected files are due to the changes from
https://github.com/github/codeql/pull/13717, which are not applied to
configuration classes extending DataFlow::Configuration or
TaintTracking::Configuration.
Removed edges were only there originally due to multiple configurations
being in scope. `DataFlow::PathNode` has union semantics for
configurations. Nodes are only generated if they are reachable from a
source, but this includes sources from other configurations.
No alerts are lost.
The extra nodes in .expected files are due to the changes from
https://github.com/github/codeql/pull/13717, which are not applied to
configuration classes extending DataFlow::Configuration or
TaintTracking::Configuration.
The extra nodes in .expected files are due to the changes from
https://github.com/github/codeql/pull/13717, which are not applied to
configuration classes extending DataFlow::Configuration or
TaintTracking::Configuration.
Removed nodes and edges were only there originally due to multiple
configurations being in scope. `DataFlow::PathNode` has union semantics
for configurations. Nodes are only generated if they are reachable from
a source, but this includes sources from other configurations.
The extra nodes in .expected files are due to the changes from
https://github.com/github/codeql/pull/13717, which are not applied to
configuration classes extending DataFlow::Configuration or
TaintTracking::Configuration.
The extra nodes in .expected files are due to the changes from
https://github.com/github/codeql/pull/13717, which are not applied to
configuration classes extending DataFlow::Configuration or
TaintTracking::Configuration.
This was amended as part of https://github.com/github/codeql/pull/12186, but the conversion was inadequate because the new implementation didn't work when a sink (type conversion) led directly to a non-`localTaintStep` step, such as a store step or an interprocedural step. Here I move the sink back one step to the argument of the type
conversion and sanitize the result of the conversion instead, to ensure there is always a unique local successor to a sink.
This should eliminate unexpected extra results that resulted from https://github.com/github/codeql/pull/12186. Independently there are also *lost* results that stem from needing a higher `fieldFlowBranchLimit` that are not addressed in this PR, but raising that limit is a performance risk and so I will address this separately.
In https://github.com/github/codeql/pull/8641, `localFlowExit` was
changed to use `Stage2::readStepCand` instead of `read`, which means
that the big-step relation is broken up less. This causes test result
changes. Nothing is lost from the `select` clause, but some results may
have fewer paths, and fewer nodes and edges are output in the test
results.
In the `subpaths` section, the last node is now printed without its type
if it is the sink of the path.
This comes from the commit "Dataflow: Bugfix: include subpaths ending at
a sink. " in https://github.com/github/codeql/pull/7526
This query already treats structs differently to usual: it includes field -> whole struct taint steps, but explicitly excludes struct -> field steps. This means that a logging framework sinking an entire struct with a tainted field yields an alert, but we don't get FPs caused by writing field `x` but then reading field `y`.
However, protobuf messages have a special treatment, with taint usually associated with the whole struct and getter methods propagating that taint out. Suppressing these getter method steps specifically for the cleartext-logging query mirrors its treatment of structs in general and avoids this sort of field-mismatch FP.
On the downside we will miss same-field propagation like `m.field = password; Log(m.GetField())` if we don't have source code for the implementation of `m`. However this is hopefully unusual since the typical use of protobufs is to serialize and deserialize, rather than using the struct as a general-purpose datastructure.
