Merge pull request #15528 from MathiasVP/flow-barrier-interface

C++: Add an interface for models to block flow

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.

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
   )

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)
+  }
 }

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);
+}

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<char *, char *> 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<char *, char *> m19, m20, m21, m22;

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<char *> 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<char *> s16, s17, s18, s19;

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() {

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()

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() {