Merge branch 'main' into python/test-container-steps

Author: yoff

config/identical-files.json

@@ -47,7 +47,6 @@
     "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl2.qll",
     "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl3.qll",
     "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll",
-    "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplForRegExp.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl1.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl2.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplForHttpClientLibraries.qll",

cpp/ql/lib/qlpack.yml

@@ -9,3 +9,4 @@ dependencies:
   codeql/ssa: ${workspace}
   codeql/tutorial: ${workspace}
   codeql/util: ${workspace}
+warnOnImplicitThis: true

cpp/ql/lib/semmle/code/cpp/controlflow/StackVariableReachability.qll

@@ -25,7 +25,7 @@ import cpp
  */
 abstract class StackVariableReachability extends string {
   bindingset[this]
-  StackVariableReachability() { length() >= 0 }
+  StackVariableReachability() { this.length() >= 0 }
 
   /** Holds if `node` is a source for the reachability analysis using variable `v`. */
   abstract predicate isSource(ControlFlowNode node, StackVariable v);
@@ -227,7 +227,7 @@ predicate bbSuccessorEntryReachesLoopInvariant(
  */
 abstract class StackVariableReachabilityWithReassignment extends StackVariableReachability {
   bindingset[this]
-  StackVariableReachabilityWithReassignment() { length() >= 0 }
+  StackVariableReachabilityWithReassignment() { this.length() >= 0 }
 
   /** Override this predicate rather than `isSource` (`isSource` is used internally). */
   abstract predicate isSourceActual(ControlFlowNode node, StackVariable v);
@@ -330,7 +330,7 @@ abstract class StackVariableReachabilityWithReassignment extends StackVariableRe
  */
 abstract class StackVariableReachabilityExt extends string {
   bindingset[this]
-  StackVariableReachabilityExt() { length() >= 0 }
+  StackVariableReachabilityExt() { this.length() >= 0 }
 
   /** `node` is a source for the reachability analysis using variable `v`. */
   abstract predicate isSource(ControlFlowNode node, StackVariable v);

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

@@ -562,6 +562,14 @@ class SsaPhiNode extends Node, TSsaPhiNode {
 
   /** Gets the source variable underlying this phi node. */
   Ssa::SourceVariable getSourceVariable() { result = phi.getSourceVariable() }
+
+  /**
+   * Holds if this phi node is a phi-read node.
+   *
+   * Phi-read nodes are like normal phi nodes, but they are inserted based
+   * on reads instead of writes.
+   */
+  predicate isPhiRead() { phi.isPhiRead() }
 }
 
 /**

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

@@ -1012,6 +1012,14 @@ class PhiNode extends SsaImpl::DefinitionExt {
     this instanceof SsaImpl::PhiNode or
     this instanceof SsaImpl::PhiReadNode
   }
+
+  /**
+   * Holds if this phi node is a phi-read node.
+   *
+   * Phi-read nodes are like normal phi nodes, but they are inserted based
+   * on reads instead of writes.
+   */
+  predicate isPhiRead() { this instanceof SsaImpl::PhiReadNode }
 }
 
 class DefinitionExt = SsaImpl::DefinitionExt;

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

@@ -206,7 +206,34 @@ private predicate deconstructSizeExpr(Expr sizeExpr, Expr lengthExpr, int sizeof
 }
 
 /** A `Function` that is a call target of an allocation. */
-private signature class CallAllocationExprTarget extends Function;
+private signature class CallAllocationExprTarget extends Function {
+  /**
+   * Gets the index of the input pointer argument to be reallocated, if
+   * this is a `realloc` function.
+   */
+  int getReallocPtrArg();
+
+  /**
+   * Gets the index of the argument for the allocation size, if any. The actual
+   * allocation size is the value of this argument multiplied by the result of
+   * `getSizeMult()`, in bytes.
+   */
+  int getSizeArg();
+
+  /**
+   * Gets the index of an argument that multiplies the allocation size given
+   * by `getSizeArg`, if any.
+   */
+  int getSizeMult();
+
+  /**
+   * Holds if this allocation requires a
+   * corresponding deallocation of some sort (most do, but `alloca` for example
+   * does not). If it is unclear, we default to no (for example a placement `new`
+   * allocation may or may not require a corresponding `delete`).
+   */
+  predicate requiresDealloc();
+}
 
 /**
  * This module abstracts over the type of allocation call-targets and provides a
@@ -220,118 +247,68 @@ private signature class CallAllocationExprTarget extends Function;
  * function using various heuristics.
  */
 private module CallAllocationExprBase<CallAllocationExprTarget Target> {
-  /** A module that contains the collection of member-predicates required on `Target`. */
-  signature module Param {
-    /**
-     * Gets the index of the input pointer argument to be reallocated, if
-     * this is a `realloc` function.
-     */
-    int getReallocPtrArg(Target target);
-
-    /**
-     * Gets the index of the argument for the allocation size, if any. The actual
-     * allocation size is the value of this argument multiplied by the result of
-     * `getSizeMult()`, in bytes.
-     */
-    int getSizeArg(Target target);
-
-    /**
-     * Gets the index of an argument that multiplies the allocation size given
-     * by `getSizeArg`, if any.
-     */
-    int getSizeMult(Target target);
-
-    /**
-     * Holds if this allocation requires a
-     * corresponding deallocation of some sort (most do, but `alloca` for example
-     * does not). If it is unclear, we default to no (for example a placement `new`
-     * allocation may or may not require a corresponding `delete`).
-     */
-    predicate requiresDealloc(Target target);
-  }
-
   /**
-   * A module that abstracts over a collection of predicates in
-   * the `Param` module). This should really be member-predicates
-   * on `CallAllocationExprTarget`, but we cannot yet write this in QL.
+   * An allocation expression that is a function call, such as call to `malloc`.
    */
-  module With<Param P> {
-    private import P
-
-    /**
-     * An allocation expression that is a function call, such as call to `malloc`.
-     */
-    class CallAllocationExprImpl instanceof FunctionCall {
-      Target target;
-
-      CallAllocationExprImpl() {
-        target = this.getTarget() and
-        // realloc(ptr, 0) only frees the pointer
-        not (
-          exists(getReallocPtrArg(target)) and
-          this.getArgument(getSizeArg(target)).getValue().toInt() = 0
-        ) and
-        // these are modeled directly (and more accurately), avoid duplication
-        not exists(NewOrNewArrayExpr new | new.getAllocatorCall() = this)
-      }
-
-      string toString() { result = super.toString() }
-
-      Expr getSizeExprImpl() {
-        exists(Expr sizeExpr | sizeExpr = super.getArgument(getSizeArg(target)) |
-          if exists(getSizeMult(target))
-          then result = sizeExpr
-          else
-            exists(Expr lengthExpr |
-              deconstructSizeExpr(sizeExpr, lengthExpr, _) and
-              result = lengthExpr
-            )
-        )
-      }
-
-      int getSizeMultImpl() {
-        // malloc with multiplier argument that is a constant
-        result = super.getArgument(getSizeMult(target)).getValue().toInt()
-        or
-        // malloc with no multiplier argument
-        not exists(getSizeMult(target)) and
-        deconstructSizeExpr(super.getArgument(getSizeArg(target)), _, result)
-      }
-
-      int getSizeBytesImpl() {
-        result = this.getSizeExprImpl().getValue().toInt() * this.getSizeMultImpl()
-      }
-
-      Expr getReallocPtrImpl() { result = super.getArgument(getReallocPtrArg(target)) }
-
-      Type getAllocatedElementTypeImpl() {
-        result =
-          super.getFullyConverted().getType().stripTopLevelSpecifiers().(PointerType).getBaseType() and
-        not result instanceof VoidType
-      }
-
-      predicate requiresDeallocImpl() { requiresDealloc(target) }
+  class CallAllocationExprImpl instanceof FunctionCall {
+    Target target;
+
+    CallAllocationExprImpl() {
+      target = this.getTarget() and
+      // realloc(ptr, 0) only frees the pointer
+      not (
+        exists(target.getReallocPtrArg()) and
+        this.getArgument(target.getSizeArg()).getValue().toInt() = 0
+      ) and
+      // these are modeled directly (and more accurately), avoid duplication
+      not exists(NewOrNewArrayExpr new | new.getAllocatorCall() = this)
     }
-  }
-}
 
-private module CallAllocationExpr {
-  private module Param implements CallAllocationExprBase<AllocationFunction>::Param {
-    int getReallocPtrArg(AllocationFunction f) { result = f.getReallocPtrArg() }
+    string toString() { result = super.toString() }
+
+    Expr getSizeExprImpl() {
+      exists(Expr sizeExpr | sizeExpr = super.getArgument(target.getSizeArg()) |
+        if exists(target.getSizeMult())
+        then result = sizeExpr
+        else
+          exists(Expr lengthExpr |
+            deconstructSizeExpr(sizeExpr, lengthExpr, _) and
+            result = lengthExpr
+          )
+      )
+    }
+
+    int getSizeMultImpl() {
+      // malloc with multiplier argument that is a constant
+      result = super.getArgument(target.getSizeMult()).getValue().toInt()
+      or
+      // malloc with no multiplier argument
+      not exists(target.getSizeMult()) and
+      deconstructSizeExpr(super.getArgument(target.getSizeArg()), _, result)
+    }
+
+    int getSizeBytesImpl() {
+      result = this.getSizeExprImpl().getValue().toInt() * this.getSizeMultImpl()
+    }
 
-    int getSizeArg(AllocationFunction f) { result = f.getSizeArg() }
+    Expr getReallocPtrImpl() { result = super.getArgument(target.getReallocPtrArg()) }
 
-    int getSizeMult(AllocationFunction f) { result = f.getSizeMult() }
+    Type getAllocatedElementTypeImpl() {
+      result =
+        super.getFullyConverted().getType().stripTopLevelSpecifiers().(PointerType).getBaseType() and
+      not result instanceof VoidType
+    }
 
-    predicate requiresDealloc(AllocationFunction f) { f.requiresDealloc() }
+    predicate requiresDeallocImpl() { target.requiresDealloc() }
   }
+}
 
+private module CallAllocationExpr {
   /**
    * A class that provides the implementation of `AllocationExpr` for an allocation
    * that calls an `AllocationFunction`.
    */
-  private class Base =
-    CallAllocationExprBase<AllocationFunction>::With<Param>::CallAllocationExprImpl;
+  private class Base = CallAllocationExprBase<AllocationFunction>::CallAllocationExprImpl;
 
   class CallAllocationExpr extends AllocationExpr, Base {
     override Expr getSizeExpr() { result = super.getSizeExprImpl() }
@@ -437,7 +414,7 @@ private module HeuristicAllocation {
     int sizeArg;
 
     HeuristicAllocationFunctionByName() {
-      Function.super.getName().matches("%alloc%") and
+      Function.super.getName().matches(["%alloc%", "%Alloc%"]) and
       Function.super.getUnspecifiedType() instanceof PointerType and
       sizeArg = unique( | | getAnUnsignedParameter(this))
     }
@@ -452,22 +429,11 @@ private module HeuristicAllocation {
     override predicate requiresDealloc() { none() }
   }
 
-  private module Param implements CallAllocationExprBase<HeuristicAllocationFunction>::Param {
-    int getReallocPtrArg(HeuristicAllocationFunction f) { result = f.getReallocPtrArg() }
-
-    int getSizeArg(HeuristicAllocationFunction f) { result = f.getSizeArg() }
-
-    int getSizeMult(HeuristicAllocationFunction f) { result = f.getSizeMult() }
-
-    predicate requiresDealloc(HeuristicAllocationFunction f) { f.requiresDealloc() }
-  }
-
   /**
    * A class that provides the implementation of `AllocationExpr` for an allocation
    * that calls an `HeuristicAllocationFunction`.
    */
-  private class Base =
-    CallAllocationExprBase<HeuristicAllocationFunction>::With<Param>::CallAllocationExprImpl;
+  private class Base = CallAllocationExprBase<HeuristicAllocationFunction>::CallAllocationExprImpl;
 
   private class CallAllocationExpr extends HeuristicAllocationExpr, Base {
     override Expr getSizeExpr() { result = super.getSizeExprImpl() }

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeAnalysisStage.qll

@@ -277,7 +277,7 @@ module RangeStage<
    */
   private class SafeCastExpr extends ConvertOrBoxExpr {
     SafeCastExpr() {
-      conversionCannotOverflow(getTrackedType(pragma[only_bind_into](getOperand())),
+      conversionCannotOverflow(getTrackedType(pragma[only_bind_into](this.getOperand())),
         pragma[only_bind_out](getTrackedType(this)))
     }
   }

cpp/ql/lib/upgrades/282c13bfdbcbd57a887972b47a471342a4ad5507/member_function_this_type.ql

@@ -24,7 +24,7 @@ class ClassPointerType extends @derivedtype {
 
   Class getBaseType() { derivedtypes(this, _, _, result) }
 
-  string toString() { result = getBaseType().toString() + "*" }
+  string toString() { result = this.getBaseType().toString() + "*" }
 }
 
 class DefinedMemberFunction extends @function {

cpp/ql/src/experimental/Likely Bugs/OverrunWriteProductFlow.ql

@@ -47,7 +47,7 @@ VariableAccess getAVariableAccess(Expr e) { e.getAChild*() = result }
  * Holds if `(n, state)` pair represents the source of flow for the size
  * expression associated with `alloc`.
  */
-predicate hasSize(AllocationExpr alloc, DataFlow::Node n, int state) {
+predicate hasSize(HeuristicAllocationExpr alloc, DataFlow::Node n, int state) {
   exists(VariableAccess va, Expr size, int delta |
     size = alloc.getSizeExpr() and
     // Get the unique variable in a size expression like `x` in `malloc(x + 1)`.

cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql

@@ -229,6 +229,10 @@ module InvalidPointerToDerefConfig implements DataFlow::ConfigSig {
 
   pragma[inline]
   predicate isSink(DataFlow::Node sink) { isInvalidPointerDerefSink(sink, _, _) }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node = any(DataFlow::SsaPhiNode phi | not phi.isPhiRead()).getAnInput(true)
+  }
 }
 
 module InvalidPointerToDerefFlow = DataFlow::Global<InvalidPointerToDerefConfig>;