Merge branch 'main' into python-flask-session-interface

Author: joefarebrother

.bazelversion

@@ -1 +1 @@
-7.1.0
+7.1.2

cpp/downgrades/19887dbd33327fb07d54251786e0cb2578539775/upgrade.properties

@@ -1,4 +1,4 @@
 description: Revert support for repeated initializers, which are allowed in C with designated initializers.
 compatibility: full
-aggregate_field_init.rel: reorder aggregate_field_init.rel (int aggregate, int initializer, int field, int position) aggregate initializer field
-aggregate_array_init.rel: reorder aggregate_array_init.rel (int aggregate, int initializer, int element_index, int position) aggregate initializer element_index
+aggregate_field_init.rel: reorder aggregate_field_init.rel (@aggregateliteral aggregate, @expr initializer, @membervariable field, int position) aggregate initializer field
+aggregate_array_init.rel: reorder aggregate_array_init.rel (@aggregateliteral aggregate, @expr initializer, int element_index, int position) aggregate initializer element_index

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

@@ -565,7 +565,7 @@ class IRGuardCondition extends Instruction {
   /** Holds if (determined by this guard) `op == k` evaluates to `areEqual` if this expression evaluates to `value`. */
   cached
   predicate comparesEq(Operand op, int k, boolean areEqual, AbstractValue value) {
-    compares_eq(this, op, k, areEqual, value)
+    unary_compares_eq(this, op, k, areEqual, false, value)
   }
 
   /**
@@ -586,7 +586,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresEq(Operand op, int k, IRBlock block, boolean areEqual) {
     exists(AbstractValue value |
-      compares_eq(this, op, k, areEqual, value) and this.valueControls(block, value)
+      unary_compares_eq(this, op, k, areEqual, false, value) and this.valueControls(block, value)
     )
   }
 
@@ -611,7 +611,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresEqEdge(Operand op, int k, IRBlock pred, IRBlock succ, boolean areEqual) {
     exists(AbstractValue value |
-      compares_eq(this, op, k, areEqual, value) and
+      unary_compares_eq(this, op, k, areEqual, false, value) and
       this.valueControlsEdge(pred, succ, value)
     )
   }
@@ -737,26 +737,66 @@ private predicate compares_eq(
   )
 }
 
-/** Holds if `op == k` is `areEqual` given that `test` is equal to `value`. */
-private predicate compares_eq(
-  Instruction test, Operand op, int k, boolean areEqual, AbstractValue value
+/**
+ * Holds if `op == k` is `areEqual` given that `test` is equal to `value`.
+ *
+ * Many internal predicates in this file have a `inNonZeroCase` column.
+ * Ideally, the `k` column would be a type such as `Option<int>::Option`, to
+ * represent whether we have a concrete value `k` such that `op == k`, or whether
+ * we only know that `op != 0`.
+ * However, cannot instantiate `Option` with an infinite type. Thus the boolean
+ * `inNonZeroCase` is used to distinquish the `Some` (where we have a concrete
+ * value `k`) and `None` cases (where we only know that `op != 0`).
+ *
+ * Thus, if `inNonZeroCase = true` then `op != 0` and the value of `k` is
+ * meaningless.
+ *
+ * To see why `inNonZeroCase` is needed consider the following C program:
+ * ```c
+ * char* p = ...;
+ * if(p) {
+ *   use(p);
+ * }
+ * ```
+ * in C++ there would be an int-to-bool conversion on `p`. However, since C
+ * does not have booleans there is no conversion. We want to be able to
+ * conclude that `p` is non-zero in the true branch, so we need to give `k`
+ * some value. However, simply setting `k = 1` would make the rest of the
+ * analysis think that `k == 1` holds inside the branch. So we distinquish
+ * between the above case and
+ * ```c
+ * if(p == 1) {
+ *   use(p)
+ * }
+ * ```
+ * by setting `inNonZeroCase` to `true` in the former case, but not in the
+ * latter.
+ */
+private predicate unary_compares_eq(
+  Instruction test, Operand op, int k, boolean areEqual, boolean inNonZeroCase, AbstractValue value
 ) {
   /* The simple case where the test *is* the comparison so areEqual = testIsTrue xor eq. */
-  exists(AbstractValue v | simple_comparison_eq(test, op, k, v) |
+  exists(AbstractValue v | unary_simple_comparison_eq(test, op, k, inNonZeroCase, v) |
     areEqual = true and value = v
     or
     areEqual = false and value = v.getDualValue()
   )
   or
-  complex_eq(test, op, k, areEqual, value)
+  unary_complex_eq(test, op, k, areEqual, inNonZeroCase, value)
   or
   /* (x is true => (op == k)) => (!x is false => (op == k)) */
-  exists(AbstractValue dual | value = dual.getDualValue() |
-    compares_eq(test.(LogicalNotInstruction).getUnary(), op, k, areEqual, dual)
+  exists(AbstractValue dual, boolean inNonZeroCase0 |
+    value = dual.getDualValue() and
+    unary_compares_eq(test.(LogicalNotInstruction).getUnary(), op, k, inNonZeroCase0, areEqual, dual)
+  |
+    k = 0 and inNonZeroCase = inNonZeroCase0
+    or
+    k != 0 and inNonZeroCase = true
   )
   or
   // ((test is `areEqual` => op == const + k2) and const == `k1`) =>
   // test is `areEqual` => op == k1 + k2
+  inNonZeroCase = false and
   exists(int k1, int k2, ConstantInstruction const |
     compares_eq(test, op, const.getAUse(), k2, areEqual, value) and
     int_value(const) = k1 and
@@ -781,35 +821,53 @@ private predicate simple_comparison_eq(
   value.(BooleanValue).getValue() = false
 }
 
-/** Rearrange various simple comparisons into `op == k` form. */
-private predicate simple_comparison_eq(Instruction test, Operand op, int k, AbstractValue value) {
+/**
+ * Holds if `test` is an instruction that is part of test that eventually is
+ * used in a conditional branch.
+ */
+private predicate relevantUnaryComparison(Instruction test) {
+  not test instanceof CompareInstruction and
+  exists(IRType type, ConditionalBranchInstruction branch |
+    type instanceof IRAddressType or type instanceof IRIntegerType
+  |
+    type = test.getResultIRType() and
+    branch.getCondition() = test
+  )
+  or
+  exists(LogicalNotInstruction logicalNot |
+    relevantUnaryComparison(logicalNot) and
+    test = logicalNot.getUnary()
+  )
+}
+
+/**
+ * Rearrange various simple comparisons into `op == k` form.
+ */
+private predicate unary_simple_comparison_eq(
+  Instruction test, Operand op, int k, boolean inNonZeroCase, AbstractValue value
+) {
   exists(SwitchInstruction switch, CaseEdge case |
     test = switch.getExpression() and
     op.getDef() = test and
     case = value.(MatchValue).getCase() and
     exists(switch.getSuccessor(case)) and
-    case.getValue().toInt() = k
+    case.getValue().toInt() = k and
+    inNonZeroCase = false
   )
   or
   // There's no implicit CompareInstruction in files compiled as C since C
   // doesn't have implicit boolean conversions. So instead we check whether
   // there's a branch on a value of pointer or integer type.
-  exists(ConditionalBranchInstruction branch, IRType type |
-    not test instanceof CompareInstruction and
-    type = test.getResultIRType() and
-    (type instanceof IRAddressType or type instanceof IRIntegerType) and
-    test = branch.getCondition() and
-    op.getDef() = test
-  |
-    // We'd like to also include a case such as:
-    // ```
-    // k = 1 and
-    // value.(BooleanValue).getValue() = true
-    // ```
-    // but all we know is that the value is non-zero in the true branch.
-    // So we can only conclude something in the false branch.
+  relevantUnaryComparison(test) and
+  op.getDef() = test and
+  (
+    k = 1 and
+    value.(BooleanValue).getValue() = true and
+    inNonZeroCase = true
+    or
     k = 0 and
-    value.(BooleanValue).getValue() = false
+    value.(BooleanValue).getValue() = false and
+    inNonZeroCase = false
   )
 }
 
@@ -821,12 +879,12 @@ private predicate complex_eq(
   add_eq(cmp, left, right, k, areEqual, value)
 }
 
-private predicate complex_eq(
-  Instruction test, Operand op, int k, boolean areEqual, AbstractValue value
+private predicate unary_complex_eq(
+  Instruction test, Operand op, int k, boolean areEqual, boolean inNonZeroCase, AbstractValue value
 ) {
-  sub_eq(test, op, k, areEqual, value)
+  unary_sub_eq(test, op, k, areEqual, inNonZeroCase, value)
   or
-  add_eq(test, op, k, areEqual, value)
+  unary_add_eq(test, op, k, areEqual, inNonZeroCase, value)
 }
 
 /*
@@ -1090,16 +1148,20 @@ private predicate sub_eq(
 }
 
 // op - x == c => op == (c+x)
-private predicate sub_eq(Instruction test, Operand op, int k, boolean areEqual, AbstractValue value) {
+private predicate unary_sub_eq(
+  Instruction test, Operand op, int k, boolean areEqual, boolean inNonZeroCase, AbstractValue value
+) {
+  inNonZeroCase = false and
   exists(SubInstruction sub, int c, int x |
-    compares_eq(test, sub.getAUse(), c, areEqual, value) and
+    unary_compares_eq(test, sub.getAUse(), c, areEqual, inNonZeroCase, value) and
     op = sub.getLeftOperand() and
     x = int_value(sub.getRight()) and
     k = c + x
   )
   or
+  inNonZeroCase = false and
   exists(PointerSubInstruction sub, int c, int x |
-    compares_eq(test, sub.getAUse(), c, areEqual, value) and
+    unary_compares_eq(test, sub.getAUse(), c, areEqual, inNonZeroCase, value) and
     op = sub.getLeftOperand() and
     x = int_value(sub.getRight()) and
     k = c + x
@@ -1153,11 +1215,13 @@ private predicate add_eq(
 }
 
 // left + x == right + c => left == right + (c-x)
-private predicate add_eq(
-  Instruction test, Operand left, int k, boolean areEqual, AbstractValue value
+private predicate unary_add_eq(
+  Instruction test, Operand left, int k, boolean areEqual, boolean inNonZeroCase,
+  AbstractValue value
 ) {
+  inNonZeroCase = false and
   exists(AddInstruction lhs, int c, int x |
-    compares_eq(test, lhs.getAUse(), c, areEqual, value) and
+    unary_compares_eq(test, lhs.getAUse(), c, areEqual, inNonZeroCase, value) and
     (
       left = lhs.getLeftOperand() and x = int_value(lhs.getRight())
       or
@@ -1166,8 +1230,9 @@ private predicate add_eq(
     k = c - x
   )
   or
+  inNonZeroCase = false and
   exists(PointerAddInstruction lhs, int c, int x |
-    compares_eq(test, lhs.getAUse(), c, areEqual, value) and
+    unary_compares_eq(test, lhs.getAUse(), c, areEqual, inNonZeroCase, value) and
     (
       left = lhs.getLeftOperand() and x = int_value(lhs.getRight())
       or

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

@@ -546,7 +546,7 @@ module ProductFlow {
       Flow1::PathGraph::edges(pred1, succ1, _, _) and
       exists(ReturnKindExt returnKind |
         succ1.getNode() = returnKind.getAnOutNode(call) and
-        pred1.getNode().(ReturnNodeExt).getKind() = returnKind
+        paramReturnNode(_, pred1.asParameterReturnNode(), _, returnKind)
       )
     }
 
@@ -574,7 +574,7 @@ module ProductFlow {
       Flow2::PathGraph::edges(pred2, succ2, _, _) and
       exists(ReturnKindExt returnKind |
         succ2.getNode() = returnKind.getAnOutNode(call) and
-        pred2.getNode().(ReturnNodeExt).getKind() = returnKind
+        paramReturnNode(_, pred2.asParameterReturnNode(), _, returnKind)
       )
     }
 

cpp/ql/lib/upgrades/ddd31fd02e51ad270bc9e6712708e5a5b6881518/upgrade.properties

@@ -1,4 +1,4 @@
 description: Removed unused column from the `folders` and `files` relations
 compatibility: full
-files.rel: reorder files.rel (int id, string name, string simple, string ext, int fromSource) id name
-folders.rel: reorder folders.rel (int id, string name, string simple) id name
+files.rel: reorder files.rel (@file id, string name, string simple, string ext, int fromSource) id name
+folders.rel: reorder folders.rel (@folder id, string name, string simple) id name

cpp/ql/src/Critical/NotInitialised.ql

@@ -54,6 +54,7 @@ predicate undefinedLocalUse(VariableAccess va) {
     // it is hard to tell when a struct or array has been initialized, so we
     // ignore them
     not isAggregateType(lv.getUnderlyingType()) and
+    not lv.isStatic() and // static variables are initialized to zero or null by default
     not lv.getType().hasName("va_list") and
     va = lv.getAnAccess() and
     noDefPath(lv, va) and
@@ -70,7 +71,8 @@ predicate uninitialisedGlobal(GlobalVariable gv) {
     va = gv.getAnAccess() and
     va.isRValue() and
     not gv.hasInitializer() and
-    not gv.hasSpecifier("extern")
+    not gv.hasSpecifier("extern") and
+    not gv.isStatic() // static variables are initialized to zero or null by default
   )
 }
 

cpp/ql/src/Likely Bugs/Format/NonConstantFormat.qhelp

@@ -42,7 +42,7 @@ in the previous example, one solution is to make the log message a trailing argu
 <p>An alternative solution is to allow <code>log_with_timestamp</code> to accept format arguments:</p>
 <sample src="NonConstantFormat-2-good.c" />
 <p>In this formulation, the non-constant format string to <code>printf</code> has been replaced with
-a non-constant format string to <code>vprintf</code>. Semmle will no longer consider the body of
+a non-constant format string to <code>vprintf</code>. The analysis will no longer consider the body of
 <code>log_with_timestamp</code> to be a problem, and will instead check that every call to
 <code>log_with_timestamp</code> passes a constant format string.</p>
 

cpp/ql/src/Likely Bugs/Memory Management/AllocaInLoop.ql

@@ -37,6 +37,19 @@ class AllocaCall extends FunctionCall {
   }
 }
 
+/**
+ * Gets an expression associated with a dataflow node.
+ */
+private Expr getExpr(DataFlow::Node node) {
+  result = node.asInstruction().getAst()
+  or
+  result = node.asOperand().getUse().getAst()
+  or
+  result = node.(DataFlow::RawIndirectInstruction).getInstruction().getAst()
+  or
+  result = node.(DataFlow::RawIndirectOperand).getOperand().getUse().getAst()
+}
+
 /**
  * A loop that contains an `alloca` call.
  */
@@ -185,19 +198,6 @@ class LoopWithAlloca extends Stmt {
     not this.conditionReachesWithoutUpdate(var, this.(Loop).getCondition())
   }
 
-  /**
-   * Gets an expression associated with a dataflow node.
-   */
-  private Expr getExpr(DataFlow::Node node) {
-    result = node.asInstruction().getAst()
-    or
-    result = node.asOperand().getUse().getAst()
-    or
-    result = node.(DataFlow::RawIndirectInstruction).getInstruction().getAst()
-    or
-    result = node.(DataFlow::RawIndirectOperand).getOperand().getUse().getAst()
-  }
-
   /**
    * Gets a definition that may be the most recent definition of the
    * controlling variable `var` before this loop.
@@ -210,7 +210,7 @@ class LoopWithAlloca extends Stmt {
       // Phi nodes will be preceded by nodes that represent actual definitions
       not result instanceof DataFlow::SsaPhiNode and
       // A source is outside the loop if it's not inside the loop
-      not exists(Expr e | e = this.getExpr(result) | this = getAnEnclosingLoopOfExpr(e))
+      not exists(Expr e | e = getExpr(result) | this = getAnEnclosingLoopOfExpr(e))
     )
   }
 
@@ -221,9 +221,9 @@ class LoopWithAlloca extends Stmt {
   private int getAControllingVarInitialValue(Variable var, DataFlow::Node source) {
     source = this.getAPrecedingDef(var) and
     (
-      result = this.getExpr(source).getValue().toInt()
+      result = getExpr(source).getValue().toInt()
       or
-      result = this.getExpr(source).(Assignment).getRValue().getValue().toInt()
+      result = getExpr(source).(Assignment).getRValue().getValue().toInt()
     )
   }
 

cpp/ql/src/Likely Bugs/Memory Management/NtohlArrayNoBound.qll

@@ -107,7 +107,7 @@ class SnprintfSizeExpr extends BufferAccess, FunctionCall {
 }
 
 class MemcmpSizeExpr extends BufferAccess, FunctionCall {
-  MemcmpSizeExpr() { this.getTarget().hasName("Memcmp") }
+  MemcmpSizeExpr() { this.getTarget().hasName("memcmp") }
 
   override Expr getPointer() {
     result = this.getArgument(0) or

cpp/ql/src/Security/CWE/CWE-416/UseOfUniquePointerAfterLifetimeEnds.ql

@@ -30,6 +30,8 @@ where
   outlivesFullExpr(c) and
   not c.isFromUninstantiatedTemplate(_) and
   isUniquePointerDerefFunction(c.getTarget()) and
+  // Exclude cases where the pointer is implicitly converted to a non-pointer type
+  not c.getActualType() instanceof IntegralType and
   isTemporary(c.getQualifier().getFullyConverted())
 select c,
   "The underlying unique pointer object is destroyed after the call to '" + c.getTarget() +