diff --git a/rust/ql/lib/codeql/rust/dataflow/FlowBarrier.qll b/rust/ql/lib/codeql/rust/dataflow/FlowBarrier.qll new file mode 100644 index 000000000000..57211509b1a7 --- /dev/null +++ b/rust/ql/lib/codeql/rust/dataflow/FlowBarrier.qll @@ -0,0 +1,54 @@ +/** + * Provides classes and predicates for defining barriers and barrier guards. + * + * Flow sinks defined here feed into data flow configurations as follows: + * + * ```text + * data from *.model.yml | QL extensions of FlowSink::Range + * v v + * FlowSink (associated with a models-as-data kind string) + * v + * sinkNode predicate | other QL defined sinks, for example using concepts + * v v + * various Sink classes for specific data flow configurations <- extending QuerySink + * ``` + * + * New sinks should be defined using models-as-data, QL extensions of + * `FlowSink::Range`, or concepts. Data flow configurations should use the + * `sinkNode` predicate and/or concepts to define their sinks. + */ + +private import rust +private import internal.FlowSummaryImpl as Impl +private import internal.DataFlowImpl as DataFlowImpl + +// import all instances below +private module Barriers { + private import codeql.rust.Frameworks + private import codeql.rust.dataflow.internal.ModelsAsData +} + +/** Provides the `Range` class used to define the extent of `FlowBarrier`. */ +module FlowBarrier { + /** A flow sink. */ + abstract class Range extends Impl::Public::BarrierElement { + bindingset[this] + Range() { any() } + + override predicate isBarrier( + string output, string kind, Impl::Public::Provenance provenance, string model + ) { + this.isBarrier(output, kind) and provenance = "manual" and model = "" + } + + /** + * Holds is this element is a flow barrier of kind `kind`, where data + * flows out as described by `output`. + */ + predicate isBarrier(string output, string kind) { none() } + } +} + +final class FlowBarrier = FlowBarrier::Range; + +predicate barrierNode = DataFlowImpl::barrierNode/2; diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll index 23424dbffd92..f416c2a20e57 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll @@ -1157,6 +1157,25 @@ private module Cached { cached predicate sinkNode(Node n, string kind) { n.(FlowSummaryNode).isSink(kind, _) } + /** Holds if `n` is a flow barrier of kind `kind`. */ + cached + predicate barrierNode(Node n, string kind) { + exists( + FlowSummaryImpl::Public::BarrierElement b, + FlowSummaryImpl::Private::SummaryComponentStack stack + | + FlowSummaryImpl::Private::barrierSpec(b, stack, kind, _) + | + n = FlowSummaryImpl::StepsInput::getSourceNode(b, stack, false) + or + // For barriers like `Argument[0]` we want to target the pre-update node + n = + FlowSummaryImpl::StepsInput::getSourceNode(b, stack, true) + .(PostUpdateNode) + .getPreUpdateNode() + ) + } + /** * A step in a flow summary defined using `OptionalStep[name]`. An `OptionalStep` is "opt-in", which means * that by default the step is not present in the flow summary and needs to be explicitly enabled by defining diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/FlowSummaryImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/FlowSummaryImpl.qll index ab0b3aff9ca9..850328146518 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/FlowSummaryImpl.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/FlowSummaryImpl.qll @@ -143,7 +143,7 @@ module Input implements InputSig { private import Make as Impl -private module StepsInput implements Impl::Private::StepsInputSig { +module StepsInput implements Impl::Private::StepsInputSig { DataFlowCall getACall(Public::SummarizedCallable sc) { result.asCall().getStaticTarget() = sc } /** Gets the argument of `source` described by `sc`, if any. */ @@ -171,18 +171,27 @@ private module StepsInput implements Impl::Private::StepsInputSig { result.asCfgScope() = source.getEnclosingCfgScope() } - RustDataFlow::Node getSourceNode(Input::SourceBase source, Impl::Private::SummaryComponentStack s) { + additional RustDataFlow::Node getSourceNode( + Input::SourceBase source, Impl::Private::SummaryComponentStack s, boolean isArgPostUpdate + ) { s.head() = Impl::Private::SummaryComponent::return(_) and - result.asExpr() = source.getCall() + result.asExpr() = source.getCall() and + isArgPostUpdate = false or exists(RustDataFlow::ArgumentPosition pos, Expr arg | s.head() = Impl::Private::SummaryComponent::parameter(pos) and arg = getSourceNodeArgument(source, s.tail().headOfSingleton()) and - result.asParameter() = getCallable(arg).getParam(pos.getPosition()) + result.asParameter() = getCallable(arg).getParam(pos.getPosition()) and + isArgPostUpdate = false ) or result.(RustDataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = - getSourceNodeArgument(source, s.headOfSingleton()) + getSourceNodeArgument(source, s.headOfSingleton()) and + isArgPostUpdate = true + } + + RustDataFlow::Node getSourceNode(Input::SourceBase source, Impl::Private::SummaryComponentStack s) { + result = getSourceNode(source, s, _) } RustDataFlow::Node getSinkNode(Input::SinkBase sink, Impl::Private::SummaryComponent sc) { diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll b/rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll index 8b55de8d54dc..f8aae488b0e3 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll @@ -44,6 +44,7 @@ */ private import rust +private import codeql.rust.dataflow.FlowBarrier private import codeql.rust.dataflow.FlowSummary private import codeql.rust.dataflow.FlowSource private import codeql.rust.dataflow.FlowSink @@ -98,6 +99,29 @@ extensible predicate neutralModel( string path, string kind, string provenance, QlBuiltins::ExtensionId madId ); +/** + * Holds if in a call to the function with canonical path `path`, the value referred + * to by `output` is a barrier of the given `kind` and `madId` is the data + * extension row number. + */ +extensible predicate barrierModel( + string path, string output, string kind, string provenance, QlBuiltins::ExtensionId madId +); + +/** + * Holds if in a call to the function with canonical path `path`, the value referred + * to by `input` is a barrier guard of the given `kind` and `madId` is the data + * extension row number. + * the value referred to by `input` is assumed to lead to a parameter of a call + * (possibly `self`), and the call is guarding the parameter. + * `branch` is either `true` or `false`, indicating which branch of the guard + * is protecting the parameter. + */ +extensible predicate barrierGuardModel( + string path, string input, string branch, string kind, string provenance, + QlBuiltins::ExtensionId madId +); + /** * Holds if the given extension tuple `madId` should pretty-print as `model`. * @@ -123,6 +147,16 @@ predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) { neutralModel(path, kind, _, madId) and model = "Neutral: " + path + "; " + kind ) + or + exists(string path, string output, string kind | + barrierModel(path, output, kind, _, madId) and + model = "Barrier: " + path + "; " + output + "; " + kind + ) + or + exists(string path, string input, string branch, string kind | + barrierGuardModel(path, input, branch, kind, _, madId) and + model = "Barrier guard: " + path + "; " + input + "; " + branch + "; " + kind + ) } private class SummarizedCallableFromModel extends SummarizedCallable::Range { @@ -206,6 +240,23 @@ private class FlowSinkFromModel extends FlowSink::Range { } } +private class BarrierFromModel extends FlowBarrier::Range { + private string path; + + BarrierFromModel() { + barrierModel(path, _, _, _, _) and + this.callResolvesTo(path) + // path = this.getCall().getResolvedTarget().getCanonicalPath() + } + + override predicate isBarrier(string output, string kind, Provenance provenance, string model) { + exists(QlBuiltins::ExtensionId madId | + barrierModel(path, output, kind, provenance, madId) and + model = "MaD:" + madId.toString() + ) + } +} + private module Debug { private import FlowSummaryImpl private import Private diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/Node.qll b/rust/ql/lib/codeql/rust/dataflow/internal/Node.qll index 1fa3983f8112..1ccb7db73f52 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/Node.qll +++ b/rust/ql/lib/codeql/rust/dataflow/internal/Node.qll @@ -82,12 +82,12 @@ class FlowSummaryNode extends Node, TFlowSummaryNode { result = this.getSummaryNode().getSinkElement() } - /** Holds is this node is a source node of kind `kind`. */ + /** Holds if this node is a source node of kind `kind`. */ predicate isSource(string kind, string model) { this.getSummaryNode().(FlowSummaryImpl::Private::SourceOutputNode).isEntry(kind, model) } - /** Holds is this node is a sink node of kind `kind`. */ + /** Holds if this node is a sink node of kind `kind`. */ predicate isSink(string kind, string model) { this.getSummaryNode().(FlowSummaryImpl::Private::SinkInputNode).isExit(kind, model) } diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/empty.model.yml b/rust/ql/lib/codeql/rust/dataflow/internal/empty.model.yml index 1a33951dfc38..61c0327171a7 100644 --- a/rust/ql/lib/codeql/rust/dataflow/internal/empty.model.yml +++ b/rust/ql/lib/codeql/rust/dataflow/internal/empty.model.yml @@ -15,3 +15,13 @@ extensions: pack: codeql/rust-all extensible: summaryModel data: [] + + - addsTo: + pack: codeql/rust-all + extensible: barrierModel + data: [] + + - addsTo: + pack: codeql/rust-all + extensible: barrierGuardModel + data: [] diff --git a/rust/ql/test/library-tests/dataflow/barrier/inline-flow.ext.yml b/rust/ql/test/library-tests/dataflow/barrier/inline-flow.ext.yml new file mode 100644 index 000000000000..7daefdf2d5bb --- /dev/null +++ b/rust/ql/test/library-tests/dataflow/barrier/inline-flow.ext.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/rust-all + extensible: barrierModel + data: + - ["main::sanitize", "ReturnValue", "test", "manual"] diff --git a/rust/ql/test/library-tests/dataflow/barrier/inline-flow.ql b/rust/ql/test/library-tests/dataflow/barrier/inline-flow.ql index 5dcb7ee70a9d..fb70ad0f06f8 100644 --- a/rust/ql/test/library-tests/dataflow/barrier/inline-flow.ql +++ b/rust/ql/test/library-tests/dataflow/barrier/inline-flow.ql @@ -3,10 +3,21 @@ */ import rust +import codeql.rust.dataflow.DataFlow +import codeql.rust.dataflow.FlowBarrier import utils.test.InlineFlowTest -import DefaultFlowTest -import TaintFlow::PathGraph +import PathGraph -from TaintFlow::PathNode source, TaintFlow::PathNode sink -where TaintFlow::flowPath(source, sink) +module CustomConfig implements DataFlow::ConfigSig { + predicate isSource = DefaultFlowConfig::isSource/1; + + predicate isSink = DefaultFlowConfig::isSink/1; + + predicate isBarrier(DataFlow::Node n) { barrierNode(n, "test") } +} + +import FlowTest + +from PathNode source, PathNode sink +where flowPath(source, sink) select sink, source, sink, "$@", source, source.toString() diff --git a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll index d76672571921..fa20405f7882 100644 --- a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll @@ -368,6 +368,19 @@ module Make< abstract predicate isSink(string input, string kind, Provenance provenance, string model); } + /** A barrier element. */ + abstract class BarrierElement extends SourceBaseFinal { + bindingset[this] + BarrierElement() { any() } + + /** + * Holds if this element is a flow barrier of kind `kind`, where data + * flows out as described by `output`. + */ + pragma[nomagic] + abstract predicate isBarrier(string output, string kind, Provenance provenance, string model); + } + private signature predicate hasKindSig(string kind); signature class NeutralCallableSig extends SummarizedCallableBaseFinal { @@ -723,7 +736,19 @@ module Make< ) } - private predicate summarySpec(string spec) { + private predicate isRelevantBarrier( + BarrierElement e, string output, string kind, Provenance provenance, string model + ) { + e.isBarrier(output, kind, provenance, model) and + ( + provenance.isManual() + or + provenance.isGenerated() and + not exists(Provenance p | p.isManual() and e.isBarrier(_, kind, p, _)) + ) + } + + private predicate flowSpec(string spec) { exists(SummarizedCallable c | c.propagatesFlow(spec, _, _, _, _, _) or @@ -732,10 +757,12 @@ module Make< or isRelevantSource(_, spec, _, _, _) or + isRelevantBarrier(_, spec, _, _, _) + or isRelevantSink(_, spec, _, _, _) } - import AccessPathSyntax::AccessPath + import AccessPathSyntax::AccessPath /** Holds if specification component `token` parses as parameter `pos`. */ predicate parseParam(AccessPathToken token, ArgumentPosition pos) { @@ -1515,6 +1542,18 @@ module Make< ) } + /** + * Holds if `barrier` is a relevant barrier element with output specification `outSpec`. + */ + predicate barrierSpec( + BarrierElement barrier, SummaryComponentStack outSpec, string kind, string model + ) { + exists(string output | + isRelevantBarrier(barrier, output, kind, _, model) and + External::interpretSpec(output, outSpec) + ) + } + signature module TypesInputSig { /** Gets the type of content `c`. */ DataFlowType getContentType(ContentSet c);