From e823712adfcb50df1daaf79b8be3114342b6664b Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Wed, 21 Oct 2020 00:48:35 -0700 Subject: [PATCH] Add utility predicates to FunctionModel Co-authored-by: Chris Smowton --- .../Security/CWE-352/ConstantOauth2State.ql | 2 +- .../go/dataflow/internal/DataFlowUtil.qll | 27 ++++++++++++---- .../dataflow/internal/TaintTrackingUtil.qll | 32 ++++++++++++------- .../go/security/SafeUrlFlowCustomizations.qll | 2 +- .../FunctionModelStep.ql | 4 +-- 5 files changed, 46 insertions(+), 21 deletions(-) diff --git a/ql/src/Security/CWE-352/ConstantOauth2State.ql b/ql/src/Security/CWE-352/ConstantOauth2State.ql index f00cf749400..d28d6c8474b 100644 --- a/ql/src/Security/CWE-352/ConstantOauth2State.ql +++ b/ql/src/Security/CWE-352/ConstantOauth2State.ql @@ -101,7 +101,7 @@ class PrivateUrlFlowsToAuthCodeUrlCall extends DataFlow::Configuration { TaintTracking::referenceStep(pred, succ) or // Propagate across Sprintf and similar calls - TaintTracking::functionModelStep(any(Fmt::Sprinter s), pred, succ) + any(Fmt::Sprinter s).taintStep(pred, succ) } predicate isSink(DataFlow::Node sink, DataFlow::CallNode call) { diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll index 4b7f8e6231e..da941b3efa4 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll @@ -896,6 +896,26 @@ class TypeCastNode extends ExprNode { abstract class FunctionModel extends Function { /** Holds if data flows through this function from `input` to `output`. */ abstract predicate hasDataFlow(FunctionInput input, FunctionOutput output); + + /** Gets an input node for this model for the call `c`. */ + DataFlow::Node getAnInputNode(DataFlow::CallNode c) { this.flowStepForCall(result, _, c) } + + /** Gets an output node for this model for the call `c`. */ + DataFlow::Node getAnOutputNode(DataFlow::CallNode c) { this.flowStepForCall(_, result, c) } + + /** Holds if this function model causes data to flow from `pred` to `succ` for the call `c`. */ + predicate flowStepForCall(DataFlow::Node pred, DataFlow::Node succ, DataFlow::CallNode c) { + c = this.getACall() and + exists(FunctionInput inp, FunctionOutput outp | this.hasDataFlow(inp, outp) | + pred = inp.getNode(c) and + succ = outp.getNode(c) + ) + } + + /** Holds if this function model causes data to flow from `pred` to `succ`. */ + predicate flowStep(DataFlow::Node pred, DataFlow::Node succ) { + this.flowStepForCall(pred, succ, _) + } } /** @@ -1006,12 +1026,7 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) { basicLocalFlowStep(nodeFrom, nodeTo) or // step through function model - exists(FunctionModel m, CallNode c, FunctionInput inp, FunctionOutput outp | - c = m.getACall() and - m.hasDataFlow(inp, outp) and - nodeFrom = inp.getNode(c) and - nodeTo = outp.getNode(c) - ) + any(FunctionModel m).flowStep(nodeFrom, nodeTo) } /** diff --git a/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll b/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll index f16b0cc4af5..b05a81bf0c2 100644 --- a/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll +++ b/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll @@ -60,7 +60,7 @@ predicate localAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { tupleStep(pred, succ) or stringConcatStep(pred, succ) or sliceStep(pred, succ) or - functionModelStep(_, pred, succ) or + any(FunctionModel fm).taintStep(pred, succ) or any(AdditionalTaintStep a).step(pred, succ) } @@ -140,16 +140,6 @@ predicate sliceStep(DataFlow::Node pred, DataFlow::Node succ) { succ.(DataFlow::SliceNode).getBase() = pred } -/** Holds if taint flows from `pred` to `succ` via a function model. */ -predicate functionModelStep(FunctionModel fn, DataFlow::Node pred, DataFlow::Node succ) { - exists(DataFlow::CallNode c, FunctionInput inp, FunctionOutput outp | - c = fn.getACall() and - fn.hasTaintFlow(inp, outp) and - pred = inp.getNode(c) and - succ = outp.getNode(c) - ) -} - /** * A model of a function specifying that the function propagates taint from * a parameter or qualifier to a result. @@ -157,6 +147,26 @@ predicate functionModelStep(FunctionModel fn, DataFlow::Node pred, DataFlow::Nod abstract class FunctionModel extends Function { /** Holds if taint propagates through this function from `input` to `output`. */ abstract predicate hasTaintFlow(FunctionInput input, FunctionOutput output); + + /** Gets an input node for this model for the call `c`. */ + DataFlow::Node getAnInputNode(DataFlow::CallNode c) { this.taintStepForCall(result, _, c) } + + /** Gets an output node for this model for the call `c`. */ + DataFlow::Node getAnOutputNode(DataFlow::CallNode c) { this.taintStepForCall(_, result, c) } + + /** Holds if this function model causes taint to flow from `pred` to `succ` for the call `c`. */ + predicate taintStepForCall(DataFlow::Node pred, DataFlow::Node succ, DataFlow::CallNode c) { + c = this.getACall() and + exists(FunctionInput inp, FunctionOutput outp | this.hasTaintFlow(inp, outp) | + pred = inp.getNode(c) and + succ = outp.getNode(c) + ) + } + + /** Holds if this function model causes taint to flow from `pred` to `succ`. */ + predicate taintStep(DataFlow::Node pred, DataFlow::Node succ) { + this.taintStepForCall(pred, succ, _) + } } /** diff --git a/ql/src/semmle/go/security/SafeUrlFlowCustomizations.qll b/ql/src/semmle/go/security/SafeUrlFlowCustomizations.qll index c73aae7f623..6b53df3e6d2 100644 --- a/ql/src/semmle/go/security/SafeUrlFlowCustomizations.qll +++ b/ql/src/semmle/go/security/SafeUrlFlowCustomizations.qll @@ -25,6 +25,6 @@ module SafeUrlFlow { /** A function model step using `UnsafeUrlMethod`, considered as a sanitizer for safe URL flow. */ private class UnsafeUrlMethodEdge extends SanitizerEdge { - UnsafeUrlMethodEdge() { TaintTracking::functionModelStep(any(UnsafeUrlMethod um), this, _) } + UnsafeUrlMethodEdge() { this = any(UnsafeUrlMethod um).getAnInputNode(_) } } } diff --git a/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionModelStep.ql b/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionModelStep.ql index 276239350d7..80bfb92cbfa 100644 --- a/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionModelStep.ql +++ b/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionModelStep.ql @@ -16,6 +16,6 @@ class ReaderReset extends TaintTracking::FunctionModel, Method { } } -from Function fn, DataFlow::Node pred, DataFlow::Node succ -where TaintTracking::functionModelStep(fn, pred, succ) +from TaintTracking::FunctionModel fn, DataFlow::Node pred, DataFlow::Node succ +where fn.taintStep(pred, succ) select fn, pred, succ