[Python] Query help: False negatives with kwargs

[UpsideDownFoxxo]

I've been trying to pin down a false negative I had concerning dictionary arguments in function calls. The surrounding conditions for selecting the function in source and sink were different, but I have replaced them here for convenience.

/**
 * @kind path-problem
 * @id broken-python-kwargs
 */

import python

import semmle.python.ApiGraphs
import semmle.python.Concepts
import semmle.python.dataflow.new.TaintTracking

module KwArgsFlow implements DataFlow::ConfigSig {
    predicate isSource(DataFlow::Node node) {
        exists(Call call, Name name | 
            // call is to source
            call.getFunc() = name and name.getId() = "source" 
            // node is the returned value
            and call = node.asExpr()
        )
    }

    predicate isSink(DataFlow::Node node) {
        exists(Call call, Name name |
            // call is to a sink
            call.getFunc() = name and name.getId() in ["sink" ,"sink2"]
            // node is an arg or kwarg
            and (
                call.getAnArg() = node.asExpr()
                or call.getKwargs() = node.asExpr()
            )
        )
    }
}

module Flow = TaintTracking::Global<KwArgsFlow>;
import Flow::PathGraph

from Flow::PathNode input, Flow::PathNode output
where Flow::flowPath(input,output)
select output.getNode(), input, output, "This does not work"

I have constructed the following toy program to test my query on.

def source():
    return "boo"

def sink(val):
    print(val)

def sink2(**kwargs):
    print(kwargs["key"])

def middleman(**kwargs):
    sink(kwargs["key"])

# sanity check, is not flagged
sink(source())

# named arg through helper, is flagged
middleman(key=source())

# temporary dictionary storage, is flagged
a = {}
a["key"] = source()
sink(a["key"])

# passed through kwargs, is flagged
middleman(**a)

# passed as kwargs, is not flagged
sink2(**a)

I have two questions here:

• why is sink(source()) not flagged? The respective start/end predicates for the flow correclty select them, how is the dataflow between them not detected?
• why is sink2(**a) not flagged? (Suspected: Technically a itself is not tainted, it just has a tainted KV-pair. However I cannot seem to drill down enough to detect that.)

[hvitved]

Hi 👋

This is weird; it works just fine when I test it using codeql test run python/ql/test/library-tests/temp.

[hvitved]

Sorry, the above was a bit too fast: the sink(source()) case is flagged, but the sink(**a) case needs a bit extra to be flagged (included in my branch); the a["key"] = source() assignment is modeled as what we call a store step, where the right-hand side is stored into a at the key "key". Store steps need to be matched by read steps (such as print(kwargs["key"])), which is not the case when data reaches sink2(**a). However, it is possible to simulate implicit read steps, using predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c), which you can see in my branch.

[hvitved]

Note also that #17493 would remove the need for allowImplicitRead.