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 deleted file mode 100644 index 70accc67d4c5..000000000000 --- a/cpp/ql/lib/change-notes/2024-02-06-flow-out-barrier-function.md +++ /dev/null @@ -1,4 +0,0 @@ ---- -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 7c2d92fee99b..bd63da2ab98e 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,11 +2,8 @@ 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 @@ -797,30 +794,10 @@ 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 - not modeledFlowBarrier(nFrom) and - nodeFrom != nodeTo + ssaFlowImpl(defOrUse, nFrom, nodeTo, uncertain) 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 cb757800d65e..446e659fac54 100644 --- a/cpp/ql/lib/semmle/code/cpp/models/implementations/Swap.qll +++ b/cpp/ql/lib/semmle/code/cpp/models/implementations/Swap.qll @@ -1,7 +1,6 @@ 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: @@ -9,7 +8,7 @@ import semmle.code.cpp.models.interfaces.FlowOutBarrier * std::swap(obj1, obj2) * ``` */ -private class Swap extends DataFlowFunction, FlowOutBarrierFunction { +private class Swap extends DataFlowFunction { Swap() { this.hasQualifiedName(["std", "bsl"], "swap") } override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { @@ -19,8 +18,6 @@ private class Swap extends DataFlowFunction, FlowOutBarrierFunction { input.isParameterDeref(1) and output.isParameterDeref(0) } - - override predicate isFlowOutBarrier(FunctionInput input) { input.isParameterDeref([0, 1]) } } /** @@ -29,9 +26,7 @@ private class Swap extends DataFlowFunction, FlowOutBarrierFunction { * obj1.swap(obj2) * ``` */ -private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction, - FlowOutBarrierFunction -{ +private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction { MemberSwap() { this.hasName("swap") and this.getNumberOfParameters() = 1 and @@ -52,8 +47,4 @@ 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 deleted file mode 100644 index d25e4381dccd..000000000000 --- a/cpp/ql/lib/semmle/code/cpp/models/interfaces/FlowOutBarrier.qll +++ /dev/null @@ -1,26 +0,0 @@ -/** - * 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 555f39779bf2..8eeb80a0f83e 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 - sink(j); // $ SPURIOUS: ast + sink(j.second); // $ SPURIOUS: ast,ir + sink(j); // $ SPURIOUS: ast,ir sink(k.first); - sink(k.second); // $ SPURIOUS: ast - sink(k); // $ SPURIOUS: ast + sink(k.second); // $ SPURIOUS: ast,ir + sink(k); // $ SPURIOUS: ast,ir 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 + sink(m15); // $ SPURIOUS: ast,ir sink(m16); // $ ast,ir sink(m17); // $ ast,ir - sink(m18); // $ SPURIOUS: ast + sink(m18); // $ SPURIOUS: ast,ir // 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 + sink(m15); // $ SPURIOUS: ast,ir sink(m16); // $ ast,ir sink(m17); // $ ast,ir - sink(m18); // $ SPURIOUS: ast + sink(m18); // $ SPURIOUS: ast,ir // 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 7c906fb72d21..c6c19d90089b 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 + sink(s12); // $ SPURIOUS: ast,ir sink(s13); // $ ast,ir sink(s14); // $ ast,ir - sink(s15); // $ SPURIOUS: ast + sink(s15); // $ SPURIOUS: ast,ir // 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 + sink(s12); // $ SPURIOUS: ast,ir sink(s13); // $ ast,ir sink(s14); // $ ast,ir - sink(s15); // $ SPURIOUS: ast + sink(s15); // $ SPURIOUS: ast,ir // 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 4f2c4afd6b0d..e2b99945724a 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 + sink(s2); // $ SPURIOUS: ast,ir sink(s3); // $ ast,ir - sink(s4); // $ SPURIOUS: ast + sink(s4); // $ SPURIOUS: ast,ir } 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 57310dfef6fd..a84b3606f922 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 + sink(ss2); // $ SPURIOUS: ast,ir sink(ss3); // $ ast,ir - sink(ss4); // $ SPURIOUS: ast + sink(ss4); // $ SPURIOUS: ast,ir } void test_stringstream_in() diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp index 1ca4957b529b..eeefa6dd427b 100644 --- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp +++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp @@ -212,7 +212,7 @@ void test_swap() { std::swap(x, y); - sink(x); // $ SPURIOUS: ast + sink(x); // $ SPURIOUS: ast,ir sink(y); // $ ast,ir } 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 14834e2a5e9d..a26ac8f05132 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 + sink(v1); // $ SPURIOUS: ast,ir sink(v2); // $ ast,ir sink(v3); // $ ast,ir - sink(v4); // $ SPURIOUS: ast + sink(v4); // $ SPURIOUS: ast,ir } void test_vector_clear() {