isAdditionalTaintStep() required for a StaticMethod ?

[Sim4n6]

Hi, I have this unusual case where a bird-view of the problem is as follow:

class Art
      @staticmethod
      def down(in1, btw):
             // the source fct_source() is being identified and it is within a static method.
             fct_source(in1,btw)

      @staticmethod 
      def copy(btw, out1):
             // the sink is supposed to be inside another static method  
             fct_sink(btw,out1)             

       @staticmethod 
       def call():
             Art.down(in1,btw)
             Art.copy(btw, out1)

Now, what I did is a very basic CodeQL query as follow:

class Config extends TaintTracking::Configuration {
  Config() { this = "Config" }

  override predicate isSource(DataFlow::Node source) {
         // catch fct_source()
         source = API::moduleimport("fct_source").getACall().getArg(0)
  }

  override predicate isSink(DataFlow::Node sink) {
    // catch out1 of fct_sink(), for instance
     sink = API::moduleimport("fct_sink").getACall().getArg(1)
      
  }
}

from Config config, DataFlow::PathNode source, DataFlow::PathNode sink
where config.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "There is a way between the source and the sink !"

My problem is there is no found way between the source and the sink despite the fact that a btw parameter exists ...
I think I need to add an isAdditionalTaintStep() somehow but I am a bit lost, please? could you help me out?

[Sim4n6]

It seems that the use of @staticmethod is somehow blocking the flow... any idea how to proceed in such a case, please?

[atorralba]

Your source is first argument of fct_source, i.e. in1. Without knowing what fct_source does, it's expected that flow ends there, because the function doesn't return anything, btw is unaffected, and in1 isn't used anywhere else.

Additionally, your sink is the second argument of fct_sink, which is out1. out1 isn't affected at all by either in1 or btw, unless you expect that fct_source taints btw from in1, and that fct_sink taints out1 from btw. If that's the case, you need an additional taint step from arg0 to arg1 in Array.down, and your actual sink should be arg0 of fct_sink.

Also note that, in your snippet, neither in1, nor btw, nor out1 are defined before use.

[Sim4n6]

how do you suggest writing the query for the additional taint step from arg0 to arg1 in Art.down(), please ?

[atorralba]

You would do something like this:

override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
  exists(CallNode c, Value v | v.getName() = "down" and v.getACall() = c |
    n1.asCfgNode() = c.getArg(0) and
    n2.asCfgNode() = c.getArg(1)
  )
}

But now that I look more detailedly into it, that won't be enough, because your source is inside down, and there's no flow going out of that function, even if you add another taint step to taint the second argument of fct_source from the first (which would also be needed if that's what you want).

At this point, I'll defer to @github/codeql-python since I don't know enough about the CodeQL libraries for Python to answer confidently.

[yoff]

As mentioned by @atorralba, there is no way to conclude the flow you are expecting without knowing something about fct_source and fct_sink. If I write the program

class Foo:
    def __init__(self, foo=NONSOURCE):
        self.foo = foo

def prop_source(in1, btw):
    btw.foo = in1.foo

def prop_sink(btw, out1):
    out1.foo = btw.foo

def test_prop():
    in1 = Foo(SOURCE)
    btw = Foo()
    out1 = Foo()
    prop_source(in1,btw)
    prop_sink(btw,out1)
    SINK(out1.foo) # $ flow="SOURCE, l:-5 -> out1.foo"

class ArtProp:
    def __init__(self, foo=NONSOURCE):
        self.foo = foo

    @staticmethod
    def down(in1, btw):
        prop_source(in1, btw)

    @staticmethod
    def copy(btw, out1):
        prop_sink(btw, out1)

    @staticmethod
    def call():
        in1 = Foo(SOURCE)
        btw = Foo()
        out1 = Foo()
        ArtProp.down(in1,btw)
        ArtProp.copy(btw, out1)
        SINK(out1.foo) # $ flow="SOURCE, l:-5 -> out1.foo"

def test_prop_static():
    ArtProp.call()

then both the annotated lines receive taint.
This works because arguments have a post-update nodes that record the change happening inside functions to the object the argument points to.
If you need to specify this effect manually, you can write

override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
    exists(DataFlow::CallCfgNode c |
      c = API::moduleImport(...).getMember(...).getACall() // use this if you have a nice API expression for the calls
      c.getFunction().asExpr().(Name).getId() = ["fct_source", "fct_sink"] // use this as a raw alternative
    |
      n1 = c.getArg(0) and
      n2.(DataFlow::PostUpdateNode).getPreUpdateNode() = c.getArg(1)
    )
  }

where you pick the line defining c that you can support.