codeql
codeql copied to clipboard
ATM: undo unsound performance optimizations
For an ordinary path-problem query, it is a requirement that at least one sink exists, otherwise there is nothing to alert on.
Thus the optimization with checking isSink(_, this, _)
pruning in Configuration::hasFlow
is sound.
For programs without any relevant sinks (which is likely to be the case when ATM is used), the Configuration::hasFlow
calls will not hold due to the above pruning step.
The optimizations removed in this commit are thus unsound for programs without any relevant sinks.
Relevant pings: @henrymercer @adityasharad
Hopefully performance works out without this 🤞🏻🤞🏻.
Impact
This means that it now is possible to predict sinks in programs without any known sinks(!!!).
Testing
Testing this is presumably tricky at the moment due to the required presence of a mlmodel. So I include my own manual test case for completeness and posterity.
Example database sources
// danger_sink_call.js
danger(sink)
// flow_to_predicted_sink.ts
// carefully selected file content that presently predicts `req.params.id` to be a sink
import { Request } from "express";
module.exports = function retrieveBasket() {
return (req: Request) => {
models.Basket.findOne({
where: { id: req.params.id }
});
};
};
Example query that produces no/some results without/with this PR.
import DataFlow::PathGraph
import experimental.adaptivethreatmodeling.TaintedPathATM
import experimental.adaptivethreatmodeling.EndpointFeatures
import experimental.adaptivethreatmodeling.EndpointScoring
from DataFlow::Node scored, EndpointType type, string basename, float scoreNum, string description
where
description = type.getDescription() and
ModelScoring::endpointScores(scored, type.getEncoding(), scoreNum) and
scored.getFile().getBaseName() = basename and
basename = ["danger_sink_call.js", "flow_to_predicted_sink.ts"]
select scored, basename, description, scoreNum
Confirming that hasFlow(...)
works for the full ATM query without any known sinks
Run TaintedPathATM.ql
.