[Go] DataFlow modeling for wrapped/argument functions

[rh-tguittet]

Hi,

I am trying to identify why or how I could get the following dataflow from source()'s result to the exec.Command() for the following type of programs:

package main

import "os/exec"

type runnerFunc func(command []string) error

type Executer struct {
	runner runnerFunc
}

func NewExecuter() *Executer {
	return &Executer{
		runner: run,
	}
}

func run(command []string) error {
	cmd := exec.Command(command[0], command[1:]...) // This is our sink
	return cmd.Run()
}

func source() []string {
	return []string{"some", "command"}
}

func wrapExecWithRetry(f func() error, retryCount int) {
	err := f()

	for i := 0; i < retryCount && err != nil; i++ {
		err = f()
	}

	if err != nil {
		panic(err)
	}
}

func main() {
	executer := NewExecuter()
	cmd := source()

	wrapExecWithRetry(func() error {
		return executer.runner(cmd)
	}, 2)
}

Debugging using partial flow, I can see cmd is correctly tracked to the return executer.runner(cmd) line but not after. (I tried doing an additional field write step but as it happens outside of the flow I am interesting in it couldn't work.)

/**
 * @name testing
 * @kind path-problem
 * @severity warning
 * @security-severity 8
 * @id testing
 */

import go
import semmle.go.security.CommandInjection

private module MyFlowImpl implements DataFlow::ConfigSig {
  predicate isSource(DataFlow::Node node) {
    exists(Function f |
      f.getName() = "source" and
      f.getACall().getResult() = node
    )
  }

  predicate isSink(DataFlow::Node node) { node instanceof CommandInjection::Sink }
}

module MyFlow = TaintTracking::Global<MyFlowImpl>;

import MyFlow::PathGraph

int explorationLimit() { result = 5 }

module MyPartialFlow = MyFlow::FlowExplorationFwd<explorationLimit/0>;

predicate adhocPartialFlow(Callable c, MyPartialFlow::PartialPathNode n, DataFlow::Node src, int dist) {
  exists(MyPartialFlow::PartialPathNode source |
    MyPartialFlow::partialFlow(source, n, dist) and
    src = source.getNode() and
    c = n.getNode().getEnclosingCallable()
  )
}

from MyFlow::PathNode src, MyFlow::PathNode sink
where MyFlow::flowPath(src, sink)
select sink, src, sink, "$@ flows to $@", src.getNode(), "src", sink.getNode(), "sink"

I somewhat expected CodeQL to resolve that wrapping/function passing automatically, but since it didn't here I question if it is a bug or simply something missing from my query.

[smowton]

No, this doesn't work at the moment in Go. This scenario was successfully implemented a few months ago for Java, but we haven't had the time yet to port the solution to the Go QL library.

To work around the problem, you would need to use the isAdditionalFlowStep predicate of MyFlowImpl to implement logic such as, for a call to wrapExecWithRetry, anything that flows to a return node of its argument also flows to the callsite of wrapExecWithRetry -- or, to make that more general, replace wrapExecWithRetry with any function with a lambda parameter where the return value of that lambda is itself returned.

If you are still struggling to get this to do what you want, post your implementation of isAdditionalFlowStep and I can advise how to improve it.

[rh-tguittet]

Thanks Chris, got that working.