[Go] How to query for a function that is passed as a variable?

[JasperSurmont]

Example:

type A struct {
    Handler func(int)
}

var as = []A{
    {
        Handler: someFunc
    },
    {
        Handler: someFunc2
    },
    ...
}

for i := 0; i < len(as); i++{
    as[i].Handler(i)
}

I want the query to return all the functions that are being called by the Handler: someFunc , someFunc2, etc. How would I go about achieving this?

[JasperSurmont]

How I am currently solving this is by querying for all the FunctionNames that are the value in a KeyValueExpr. I then must match the VariableName to a predefined name (in my example this would be as).

However, this method is not flexible at all: e.g. every change to structure of the variable as messes up this query. Iḿ hoping to get a bit of guidance on how I could create a more robust query.

[JasperSurmont]

I'm looking for the flow of a specified function (like in your elaborated example user) to any other function (also 'indirect' functions called like in the example). In my project, this specified function has other restrictions (e.g. no platform dependent variables like int), so I'd use this characteristic to then match several other.

However, with my limited knowledge I couldn't get it to work and thought the reason was the 'indirection' becsuse the source code never explicitly calls the function.

Maybe I just did something silly and it is really not difficult at all, for which my excuses.

[smowton]

You mentioned though that the actual call is happening in an opaque third-party library, so what is the sign that a particular function pointer will go on to be executed by that library? Is the function pointer assigned to a particular field? Passed to a library function?

Could you give another small example that is more representative of your actual problem?

[JasperSurmont]

Yes of course, sorry for the confusion.

An example can be found in this repo, using the Cosmos SDK.

Here the variable _Msg_serviceDesc is 'registered'. This variable (declaration found here) contains Handlers which are function pointers.

The RegisterMsgService basicallly just sets the key-value map of the name of the handler to the handler itself.

Then, at one point, the Cosmos SDK calls runMsgs, which will eventually do:

handler := app.msgServiceRouter.Handler(msg)
if handler == nil {
	return nil, errorsmod.Wrapf(sdkerrors.ErrUnknownRequest, "can't route message %+v", msg)
}

msgResult, err := handler(ctx, msg)

What I want to achieve is querying all functions that are called when runMsgs is called, which thus also should include the Handlers passed to the _Msg_serviceDesc variable.

I hope this clears up my situation.
Thanks again for your time!

[smowton]

Depending on how precise you want to be, here's a sink that identifies any write to the field MethodDesc.Handler:

  override predicate isSink(DataFlow::Node n) {
    exists(Write w |
      w.writesField(_, any(Field f | f.hasQualifiedName("google.golang.org/grpc", "MethodDesc", "Handler")), n)
    )
  }

However perhaps you're concerned that not all MethodDesc.Handlers end up passed to RegisterService? In that case we can make our sink any argument to RegisterService:

  override predicate isSink(DataFlow::Node n) {
    n = any(DataFlow::CallNode cn |
      cn.getTarget().(Method).hasQualifiedName("github.com/gogo/protobuf/grpc", "Server", "RegisterService")
    ).getArgument(0)
  }

...then we must add additional flow steps to propagate flow from the Handler field to its enclosing MethodDesc, then to the array of MethodDescs, then to the ServiceDesc surrounding that. We can make array-writes taint-propagating (along with various other things; see the predicate defaultAdditionalTaintStep) by turning our DataFlow::Configuration into a TaintTracking::Configuration, then we can make field-writes taint-propagating by defining

  override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
    exists(Write w |
      w.writesField(toNode.(DataFlow::PostUpdateNode).getPreUpdateNode(), _, fromNode)
    )
  }

That's pretty blunt -- we could restrict it to the Handler and Methods fields by replacing the _ with an any(Field f | ...) definition like that used in our first sink definition if need be.

With those changes made analysing the juno database successfully finds that _Query_FeeShares_Handler and 19 other similarly-named handler functions make it to a RegisterService call.

[JasperSurmont]

Wow, thank you so much for your help! Exactly what I wanted to achieve!

[JasperSurmont]

Hey @smowton

I have one additional question. So your solution works as expected (currently using your first suggestion without tainttracking), but I wanted to expand it such that a query finds all functions being called in the found FunctionNode. As an example, you mentioned you found _Query_FeeShares_Handler, but I'd like to go further and also find the functions being called in there, like queryClient.FeeShare, and recursively all the funcitons called in there.

I came up with the following which kind of works, but I'm quite certain this is not a good/beautiful solution. Could you maybe guide me a little bit on how to solve this problem a bit better?

My solution:

class FunctionNodeFam extends DataFlow::FunctionNode {
    FunctionNodeFam() {this instanceof DataFlow::FunctionNode}
  
    FunctionNodeFam getAChild() {result = any(FunctionNodeFam fn | fn.getFunction().getACall().getRoot() = this.getFunction().getFuncDecl())}
 }
  
class MsgConfig extends DataFlow::Configuration {
      MsgConfig() {this = "Config of ABCI messages / transactions"}
  
      override predicate isSink(DataFlow::Node n) {
          exists(Write w |
            w.writesField(_, any(Field f | f.hasQualifiedName("google.golang.org/grpc", "MethodDesc", "Handler")), n)
          )
      }
  
      override predicate isSource(DataFlow::Node n) {
          n instanceof DataFlow::FunctionNode
      }
  }

// Final query:
from FunctionNodeFam fn, MsgQuery c
where  c.hasFlow(fn, _)
select fn.getAChild*().getFunction().getFuncDecl()

[smowton]

How about

Function reachableFromCalledService() {
  // Base case: function flows to a Handler field
  exists(Config c, DataFlow::FunctionNode serviceHandler |
    c.hasFlow(serviceHandler, _) and
    result = serviceHandler.getFunction()
  )
  or
  // Recursive case: walking the transitive call graph
  exists(DataFlow::CallNode cn |
    cn.getRoot() = reachableFromCalledService().getFuncDecl() and
    result = cn.getACalleeIncludingExternals().asFunction()
  )
}

It's good to restrict the base case like that because it ensures QL will only compute the subset of the transitive call graph you actually care about, not that of all functions.

[JasperSurmont]

Thank you! Very clean.