Does / will this have a corresponding bounty application?

Does / will this have a corresponding bounty application?

Yes, was planning if it is merged. Are there any differences? Does bounty application speeds up review?

No particular speed difference but it affects who does the work. In this case you should apply before merging, and you'll get accuracy comments from the security lab team.

Sorry, didn't know that. Just created it.

Path tracking ended up as a big change, but let's think big - global flow tracking :) There is one odd thing though. A query like:

from BoolToGorillaSessionOptionsTrackingConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "Cookie attribute 'HttpOnly' is not set to true."

gives me odd path:


The sink and the source are good, but the intermediate path is funny. Do I do something wrong?

No, you didn't do anything wrong. The first path is fine, though some steps are skipped that you might expect to see. The extra paths are a quirk of how to we track data flow. I think this was explained here.

Oh, and I forgot to mention that you need to regenerate the .expected file because you reformatted the .go file and that change the line numbers. (This isn't a problem for tests that use InlineExpectations, as you put comments in the code to indicate where results are expected, so they move with the reformatting.)

Oh, and my unresolved comment about changing the definition of SetCookieSink from the previous review still applies.

Oh, and my unresolved comment about changing the definition of SetCookieSink from the previous review still applies.

Do you mean this one?
I tried something like that:

private class SetCookieSink extends DataFlow::Node {
  SetCookieSink() {
    exists(DataFlow::CallNode cn |
      cn.getTarget().hasQualifiedName(package("net/http", ""), "SetCookie") and
      this = cn.getArgument(1) and
      exists(NameToNetHttpCookieTrackingConfiguration cfg, DataFlow::Node source |
        cfg.hasFlow(source, this)
      )
    )
  }
}

class NameToNetHttpCookieTrackingConfiguration extends TaintTracking2::Configuration {
  NameToNetHttpCookieTrackingConfiguration() { this = "NameToNetHttpCookieTrackingConfiguration" }

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

  override predicate isSink(DataFlow::Node sink) { sink instanceof DataFlow::Node }

  override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
    exists(StructLit sl |
      sl.getType() instanceof NetHttpCookieType and
      getValueForFieldWrite(sl, "Name") = pred and
      sl = succ.asExpr()
    )
  }
}

The sink predicate becomes odd, otherwise it is non monotonic recursion...
This gives PathNode is not compatible with PathNode. If I change TaintTracking2 to TaintTracking it is again non monotonic.
I use NameToNetHttpCookieTrackingConfiguration separately in isNetHttpCookieFlow to the path from sensitive name to sink in case no boolean is assigned.