Shared: Add DataFlow::DeduplicatePathGraph

[asgerf]

Adds a shared parameterised module, DataFlow::DeduplicatePathGraph, for post-processing a PathGraph so that it doesn't result in duplicate alerts or alerts with multiple identical paths.

This issue usually arises from using FlowState, which is embedded in the PathNode but not rendered as part of its string value. This can thus result in paths that have different intermediate flow states but appear to be identical to the end-user.

The issue with multiple alerts, i.e. seemingly-identical rows in the #select clause, is particularly bad for tools that attempt to diff results (such as DCA) but does not perform its own deduplication in advance.

The module works by projecting PathNode down to their (node, toString) values, which is closer to what the end-user actually sees in the end. By seeing the path graph as an NFA that accepts input symbols of type (node, toString) we try to minimise this NFA by merging states.

This is needed by the JavaScript data-flow migration, but I've put this in its own PR so it can be reviewed separately. I've used the library in a Ruby query that had some very ad-hoc alert deduplication logic. Note that the expected output diff is mainly due to reordering of result sets in the output.

[hvitved]

I wonder if this should be the default graph exposed by PathGraph, and then have the original graph exposed as, say, RawPathGraph?

[asgerf]

I've found a bug in the handling of subpaths that I'd like to fix, so taking this back into draft state for now.

[aschackmull]

I think you need to amend this case slightly if you want the call bit to have meaning:

Suggested change

	Graph::edges(prev, node, _, _)
	Graph::edges(prev, node, _, _) and
	not Graph::subpaths(prev, node, _, _)

[aschackmull]

This line ought to be superfluous, but it can't hurt.

[aschackmull]

I think you meant to exclude subpath enter- and exit-steps here, right?

Suggested change

	step(node, result, _, _)
	step(node, result, _, _) and
	not result = enterSubpathStep(node) and
	not result = exitSubpathStep(node)

[aschackmull]

This should already be present in step, so it should be superfluous here, but it cannot hurt, so I don't mind keeping it. But maybe add a comment about this.

[aschackmull]

How about not re-exporting the dedup'ed pathgraph, and instead export the boilerplate-translated flowPath predicate? Then we'd write

Suggested change

	import DataFlow::DeduplicatePathGraph<CodeInjectionFlow::PathNode, CodeInjectionFlow::PathGraph>
	module CodeInjectionFlowDeDup = DataFlow::DeduplicatePathGraph<CodeInjectionFlow::PathNode, CodeInjectionFlow::PathGraph>;
	import CodeInjectionFlowDeDup::PathGraph

[aschackmull]

.. and

Suggested change

	from PathNode source, PathNode sink
	where CodeInjectionFlow::flowPath(source.getAnOriginalPathNode(), sink.getAnOriginalPathNode())
	from CodeInjectionFlowDeDup::PathNode source, CodeInjectionFlowDeDup::PathNode sink
	where CodeInjectionFlowDeDup::flowPath(source, sink)

[aschackmull]

There's still the open question about whether this algorithm can introduce FP path explanations, i.e. whether all collapsed paths can be translated back to a path in the original graph. We tried to construct a proof, but it's tricky to properly account for subpaths. But that probably shouldn't hold the PR back - it seems plausibly good enough, and we're still referring to the original flowPath predicate, so we cannot introduce any actual new FP results, so it's reasonably safe.

[aschackmull]

LGTM

[asgerf]

Doing another DCA run just to be safe

[asgerf]

DCA run looks fine