C++: Barrier guards API for indirect expressions

cpp/ql/lib/change-notes/2023-04-28-indirect-barrier-node.md

@@ -0,0 +1,4 @@
+---
+category: feature
+---
+* A new predicate `BarrierGuard::getAnIndirectBarrierNode` has been added to the new dataflow library (`semmle.code.cpp.dataflow.new.DataFlow`) to mark indirect expressions as barrier nodes using the `BarrierGuard` API.

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

@@ -1903,7 +1903,38 @@ signature predicate guardChecksSig(IRGuardCondition g, Expr e, boolean branch);
  * in data flow and taint tracking.
  */
 module BarrierGuard<guardChecksSig/3 guardChecks> {
-  /** Gets a node that is safely guarded by the given guard check. */
+  /**
+   * Gets an expression node that is safely guarded by the given guard check.
+   *
+   * For example, given the following code:
+   * ```cpp
+   * int x = source();
+   * // ...
+   * if(is_safe_int(x)) {
+   *   sink(x);
+   * }
+   * ```
+   * and the following barrier guard predicate:
+   * ```ql
+   * predicate myGuardChecks(IRGuardCondition g, Expr e, boolean branch) {
+   *   exists(Call call |
+   *     g.getUnconvertedResultExpression() = call and
+   *     call.getTarget().hasName("is_safe_int") and
+   *     e = call.getAnArgument() and
+   *     branch = true
+   *   )
+   * }
+   * ```
+   * implementing `isBarrier` as:
+   * ```ql
+   * predicate isBarrier(DataFlow::Node barrier) {
+   *   barrier = DataFlow::BarrierGuard<myGuardChecks/3>::getABarrierNode()
+   * }
+   * ```
+   * will block flow from `x = source()` to `sink(x)`.
+   *
+   * NOTE: If an indirect expression is tracked, use `getAnIndirectBarrierNode` instead.
+   */
   ExprNode getABarrierNode() {
     exists(IRGuardCondition g, Expr e, ValueNumber value, boolean edge |
       e = value.getAnInstruction().getConvertedResultExpression() and
@@ -1912,6 +1943,84 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
       g.controls(result.getBasicBlock(), edge)
     )
   }
+
+  /**
+   * Gets an indirect expression node that is safely guarded by the given guard check.
+   *
+   * For example, given the following code:
+   * ```cpp
+   * int* p;
+   * // ...
+   * *p = source();
+   * if(is_safe_pointer(p)) {
+   *   sink(*p);
+   * }
+   * ```
+   * and the following barrier guard check:
+   * ```ql
+   * predicate myGuardChecks(IRGuardCondition g, Expr e, boolean branch) {
+   *   exists(Call call |
+   *     g.getUnconvertedResultExpression() = call and
+   *     call.getTarget().hasName("is_safe_pointer") and
+   *     e = call.getAnArgument() and
+   *     branch = true
+   *   )
+   * }
+   * ```
+   * implementing `isBarrier` as:
+   * ```ql
+   * predicate isBarrier(DataFlow::Node barrier) {
+   *   barrier = DataFlow::BarrierGuard<myGuardChecks/3>::getAnIndirectBarrierNode()
+   * }
+   * ```
+   * will block flow from `x = source()` to `sink(x)`.
+   *
+   * NOTE: If a non-indirect expression is tracked, use `getABarrierNode` instead.
+   */
+  IndirectExprNode getAnIndirectBarrierNode() { result = getAnIndirectBarrierNode(_) }
+
+  /**
+   * Gets an indirect expression node with indirection index `indirectionIndex` that is
+   * safely guarded by the given guard check.
+   *
+   * For example, given the following code:
+   * ```cpp
+   * int* p;
+   * // ...
+   * *p = source();
+   * if(is_safe_pointer(p)) {
+   *   sink(*p);
+   * }
+   * ```
+   * and the following barrier guard check:
+   * ```ql
+   * predicate myGuardChecks(IRGuardCondition g, Expr e, boolean branch) {
+   *   exists(Call call |
+   *     g.getUnconvertedResultExpression() = call and
+   *     call.getTarget().hasName("is_safe_pointer") and
+   *     e = call.getAnArgument() and
+   *     branch = true
+   *   )
+   * }
+   * ```
+   * implementing `isBarrier` as:
+   * ```ql
+   * predicate isBarrier(DataFlow::Node barrier) {
+   *   barrier = DataFlow::BarrierGuard<myGuardChecks/3>::getAnIndirectBarrierNode(1)
+   * }
+   * ```
+   * will block flow from `x = source()` to `sink(x)`.
+   *
+   * NOTE: If a non-indirect expression is tracked, use `getABarrierNode` instead.
+   */
+  IndirectExprNode getAnIndirectBarrierNode(int indirectionIndex) {
+    exists(IRGuardCondition g, Expr e, ValueNumber value, boolean edge |
+      e = value.getAnInstruction().getConvertedResultExpression() and
+      result.getConvertedExpr(indirectionIndex) = e and
+      guardChecks(g, value.getAnInstruction().getConvertedResultExpression(), edge) and
+      g.controls(result.getBasicBlock(), edge)
+    )
+  }
 }
 
 /**

cpp/ql/test/library-tests/dataflow/dataflow-tests/BarrierGuard.cpp

@@ -1,5 +1,5 @@
 int source();
-void sink(int);
+void sink(...);
 bool guarded(int);
 
 void bg_basic(int source) {
@@ -66,3 +66,13 @@ void bg_structptr(XY *p1, XY *p2) { // $ ast-def=p1 ast-def=p2
     sink(p1->x); // $ ast,ir
   }
 }
+
+int* indirect_source();
+bool guarded(const int*);
+
+void bg_indirect_expr() {
+  int *buf = indirect_source();
+  if (guarded(buf)) {
+    sink(buf);
+  }
+}

cpp/ql/test/library-tests/dataflow/dataflow-tests/test.ql

@@ -47,6 +47,7 @@ module AstTest {
 }
 
 module IRTest {
+  private import cpp
   private import semmle.code.cpp.ir.dataflow.DataFlow
   private import semmle.code.cpp.ir.IR
   private import semmle.code.cpp.controlflow.IRGuards
@@ -56,10 +57,13 @@ module IRTest {
    * S in `if (guarded(x)) S`.
    */
   // This is tested in `BarrierGuard.cpp`.
-  predicate testBarrierGuard(IRGuardCondition g, Instruction checked, boolean isTrue) {
-    g.(CallInstruction).getStaticCallTarget().getName() = "guarded" and
-    checked = g.(CallInstruction).getPositionalArgument(0) and
-    isTrue = true
+  predicate testBarrierGuard(IRGuardCondition g, Expr checked, boolean isTrue) {
+    exists(Call call |
+      call = g.getUnconvertedResultExpression() and
+      call.getTarget().hasName("guarded") and
+      checked = call.getArgument(0) and
+      isTrue = true
+    )
   }
 
   /** Common data flow configuration to be used by tests. */
@@ -90,7 +94,9 @@ module IRTest {
         barrierExpr.(VariableAccess).getTarget().hasName("barrier")
       )
       or
-      barrier = DataFlow::InstructionBarrierGuard<testBarrierGuard/3>::getABarrierNode()
+      barrier = DataFlow::BarrierGuard<testBarrierGuard/3>::getABarrierNode()
+      or
+      barrier = DataFlow::BarrierGuard<testBarrierGuard/3>::getAnIndirectBarrierNode()
     }
   }
 }