Generalization of FlowAfterFree

cpp/ql/lib/semmle/code/cpp/models/implementations/Deallocation.qll

@@ -27,10 +27,12 @@ private class StandardDeallocationFunction extends DeallocationFunction {
     or
     this.hasGlobalOrStdName([
         // --- Windows Memory Management for Windows Drivers
-        "ExFreePoolWithTag", "ExDeleteTimer", "IoFreeMdl", "IoFreeWorkItem", "IoFreeErrorLogEntry",
-        "MmFreeContiguousMemory", "MmFreeContiguousMemorySpecifyCache", "MmFreeNonCachedMemory",
-        "MmFreeMappingAddress", "MmFreePagesFromMdl", "MmUnmapReservedMapping",
-        "MmUnmapLockedPages",
+        "ExFreePool", "ExFreePoolWithTag", "ExDeleteTimer", "IoFreeIrp", "IoFreeMdl",
+        "IoFreeErrorLogEntry", "IoFreeWorkItem", "MmFreeContiguousMemory",
+        "MmFreeContiguousMemorySpecifyCache", "MmFreeNonCachedMemory", "MmFreeMappingAddress",
+        "MmFreePagesFromMdl", "MmUnmapReservedMapping", "MmUnmapLockedPages",
+        "NdisFreeGenericObject", "NdisFreeMemory", "NdisFreeMemoryWithTag", "NdisFreeMdl",
+        "NdisFreeNetBufferListPool", "NdisFreeNetBufferPool",
         // --- Windows Global / Local legacy allocation
         "LocalFree", "GlobalFree", "LocalReAlloc", "GlobalReAlloc",
         // --- Windows System Services allocation
@@ -47,6 +49,7 @@ private class StandardDeallocationFunction extends DeallocationFunction {
     this.hasGlobalOrStdName([
         // --- Windows Memory Management for Windows Drivers
         "ExFreeToLookasideListEx", "ExFreeToPagedLookasideList", "ExFreeToNPagedLookasideList",
+        "NdisFreeMemoryWithTagPriority", "StorPortFreeMdl", "StorPortFreePool",
         // --- NetBSD pool manager
         "pool_put", "pool_cache_put"
       ]) and

cpp/ql/src/Critical/FlowAfterFree.qll → cpp/ql/lib/semmle/code/cpp/security/flowafterfree/FlowAfterFree.qll

@@ -1,20 +1,10 @@
-import cpp
-import semmle.code.cpp.dataflow.new.DataFlow
-private import semmle.code.cpp.ir.IR
-
 /**
- * Signature for a predicate that holds if `n.asExpr() = e` and `n` is a sink in
- * the `FlowFromFreeConfig` module.
+ * General library for finding flow from a pointer being freed to a user-specified sink
  */
-private signature predicate isSinkSig(DataFlow::Node n, Expr e);
 
-/**
- * Holds if `dealloc` is a deallocation expression and `e` is an expression such
- * that `isFree(_, e)` holds for some `isFree` predicate satisfying `isSinkSig`,
- * and this source-sink pair should be excluded from the analysis.
- */
-bindingset[dealloc, e]
-private signature predicate isExcludedSig(DeallocationExpr dealloc, Expr e);
+import cpp
+import semmle.code.cpp.dataflow.new.DataFlow
+private import semmle.code.cpp.ir.IR
 
 /**
  * Holds if `(b1, i1)` strictly post-dominates `(b2, i2)`
@@ -38,6 +28,31 @@ predicate strictlyDominates(IRBlock b1, int i1, IRBlock b2, int i2) {
   b1.strictlyDominates(b2)
 }
 
+/**
+ * The signature for a module that is used to specify the inputs to the `FlowFromFree` module.
+ */
+signature module FlowFromFreeParamSig {
+  /**
+   * Holds if `n.asExpr() = e` and `n` is a sink in the `FlowFromFreeConfig`
+   * module.
+   */
+  predicate isSink(DataFlow::Node n, Expr e);
+
+  /**
+   * Holds if `dealloc` is a deallocation expression and `e` is an expression such
+   * that `isFree(_, e)` holds for some `isFree` predicate satisfying `isSinkSig`,
+   * and this source-sink pair should be excluded from the analysis.
+   */
+  bindingset[dealloc, e]
+  predicate isExcluded(DeallocationExpr dealloc, Expr e);
+
+  /**
+   * Holds if `sink` should be considered a `sink` when the source of flow is `source`.
+   */
+  bindingset[source, sink]
+  default predicate sourceSinkIsRelated(DataFlow::Node source, DataFlow::Node sink) { any() }
+}
+
 /**
  * Constructs a `FlowFromFreeConfig` module that can be used to find flow between
  * a pointer being freed by some deallocation function, and a user-specified sink.
@@ -47,8 +62,8 @@ predicate strictlyDominates(IRBlock b1, int i1, IRBlock b2, int i2) {
  * 1. The source dominates the sink, or
  * 2. The sink post-dominates the source.
  */
-module FlowFromFree<isSinkSig/2 isASink, isExcludedSig/2 isExcluded> {
-  module FlowFromFreeConfig implements DataFlow::StateConfigSig {
+module FlowFromFree<FlowFromFreeParamSig P> {
+  private module FlowFromFreeConfig implements DataFlow::StateConfigSig {
     class FlowState instanceof Expr {
       FlowState() { isFree(_, _, this, _) }
 
@@ -59,20 +74,12 @@ module FlowFromFree<isSinkSig/2 isASink, isExcludedSig/2 isExcluded> {
 
     pragma[inline]
     predicate isSink(DataFlow::Node sink, FlowState state) {
-      exists(
-        Expr e, DataFlow::Node source, IRBlock b1, int i1, IRBlock b2, int i2,
-        DeallocationExpr dealloc
-      |
-        isASink(sink, e) and
+      exists(Expr e, DataFlow::Node source, DeallocationExpr dealloc |
+        P::isSink(sink, e) and
         isFree(source, _, state, dealloc) and
         e != state and
-        source.hasIndexInBlock(b1, i1) and
-        sink.hasIndexInBlock(b2, i2) and
-        not isExcluded(dealloc, e)
-      |
-        strictlyDominates(b1, i1, b2, i2)
-        or
-        strictlyPostDominates(b2, i2, b1, i1)
+        not P::isExcluded(dealloc, e) and
+        P::sourceSinkIsRelated(source, sink)
       )
     }
 
@@ -127,3 +134,38 @@ predicate isExFreePoolCall(FunctionCall fc, Expr e) {
     fc.getTarget().hasGlobalName("ExFreePool")
   )
 }
+
+/**
+ * Holds if either `source` strictly dominates `sink`, or `sink` strictly
+ * post-dominates `source`.
+ */
+bindingset[source, sink]
+predicate defaultSourceSinkIsRelated(DataFlow::Node source, DataFlow::Node sink) {
+  exists(IRBlock b1, int i1, IRBlock b2, int i2 |
+    source.hasIndexInBlock(b1, i1) and
+    sink.hasIndexInBlock(b2, i2)
+  |
+    strictlyDominates(b1, i1, b2, i2)
+    or
+    strictlyPostDominates(b2, i2, b1, i1)
+  )
+}
+
+/**
+ * `dealloc1` is a deallocation expression, `e` is an expression that dereferences a
+ * pointer, and the `(dealloc1, e)` pair should be excluded by the `FlowFromFree` library.
+ *
+ * Note that `e` is not necessarily the expression deallocated by `dealloc1`. It will
+ * be bound to the second deallocation as identified by the `FlowFromFree` library.
+ *
+ * From https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-mmfreepagesfrommdl:
+ * "After calling MmFreePagesFromMdl, the caller must also call ExFreePool
+ * to release the memory that was allocated for the MDL structure."
+ */
+bindingset[dealloc1, e]
+predicate isExcludedMmFreePageFromMdl(DeallocationExpr dealloc1, Expr e) {
+  exists(DeallocationExpr dealloc2 | isFree(_, _, e, dealloc2) |
+    dealloc1.(FunctionCall).getTarget().hasGlobalName("MmFreePagesFromMdl") and
+    isExFreePoolCall(dealloc2, _)
+  )
+}

cpp/ql/lib/semmle/code/cpp/security/flowafterfree/UseAfterFree.qll

@@ -0,0 +1,159 @@
+/**
+ * General library for tracing Use After Free vulnerabilities.
+ */
+
+import cpp
+private import semmle.code.cpp.security.flowafterfree.FlowAfterFree
+private import semmle.code.cpp.ir.IR
+
+/**
+ * Holds if `call` is a call to a function that obviously
+ * doesn't dereference its `i`'th argument.
+ */
+private predicate externalCallNeverDereferences(FormattingFunctionCall call, int arg) {
+  exists(int formatArg |
+    pragma[only_bind_out](call.getFormatArgument(formatArg)) =
+      pragma[only_bind_out](call.getArgument(arg)) and
+    call.getFormat().(FormatLiteral).getConvSpec(formatArg) != "%s"
+  )
+}
+
+/**
+ * Holds if `e` is a use. A use is a pointer dereference or a
+ * parameter to a call with no function definition.
+ * Uses in deallocation expressions (e.g., free) are excluded.
+ * Default isUse definition for an expression.
+ */
+predicate isUse0(Expr e) {
+  not isFree(_, _, e, _) and
+  (
+    // TODO: use DirectDefereferencedByOperation in Dereferenced.qll
+    e = any(PointerDereferenceExpr pde).getOperand()
+    or
+    e = any(PointerFieldAccess pfa).getQualifier()
+    or
+    e = any(ArrayExpr ae).getArrayBase()
+    or
+    e = any(Call call).getQualifier()
+    or
+    // Assume any function without a body will dereference the pointer
+    exists(int i, Call call, Function f |
+      e = call.getArgument(i) and
+      f = call.getTarget() and
+      not f.hasEntryPoint() and
+      // Exclude known functions we know won't dereference the pointer.
+      // For example, a call such as `printf("%p", myPointer)`.
+      not externalCallNeverDereferences(call, i)
+    )
+  )
+}
+
+private module ParameterSinks {
+  import semmle.code.cpp.ir.ValueNumbering
+
+  private predicate flowsToUse(DataFlow::Node n) {
+    isUse0(n.asExpr())
+    or
+    exists(DataFlow::Node succ |
+      flowsToUse(succ) and
+      DataFlow::localFlowStep(n, succ)
+    )
+  }
+
+  private predicate flowsFromParam(DataFlow::Node n) {
+    flowsToUse(n) and
+    (
+      n.asParameter().getUnspecifiedType() instanceof PointerType
+      or
+      exists(DataFlow::Node prev |
+        flowsFromParam(prev) and
+        DataFlow::localFlowStep(prev, n)
+      )
+    )
+  }
+
+  private predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+    flowsFromParam(n1) and
+    flowsFromParam(n2) and
+    DataFlow::localFlowStep(n1, n2)
+  }
+
+  private predicate paramToUse(DataFlow::Node n1, DataFlow::Node n2) = fastTC(step/2)(n1, n2)
+
+  private predicate hasFlow(
+    DataFlow::Node source, InitializeParameterInstruction init, DataFlow::Node sink
+  ) {
+    pragma[only_bind_out](source.asParameter()) = pragma[only_bind_out](init.getParameter()) and
+    paramToUse(source, sink) and
+    isUse0(sink.asExpr())
+  }
+
+  private InitializeParameterInstruction getAnAlwaysDereferencedParameter0() {
+    exists(DataFlow::Node source, DataFlow::Node sink, IRBlock b1, int i1, IRBlock b2, int i2 |
+      hasFlow(pragma[only_bind_into](source), result, pragma[only_bind_into](sink)) and
+      source.hasIndexInBlock(b1, pragma[only_bind_into](i1)) and
+      sink.hasIndexInBlock(b2, pragma[only_bind_into](i2)) and
+      strictlyPostDominates(b2, i2, b1, i1)
+    )
+  }
+
+  private CallInstruction getAnAlwaysReachedCallInstruction() {
+    exists(IRFunction f | result.getBlock().postDominates(f.getEntryBlock()))
+  }
+
+  pragma[nomagic]
+  private predicate callHasTargetAndArgument(Function f, int i, Instruction argument) {
+    exists(CallInstruction call |
+      call.getStaticCallTarget() = f and
+      call.getArgument(i) = argument and
+      call = getAnAlwaysReachedCallInstruction()
+    )
+  }
+
+  pragma[nomagic]
+  private predicate initializeParameterInFunction(Function f, int i) {
+    exists(InitializeParameterInstruction init |
+      pragma[only_bind_out](init.getEnclosingFunction()) = f and
+      init.hasIndex(i) and
+      init = getAnAlwaysDereferencedParameter()
+    )
+  }
+
+  pragma[nomagic]
+  private predicate alwaysDereferencedArgumentHasValueNumber(ValueNumber vn) {
+    exists(int i, Function f, Instruction argument |
+      callHasTargetAndArgument(f, i, argument) and
+      initializeParameterInFunction(pragma[only_bind_into](f), pragma[only_bind_into](i)) and
+      vn.getAnInstruction() = argument
+    )
+  }
+
+  InitializeParameterInstruction getAnAlwaysDereferencedParameter() {
+    result = getAnAlwaysDereferencedParameter0()
+    or
+    exists(ValueNumber vn |
+      alwaysDereferencedArgumentHasValueNumber(vn) and
+      vn.getAnInstruction() = result
+    )
+  }
+}
+
+private import semmle.code.cpp.ir.dataflow.internal.DataFlowImplCommon
+
+/**
+ * Holds if `n` represents the expression `e`, and `e` is a pointer that is
+ * guaranteed to be dereferenced (either because it's an operand of a
+ * dereference operation, or because it's an argument to a function that
+ * always dereferences the parameter).
+ */
+predicate isUse(DataFlow::Node n, Expr e) {
+  isUse0(e) and n.asExpr() = e
+  or
+  exists(CallInstruction call, InitializeParameterInstruction init |
+    n.asOperand().getDef().getUnconvertedResultExpression() = e and
+    pragma[only_bind_into](init) = ParameterSinks::getAnAlwaysDereferencedParameter() and
+    viableParamArg(call, DataFlow::instructionNode(init), n) and
+    pragma[only_bind_out](init.getEnclosingFunction()) =
+      pragma[only_bind_out](call.getStaticCallTarget())
+  )
+}

cpp/ql/src/Critical/DoubleFree.ql

@@ -13,7 +13,7 @@
 
 import cpp
 import semmle.code.cpp.dataflow.new.DataFlow
-import FlowAfterFree
+import semmle.code.cpp.security.flowafterfree.FlowAfterFree
 import DoubleFree::PathGraph
 
 /**
@@ -22,26 +22,15 @@ import DoubleFree::PathGraph
  */
 predicate isFree(DataFlow::Node n, Expr e) { isFree(_, n, e, _) }
 
-/**
- * `dealloc1` is a deallocation expression and `e` is an expression such
- * that is deallocated by a deallocation expression, and the `(dealloc1, e)` pair
- * should be excluded by the `FlowFromFree` library.
- *
- * Note that `e` is not necessarily the expression deallocated by `dealloc1`. It will
- * be bound to the second deallocation as identified by the `FlowFromFree` library.
- */
-bindingset[dealloc1, e]
-predicate isExcludeFreePair(DeallocationExpr dealloc1, Expr e) {
-  exists(DeallocationExpr dealloc2 | isFree(_, _, e, dealloc2) |
-    dealloc1.(FunctionCall).getTarget().hasGlobalName("MmFreePagesFromMdl") and
-    // From https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-mmfreepagesfrommdl:
-    // "After calling MmFreePagesFromMdl, the caller must also call ExFreePool
-    // to release the memory that was allocated for the MDL structure."
-    isExFreePoolCall(dealloc2, _)
-  )
+module DoubleFreeParam implements FlowFromFreeParamSig {
+  predicate isSink = isFree/2;
+
+  predicate isExcluded = isExcludedMmFreePageFromMdl/2;
+
+  predicate sourceSinkIsRelated = defaultSourceSinkIsRelated/2;
 }
 
-module DoubleFree = FlowFromFree<isFree/2, isExcludeFreePair/2>;
+module DoubleFree = FlowFromFree<DoubleFreeParam>;
 
 from DoubleFree::PathNode source, DoubleFree::PathNode sink, DeallocationExpr dealloc, Expr e2
 where