Displaying path query results in VS Code: alerts doesn't do anything

[adriaanjacobs]

I run a path query closely following the documentation for it, yet I can't seem to figure out how to display the path from source to sink in VS Code using the CodeQL extension.

I imported the DataFlow::PathGraph module and my query looks like this:

from DataFlow::PathNode address, DataFlow::PathNode condjump, AddressToCondJumpConfig config
where config.hasFlowPath(address, condjump)
select address, "taints", condjump

Now according to the documentation the select clause should contain 4 columns element, source, sink, string but even if I do that it doesn't show the path.

It is mentioned I should drop down the 'Results' box and hit alerts, but that doesn't do anything. I'm able to click on 'nodes' and 'edges' though, but I can't make sense of what is displayed there and to which query result it is related (a lot of entries there).

Is there something else I should do / could try?

I'm using the coreutils mirror on github downloaded from lgtm.com

Full query included below:

/**
 * @name address leaking into condjump
 * @kind path-problem
 * @problem.severity warning
 * @id addresstaintscondjump
 */

import cpp
import semmle.code.cpp.dataflow.TaintTracking
import DataFlow::PathGraph
import semmle.code.cpp.controlflow.Dereferenced

predicate isAddressType(Type type) {
    type instanceof PointerType or type instanceof ArrayType
}

predicate isNullCheck(Expr expr) {
    exists(NotExpr n | expr = n and isAddressType
        (n.getAnOperand().getUnderlyingType()))
}

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

    override predicate isSource(DataFlow::Node source) {
        isAddressType
        (source.asExpr().getUnderlyingType())
    }

    override predicate isSanitizer(DataFlow::Node intermediate) {
        dereferenced(intermediate.asExpr())
        or exists(SizeofOperator sizeof | sizeof.getAChild+() = intermediate.asExpr()) // I should probably cover more unevaluated contexts, but it's a start
        or intermediate.asExpr() instanceof PointerDiffExpr
        or exists(ArrayExpr arrayaccess | intermediate.asExpr() = arrayaccess
            and not isAddressType(arrayaccess.getUnderlyingType())
        )
        or isNullCheck(intermediate.asExpr())
    }

    override predicate isSink(DataFlow::Node sink) {
        (sink.asExpr().isCondition() 
            or sink.asExpr().getEnclosingStmt() instanceof ConditionalStmt
            or exists(Loop loop | loop.getCondition() = sink.asExpr()))
        and not isAddressType(sink.asExpr().getUnderlyingType())
    }
}

from DataFlow::PathNode address, DataFlow::PathNode condjump, AddressToCondJumpConfig config
where config.hasFlowPath(address, condjump)
select address, "taints", condjump

[intrigus-lgtm]

Did you add

/**
 * ...
 * @kind path-problem
 * ...
 */

to the start of your query?
If I remember correctly that is also needed.

[adriaanjacobs]

Oh yes, I forgot to mention, that's also there. Could you show a screenshot of what it's supposed to look like? I think it displays all the nodes in the Results view, the ordering just seems off (I can't make sense of it).

I have included the full query in my edited answer.

[p0]

I think your select clause doesn't follow the expectations. You probably want something like this: select condjump, address, condjump, "tainted by $@", address, address.toString(). See also here for the full details.

We should really display a better error here, rather than silently producing no alerts...

[p0]

That's very odd. Could you share the database somewhere so I can try to reproduce? Any error logs would be in the extension output pane I mentioned earlier.

[adriaanjacobs]

I've sent you the project on Slack, I hope that's alright. The extension output log shows this only once, at the very bottom:

'[2021-02-20 13:04:37] CSV_IMB_QUERIES: Query,edges#query#ffffffffffffff nodes#query#fffffffff #select#query#ffffffffffffffffffffff,addresstocondjump.ql,23,Success,30.134,437303,30365
Exception during results interpretation: Reading output of interpretation failed: Error: Cannot create a string longer than 0x1fffffe8 characters. Will show raw results instead.'

[p0]

'[2021-02-20 13:04:37] CSV_IMB_QUERIES: Query,edges#query#ffffffffffffff nodes#query#fffffffff #select#query#ffffffffffffffffffffff,addresstocondjump.ql,23,Success,30.134,437303,30365
Exception during results interpretation: Reading output of interpretation failed: Error: Cannot create a string longer than 0x1fffffe8 characters. Will show raw results instead.'

Thanks, that's the root cause of the issue -- we're hitting a Node limitation on string length, and the CodeQL extension is falling back on raw results display (which means showing the different query predicates -- #select, edges and so on -- rather than the alerts view).

As for why we trigger a string length limit: The query as written tuns out to have way too many results on glibc (hundreds of thousands). There are some sinks that are tainted by very very many sources, and for such cases the alerts view currently attempts to construct an alert message string that consists of the messages from the select for each such source; this is what goes wrong.

To confirm, try running with the following rewrite of the select (which arbitrarily restricts to at most 4 sources per sink):

from
  DataFlow::PathNode address, DataFlow::PathNode condjump, AddressToCondJumpConfig config,
  int numSources, int numToReport, string extra
where
  address =
    rank[[1 .. numToReport]](DataFlow::PathNode node |
      config.hasFlowPath(node, condjump)
    |
      node
      order by
        node.getNode().getLocation().getFile().getAbsolutePath(),
        node.getNode().getLocation().getStartLine()
    ) and
  numSources = count(DataFlow::PathNode node | config.hasFlowPath(node, condjump)) and
  numToReport = 4 and
  if numSources > numToReport
  then extra = " and " + (numSources - numToReport) + " more"
  else extra = ""
select condjump, address, condjump, "tainted by " + address.toString() + extra

You should see that this does render in the alerts view.

---

Why are there so many results, and so many source/sink pairs? It turns out the worst offender is in a macro-generated location, which makes it hard to understand the flows. Looking at the query, though, you are asking to track from any expression whose type is in isAddressType to any expression that is enclosed in an if, or is a condition itself. This is likely to have multiple results for any condition that works with pointers or arrays. I would strongly recommend strengthening your definition of source: Perhaps you're interested in function parameters whose types are addresses, or in addresses that come from particular APIs?

---

Still, I think we could do a better job reporting why the alerts view is non-functional in your setup. I'll see if we can make that happen.

[p0]

I've filed github/vscode-codeql#759 for this.

[adriaanjacobs]

Yes, limiting the amount of sources that flow into the same sink has been on my todo-list.
Thank you so much for helping me get started with that and resolving this issue, as well as providing a detailed explanation of what's going on under the hood.