Merge branch 'main' into amammad-ruby-YAMLunsafeLoad

Author: aibaars

.github/workflows/swift.yml

@@ -16,6 +16,7 @@ on:
     branches:
       - main
       - rc/*
+      - codeql-cli-*
   push:
     paths:
       - "swift/**"
@@ -30,6 +31,7 @@ on:
     branches:
       - main
       - rc/*
+      - codeql-cli-*
 
 jobs:
   # not using a matrix as you cannot depend on a specific job in a matrix, and we want to start linux checks

config/identical-files.json

@@ -511,7 +511,8 @@
   "SensitiveDataHeuristics Python/JS": [
     "javascript/ql/lib/semmle/javascript/security/internal/SensitiveDataHeuristics.qll",
     "python/ql/lib/semmle/python/security/internal/SensitiveDataHeuristics.qll",
-    "ruby/ql/lib/codeql/ruby/security/internal/SensitiveDataHeuristics.qll"
+    "ruby/ql/lib/codeql/ruby/security/internal/SensitiveDataHeuristics.qll",
+    "swift/ql/lib/codeql/swift/security/internal/SensitiveDataHeuristics.qll"
   ],
   "CFG": [
     "csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImplShared.qll",
@@ -598,4 +599,4 @@
     "python/ql/lib/semmle/python/security/internal/EncryptionKeySizes.qll",
     "java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll"
   ]
-}
+}

cpp/ql/lib/CHANGELOG.md

@@ -1,3 +1,19 @@
+## 0.7.2
+
+### New Features
+
+* Added an AST-based interface (`semmle.code.cpp.rangeanalysis.new.RangeAnalysis`) for the relative range analysis library.
+* 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.
+
+### Major Analysis Improvements
+
+* In the intermediate representation, handling of control flow after non-returning calls has been improved. This should remove false positives in queries that use the intermedite representation or libraries based on it, including the new data flow library.
+
+### Minor Analysis Improvements
+
+* The `StdNamespace` class now also includes all inline namespaces that are children of `std` namespace.
+* The new dataflow (`semmle.code.cpp.dataflow.new.DataFlow`) and taint-tracking libraries (`semmle.code.cpp.dataflow.new.TaintTracking`) now support tracking flow through static local variables.
+
 ## 0.7.1
 
 No user-facing changes.

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

@@ -0,0 +1,15 @@
+## 0.7.2
+
+### New Features
+
+* Added an AST-based interface (`semmle.code.cpp.rangeanalysis.new.RangeAnalysis`) for the relative range analysis library.
+* 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.
+
+### Major Analysis Improvements
+
+* In the intermediate representation, handling of control flow after non-returning calls has been improved. This should remove false positives in queries that use the intermedite representation or libraries based on it, including the new data flow library.
+
+### Minor Analysis Improvements
+
+* The `StdNamespace` class now also includes all inline namespaces that are children of `std` namespace.
+* The new dataflow (`semmle.code.cpp.dataflow.new.DataFlow`) and taint-tracking libraries (`semmle.code.cpp.dataflow.new.TaintTracking`) now support tracking flow through static local variables.

cpp/ql/lib/change-notes/2023-04-28-static-local-dataflow.md

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.7.1
+lastReleaseVersion: 0.7.2

cpp/ql/lib/change-notes/2023-05-02-ir-noreturn-calls.md

@@ -1,5 +1,5 @@
 name: codeql/cpp-all
-version: 0.7.2-dev
+version: 0.7.3-dev
 groups: cpp
 dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp

cpp/ql/lib/change-notes/2023-05-02-range-analysis-wrapper.md

@@ -34,7 +34,7 @@ class Macro extends PreprocessorDirective, @ppd_define {
    * Gets the name of the macro.  For example, `MAX` in
    * `#define MAX(x,y) (((x)>(y))?(x):(y))`.
    */
-  string getName() { result = this.getHead().splitAt("(", 0) }
+  string getName() { result = this.getHead().regexpCapture("([^(]*+).*", 1) }
 
   /** Holds if the macro has name `name`. */
   predicate hasName(string name) { this.getName() = name }

cpp/ql/lib/change-notes/2023-05-22-inline-in-std-namespace.md

@@ -144,6 +144,20 @@ class AllocationInstruction extends CallInstruction {
   AllocationInstruction() { this.getStaticCallTarget() instanceof Cpp::AllocationFunction }
 }
 
+private predicate isIndirectionType(Type t) { t instanceof Indirection }
+
+private predicate hasUnspecifiedBaseType(Indirection t, Type base) {
+  base = t.getBaseType().getUnspecifiedType()
+}
+
+/**
+ * Holds if `t2` is the same type as `t1`, but after stripping away `result` number
+ * of indirections.
+ * Furthermore, specifies in `t2` been deeply stripped and typedefs has been resolved.
+ */
+private int getNumberOfIndirectionsImpl(Type t1, Type t2) =
+  shortestDistances(isIndirectionType/1, hasUnspecifiedBaseType/2)(t1, t2, result)
+
 /**
  * An abstract class for handling indirections.
  *
@@ -162,7 +176,10 @@ abstract class Indirection extends Type {
    * For example, the number of indirections of a variable `p` of type
    * `int**` is `3` (i.e., `p`, `*p` and `**p`).
    */
-  abstract int getNumberOfIndirections();
+  final int getNumberOfIndirections() {
+    result =
+      getNumberOfIndirectionsImpl(this.getType(), any(Type end | not end instanceof Indirection))
+  }
 
   /**
    * Holds if `deref` is an instruction that behaves as a `LoadInstruction`
@@ -200,19 +217,11 @@ private class PointerOrArrayOrReferenceTypeIndirection extends Indirection insta
   PointerOrArrayOrReferenceTypeIndirection() {
     baseType = PointerOrArrayOrReferenceType.super.getBaseType()
   }
-
-  override int getNumberOfIndirections() {
-    result = 1 + countIndirections(this.getBaseType().getUnspecifiedType())
-  }
 }
 
 private class PointerWrapperTypeIndirection extends Indirection instanceof PointerWrapper {
   PointerWrapperTypeIndirection() { baseType = PointerWrapper.super.getBaseType() }
 
-  override int getNumberOfIndirections() {
-    result = 1 + countIndirections(this.getBaseType().getUnspecifiedType())
-  }
-
   override predicate isAdditionalDereference(Instruction deref, Operand address) {
     exists(CallInstruction call |
       operandForFullyConvertedCall(getAUse(deref), call) and
@@ -233,10 +242,6 @@ private module IteratorIndirections {
       baseType = super.getValueType()
     }
 
-    override int getNumberOfIndirections() {
-      result = 1 + countIndirections(this.getBaseType().getUnspecifiedType())
-    }
-
     override predicate isAdditionalDereference(Instruction deref, Operand address) {
       exists(CallInstruction call |
         operandForFullyConvertedCall(getAUse(deref), call) and

cpp/ql/lib/change-notes/released/0.7.2.md

@@ -255,28 +255,14 @@ private module Cached {
   cached
   newtype TIRBlock = MkIRBlock(Instruction firstInstr) { startsBasicBlock(firstInstr) }
 
-  /** Gets the index of `i` in its `IRBlock`. */
-  private int getMemberIndex(Instruction i) {
-    startsBasicBlock(i) and
-    result = 0
-    or
-    exists(Instruction iPrev |
-      adjacentInBlock(iPrev, i) and
-      result = getMemberIndex(iPrev) + 1
-    )
-  }
-
-  private module BlockAdjacency = QlBuiltins::EquivalenceRelation<Instruction, adjacentInBlock/2>;
+  /** Holds if `i` is the `index`th instruction the block starting with `first`. */
+  private Instruction getInstructionFromFirst(Instruction first, int index) =
+    shortestDistances(startsBasicBlock/1, adjacentInBlock/2)(first, result, index)
 
   /** Holds if `i` is the `index`th instruction in `block`. */
   cached
   Instruction getInstruction(TIRBlock block, int index) {
-    exists(Instruction first | block = MkIRBlock(first) |
-      first = result and index = 0
-      or
-      index = getMemberIndex(result) and
-      BlockAdjacency::getEquivalenceClass(first) = BlockAdjacency::getEquivalenceClass(result)
-    )
+    result = getInstructionFromFirst(getFirstInstruction(block), index)
   }
 
   cached

cpp/ql/lib/codeql-pack.release.yml

@@ -255,28 +255,14 @@ private module Cached {
   cached
   newtype TIRBlock = MkIRBlock(Instruction firstInstr) { startsBasicBlock(firstInstr) }
 
-  /** Gets the index of `i` in its `IRBlock`. */
-  private int getMemberIndex(Instruction i) {
-    startsBasicBlock(i) and
-    result = 0
-    or
-    exists(Instruction iPrev |
-      adjacentInBlock(iPrev, i) and
-      result = getMemberIndex(iPrev) + 1
-    )
-  }
-
-  private module BlockAdjacency = QlBuiltins::EquivalenceRelation<Instruction, adjacentInBlock/2>;
+  /** Holds if `i` is the `index`th instruction the block starting with `first`. */
+  private Instruction getInstructionFromFirst(Instruction first, int index) =
+    shortestDistances(startsBasicBlock/1, adjacentInBlock/2)(first, result, index)
 
   /** Holds if `i` is the `index`th instruction in `block`. */
   cached
   Instruction getInstruction(TIRBlock block, int index) {
-    exists(Instruction first | block = MkIRBlock(first) |
-      first = result and index = 0
-      or
-      index = getMemberIndex(result) and
-      BlockAdjacency::getEquivalenceClass(first) = BlockAdjacency::getEquivalenceClass(result)
-    )
+    result = getInstructionFromFirst(getFirstInstruction(block), index)
   }
 
   cached

cpp/ql/lib/qlpack.yml

@@ -255,28 +255,14 @@ private module Cached {
   cached
   newtype TIRBlock = MkIRBlock(Instruction firstInstr) { startsBasicBlock(firstInstr) }
 
-  /** Gets the index of `i` in its `IRBlock`. */
-  private int getMemberIndex(Instruction i) {
-    startsBasicBlock(i) and
-    result = 0
-    or
-    exists(Instruction iPrev |
-      adjacentInBlock(iPrev, i) and
-      result = getMemberIndex(iPrev) + 1
-    )
-  }
-
-  private module BlockAdjacency = QlBuiltins::EquivalenceRelation<Instruction, adjacentInBlock/2>;
+  /** Holds if `i` is the `index`th instruction the block starting with `first`. */
+  private Instruction getInstructionFromFirst(Instruction first, int index) =
+    shortestDistances(startsBasicBlock/1, adjacentInBlock/2)(first, result, index)
 
   /** Holds if `i` is the `index`th instruction in `block`. */
   cached
   Instruction getInstruction(TIRBlock block, int index) {
-    exists(Instruction first | block = MkIRBlock(first) |
-      first = result and index = 0
-      or
-      index = getMemberIndex(result) and
-      BlockAdjacency::getEquivalenceClass(first) = BlockAdjacency::getEquivalenceClass(result)
-    )
+    result = getInstructionFromFirst(getFirstInstruction(block), index)
   }
 
   cached

cpp/ql/lib/semmle/code/cpp/Macro.qll

@@ -1,3 +1,7 @@
+## 0.6.2
+
+No user-facing changes.
+
 ## 0.6.1
 
 ### New Queries

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

@@ -0,0 +1,3 @@
+## 0.6.2
+
+No user-facing changes.

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/IRBlock.qll

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.6.1
+lastReleaseVersion: 0.6.2

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/IRBlock.qll

@@ -0,0 +1,33 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Detects <code>if (a+b>c) a=c-b</code>, which incorrectly implements
+<code>a = min(a,c-b)</code> if <code>a+b</code> overflows.
+</p>
+<p>
+Also detects variants such as <code>if (b+a>c) a=c-b</code> (swapped
+terms in addition), <code>if (a+b>c) { a=c-b }</code> (assignment
+inside block), <code>c&lt;a+b</code> (swapped operands), and
+<code>&gt;=</code>, <code>&lt;</code>, <code>&lt;=</code> instead of 
+<code>&gt;</code> (all operators).
+</p>
+<p>
+This integer overflow is the root cause of the buffer overflow in
+the SHA-3 reference implementation (CVE-2022-37454).
+</p>
+</overview>
+<recommendation>
+<p>
+Replace by <code>if (a>c-b) a=c-b</code>. This avoids the overflow
+and makes it easy to see that <code>a = min(a,c-b)</code>.
+</p>
+</recommendation>
+<references>
+<li>CVE-2022-37454: <a href="https://nvd.nist.gov/vuln/detail/CVE-2022-37454">The Keccak XKCP SHA-3 reference implementation before fdc6fef has an integer overflow and resultant buffer overflow that allows attackers to execute arbitrary code or eliminate expected cryptographic properties. This occurs in the sponge function interface.</a></li>
+<li>GitHub Advisory Database: <a href="https://github.com/advisories/GHSA-6w4m-2xhg-2658">CVE-2022-37454: Buffer overflow in sponge queue functions</a></li>
+</references>
+</qhelp>

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/IRBlock.qll

@@ -0,0 +1,42 @@
+/**
+ * @name Integer addition may overflow inside if statement
+ * @description Writing 'if (a+b>c) a=c-b' incorrectly implements
+ *              'a = min(a,c-b)' if 'a+b' overflows. This integer
+ *              overflow is the root cause of the buffer overflow
+ *              in the SHA-3 reference implementation (CVE-2022-37454).
+ * @kind problem
+ * @problem.severity warning
+ * @id cpp/if-statement-addition-overflow
+ * @tags: experimental
+ *        correctness
+ *        security
+ *        external/cwe/cwe-190
+ */
+
+import cpp
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+import semmle.code.cpp.valuenumbering.HashCons
+import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+import semmle.code.cpp.controlflow.Guards
+
+from
+  GuardCondition guard, Expr expr, ExprStmt exprstmt, BasicBlock block, AssignExpr assignexpr,
+  AddExpr addexpr, SubExpr subexpr
+where
+  (guard.ensuresLt(expr, addexpr, 0, block, _) or guard.ensuresLt(addexpr, expr, 0, block, _)) and
+  addexpr.getUnspecifiedType() instanceof IntegralType and
+  exprMightOverflowPositively(addexpr) and
+  block.getANode() = exprstmt and
+  exprstmt.getExpr() = assignexpr and
+  assignexpr.getRValue() = subexpr and
+  (
+    hashCons(addexpr.getLeftOperand()) = hashCons(assignexpr.getLValue()) and
+    globalValueNumber(addexpr.getRightOperand()) = globalValueNumber(subexpr.getRightOperand())
+    or
+    hashCons(addexpr.getRightOperand()) = hashCons(assignexpr.getLValue()) and
+    globalValueNumber(addexpr.getLeftOperand()) = globalValueNumber(subexpr.getRightOperand())
+  ) and
+  globalValueNumber(expr) = globalValueNumber(subexpr.getLeftOperand())
+select guard,
+  "\"if (a+b>c) a=c-b\" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as \"if (a>c-b) a=c-b\" which avoids the overflow.",
+  addexpr, "addition"

cpp/ql/src/CHANGELOG.md

@@ -78,11 +78,17 @@ predicate isInvalidPointerDerefSink2(DataFlow::Node sink, Instruction i, string
   )
 }
 
+pragma[nomagic]
+predicate arrayTypeHasSizes(ArrayType arr, int baseTypeSize, int arraySize) {
+  arr.getBaseType().getSize() = baseTypeSize and
+  arr.getArraySize() = arraySize
+}
+
 predicate pointerArithOverflow0(
   PointerArithmeticInstruction pai, Field f, int size, int bound, int delta
 ) {
-  pai.getElementSize() = f.getUnspecifiedType().(ArrayType).getBaseType().getSize() and
-  f.getUnspecifiedType().(ArrayType).getArraySize() = size and
+  not f.getNamespace() instanceof StdNamespace and
+  arrayTypeHasSizes(f.getUnspecifiedType(), pai.getElementSize(), size) and
   semBounded(getSemanticExpr(pai.getRight()), any(SemZeroBound b), bound, true, _) and
   delta = bound - size and
   delta >= 0 and