diff --git a/cpp/ql/lib/change-notes/2024-02-06-flow-out-barrier-function.md b/cpp/ql/lib/change-notes/2024-02-06-flow-out-barrier-function.md new file mode 100644 index 000000000000..70accc67d4c5 --- /dev/null +++ b/cpp/ql/lib/change-notes/2024-02-06-flow-out-barrier-function.md @@ -0,0 +1,4 @@ +--- +category: feature +--- +* Added an abstract class `FlowOutBarrierFunction` that can be used to block flow out of a function. \ No newline at end of file diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll index 996385f2bfac..3e7cfbe9e114 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll @@ -2,8 +2,11 @@ private import codeql.ssa.Ssa as SsaImplCommon private import semmle.code.cpp.ir.IR private import DataFlowUtil private import DataFlowImplCommon as DataFlowImplCommon +private import semmle.code.cpp.ir.dataflow.internal.ModelUtil private import semmle.code.cpp.models.interfaces.Allocation as Alloc private import semmle.code.cpp.models.interfaces.DataFlow as DataFlow +private import semmle.code.cpp.models.interfaces.FlowOutBarrier as FOB +private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs as FIO private import semmle.code.cpp.ir.internal.IRCppLanguage private import DataFlowPrivate private import ssa0.SsaInternals as SsaInternals0 @@ -784,10 +787,30 @@ private Node getAPriorDefinition(SsaDefOrUse defOrUse) { ) } +/** + * Holds if there should not be use-use flow out of `n` (or a conversion that + * flows to `n`). + */ +private predicate modeledFlowBarrier(Node n) { + exists(FIO::FunctionInput input, CallInstruction call | + call.getStaticCallTarget().(FOB::FlowOutBarrierFunction).isFlowOutBarrier(input) and + n = callInput(call, input) + ) + or + exists(Operand operand, Instruction instr, Node n0, int indirectionIndex | + modeledFlowBarrier(n0) and + nodeHasInstruction(n0, instr, indirectionIndex) and + conversionFlow(operand, instr, false, _) and + nodeHasOperand(n, operand, indirectionIndex) + ) +} + /** Holds if there is def-use or use-use flow from `nodeFrom` to `nodeTo`. */ predicate ssaFlow(Node nodeFrom, Node nodeTo) { exists(Node nFrom, boolean uncertain, SsaDefOrUse defOrUse | - ssaFlowImpl(defOrUse, nFrom, nodeTo, uncertain) and nodeFrom != nodeTo + ssaFlowImpl(defOrUse, nFrom, nodeTo, uncertain) and + not modeledFlowBarrier(nFrom) and + nodeFrom != nodeTo | if uncertain = true then nodeFrom = [nFrom, getAPriorDefinition(defOrUse)] else nodeFrom = nFrom ) diff --git a/cpp/ql/lib/semmle/code/cpp/models/implementations/Swap.qll b/cpp/ql/lib/semmle/code/cpp/models/implementations/Swap.qll index 446e659fac54..325fd6f58b2a 100644 --- a/cpp/ql/lib/semmle/code/cpp/models/implementations/Swap.qll +++ b/cpp/ql/lib/semmle/code/cpp/models/implementations/Swap.qll @@ -1,6 +1,7 @@ import semmle.code.cpp.models.interfaces.DataFlow import semmle.code.cpp.models.interfaces.Taint import semmle.code.cpp.models.interfaces.Alias +import semmle.code.cpp.models.interfaces.FlowOutBarrier /** * The standard function `swap`. A use of `swap` looks like this: @@ -8,7 +9,7 @@ import semmle.code.cpp.models.interfaces.Alias * std::swap(obj1, obj2) * ``` */ -private class Swap extends DataFlowFunction { +private class Swap extends DataFlowFunction, FlowOutBarrierFunction { Swap() { this.hasQualifiedName(["std", "bsl"], "swap") } override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { @@ -18,6 +19,8 @@ private class Swap extends DataFlowFunction { input.isParameterDeref(1) and output.isParameterDeref(0) } + + override predicate isFlowOutBarrier(FunctionInput input) { input.isParameterDeref(1) } } /** @@ -26,7 +29,9 @@ private class Swap extends DataFlowFunction { * obj1.swap(obj2) * ``` */ -private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction { +private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction, + FlowOutBarrierFunction +{ MemberSwap() { this.hasName("swap") and this.getNumberOfParameters() = 1 and @@ -47,4 +52,8 @@ private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction { override predicate parameterEscapesOnlyViaReturn(int index) { index = 0 } override predicate parameterIsAlwaysReturned(int index) { index = 0 } + + override predicate isFlowOutBarrier(FunctionInput input) { + input.isQualifierObject() or input.isParameterDeref(0) + } } diff --git a/cpp/ql/lib/semmle/code/cpp/models/interfaces/FlowOutBarrier.qll b/cpp/ql/lib/semmle/code/cpp/models/interfaces/FlowOutBarrier.qll new file mode 100644 index 000000000000..d25e4381dccd --- /dev/null +++ b/cpp/ql/lib/semmle/code/cpp/models/interfaces/FlowOutBarrier.qll @@ -0,0 +1,26 @@ +/** + * Provides an abstract class for blocking flow out of functions. To use this + * QL library, create a QL class extending `FlowOutBarrierFunction` with a + * characteristic predicate that selects the function or set of functions you + * are modeling. Within that class, override the predicates provided by + * `FlowOutBarrierFunction` to match the flow within that function. + */ + +import semmle.code.cpp.Function +import FunctionInputsAndOutputs + +/** + * A library function for which flow should not continue after reaching one + * of its inputs. + * + * For example, since `std::swap(a, b)` swaps the values pointed to by `a` + * and `b` there should not be use-use flow out of `a` or `b`. + */ +abstract class FlowOutBarrierFunction extends Function { + /** + * Holds if use-use flow should not continue onwards after reaching + * the argument, qualifier, or buffer represented by `input`. + */ + pragma[nomagic] + abstract predicate isFlowOutBarrier(FunctionInput input); +} diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/map.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/map.cpp index 8eeb80a0f83e..555f39779bf2 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/map.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/map.cpp @@ -71,11 +71,11 @@ void test_pair() sink(i.second); // $ MISSING: ast,ir sink(i); // $ ast,ir sink(j.first); - sink(j.second); // $ SPURIOUS: ast,ir - sink(j); // $ SPURIOUS: ast,ir + sink(j.second); // $ SPURIOUS: ast + sink(j); // $ SPURIOUS: ast sink(k.first); - sink(k.second); // $ SPURIOUS: ast,ir - sink(k); // $ SPURIOUS: ast,ir + sink(k.second); // $ SPURIOUS: ast + sink(k); // $ SPURIOUS: ast sink(l.first); sink(l.second); // $ MISSING: ast,ir sink(l); // $ ast,ir @@ -196,10 +196,10 @@ void test_map() sink(m18); // $ ast,ir m15.swap(m16); m17.swap(m18); - sink(m15); // $ SPURIOUS: ast,ir + sink(m15); // $ SPURIOUS: ast sink(m16); // $ ast,ir sink(m17); // $ ast,ir - sink(m18); // $ SPURIOUS: ast,ir + sink(m18); // $ SPURIOUS: ast // merge std::map m19, m20, m21, m22; @@ -345,10 +345,10 @@ void test_unordered_map() sink(m18); // $ ast,ir m15.swap(m16); m17.swap(m18); - sink(m15); // $ SPURIOUS: ast,ir + sink(m15); // $ SPURIOUS: ast sink(m16); // $ ast,ir sink(m17); // $ ast,ir - sink(m18); // $ SPURIOUS: ast,ir + sink(m18); // $ SPURIOUS: ast // merge std::unordered_map m19, m20, m21, m22; diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/set.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/set.cpp index c6c19d90089b..7c906fb72d21 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/set.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/set.cpp @@ -81,10 +81,10 @@ void test_set() sink(s15); // $ ast,ir s12.swap(s13); s14.swap(s15); - sink(s12); // $ SPURIOUS: ast,ir + sink(s12); // $ SPURIOUS: ast sink(s13); // $ ast,ir sink(s14); // $ ast,ir - sink(s15); // $ SPURIOUS: ast,ir + sink(s15); // $ SPURIOUS: ast // merge std::set s16, s17, s18, s19; @@ -193,10 +193,10 @@ void test_unordered_set() sink(s15); // $ ast,ir s12.swap(s13); s14.swap(s15); - sink(s12); // $ SPURIOUS: ast,ir + sink(s12); // $ SPURIOUS: ast sink(s13); // $ ast,ir sink(s14); // $ ast,ir - sink(s15); // $ SPURIOUS: ast,ir + sink(s15); // $ SPURIOUS: ast // merge std::unordered_set s16, s17, s18, s19; diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/string.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/string.cpp index e2b99945724a..4f2c4afd6b0d 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/string.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/string.cpp @@ -280,9 +280,9 @@ void test_string_swap() { s4.swap(s3); sink(s1); // $ ast,ir - sink(s2); // $ SPURIOUS: ast,ir + sink(s2); // $ SPURIOUS: ast sink(s3); // $ ast,ir - sink(s4); // $ SPURIOUS: ast,ir + sink(s4); // $ SPURIOUS: ast } void test_string_clear() { diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp index a84b3606f922..57310dfef6fd 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/stringstream.cpp @@ -118,9 +118,9 @@ void test_stringstream_swap() ss4.swap(ss3); sink(ss1); // $ ast,ir - sink(ss2); // $ SPURIOUS: ast,ir + sink(ss2); // $ SPURIOUS: ast sink(ss3); // $ ast,ir - sink(ss4); // $ SPURIOUS: ast,ir + sink(ss4); // $ SPURIOUS: ast } void test_stringstream_in() diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/vector.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/vector.cpp index a26ac8f05132..14834e2a5e9d 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/vector.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/vector.cpp @@ -114,10 +114,10 @@ void test_vector_swap() { v1.swap(v2); v3.swap(v4); - sink(v1); // $ SPURIOUS: ast,ir + sink(v1); // $ SPURIOUS: ast sink(v2); // $ ast,ir sink(v3); // $ ast,ir - sink(v4); // $ SPURIOUS: ast,ir + sink(v4); // $ SPURIOUS: ast } void test_vector_clear() {