Merge branch 'main' into loc2 and accept new test results.

MODULE.bazel

@@ -128,6 +128,7 @@ use_repo(
     "kotlin-compiler-1.9.20-Beta",
     "kotlin-compiler-2.0.0-RC1",
     "kotlin-compiler-2.0.20-Beta2",
+    "kotlin-compiler-2.1.0-Beta1",
     "kotlin-compiler-embeddable-1.5.0",
     "kotlin-compiler-embeddable-1.5.10",
     "kotlin-compiler-embeddable-1.5.20",
@@ -141,6 +142,7 @@ use_repo(
     "kotlin-compiler-embeddable-1.9.20-Beta",
     "kotlin-compiler-embeddable-2.0.0-RC1",
     "kotlin-compiler-embeddable-2.0.20-Beta2",
+    "kotlin-compiler-embeddable-2.1.0-Beta1",
     "kotlin-stdlib-1.5.0",
     "kotlin-stdlib-1.5.10",
     "kotlin-stdlib-1.5.20",
@@ -154,6 +156,7 @@ use_repo(
     "kotlin-stdlib-1.9.20-Beta",
     "kotlin-stdlib-2.0.0-RC1",
     "kotlin-stdlib-2.0.20-Beta2",
+    "kotlin-stdlib-2.1.0-Beta1",
 )
 
 go_sdk = use_extension("@rules_go//go:extensions.bzl", "go_sdk")

config/identical-files.json

@@ -57,10 +57,6 @@
     "java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/SsaReadPositionCommon.qll",
     "csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/SsaReadPositionCommon.qll"
   ],
-  "Model as Data Generation Java/C# - CaptureModels": [
-    "java/ql/src/utils/modelgenerator/internal/CaptureModels.qll",
-    "csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll"
-  ],
   "Sign Java/C#": [
     "java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/Sign.qll",
     "csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/Sign.qll"

cpp/ql/lib/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 2.0.1
+
+No user-facing changes.
+
 ## 2.0.0
 
 ### Breaking Changes

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

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

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 2.0.0
+lastReleaseVersion: 2.0.1

cpp/ql/lib/qlpack.yml

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

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

@@ -651,7 +651,8 @@ class FunctionDeclarationEntry extends DeclarationEntry, @fun_decl {
 
   /**
    * Holds if this declaration is an implicit function declaration, that is,
-   * where a function is used before it is declared (under older C standards).
+   * where a function is used before it is declared (under older C standards,
+   * or when there were parse errors).
    */
   predicate isImplicit() { fun_implicit(underlyingElement(this)) }
 

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

@@ -39,8 +39,8 @@ class Type extends Locatable, @type {
 
   /**
    * Gets a specifier of this type, recursively looking through `typedef` and
-   * `decltype`. For example, in the context of `typedef const int *restrict
-   * t`, the type `volatile t` has specifiers `volatile` and `restrict` but not
+   * `decltype`. For example, in the context of `typedef const int *restrict t`,
+   * the type `volatile t` has specifiers `volatile` and `restrict` but not
    * `const` since the `const` is attached to the type being pointed to rather
    * than the pointer itself.
    */

cpp/ql/lib/semmle/code/cpp/models/interfaces/FormattingFunction.qll

@@ -118,19 +118,34 @@ abstract class FormattingFunction extends ArrayFunction, TaintFunction {
 
   /**
    * Gets the position of the first format argument, corresponding with
-   * the first format specifier in the format string.
+   * the first format specifier in the format string. We ignore all
+   * implicit function definitions.
    */
   int getFirstFormatArgumentIndex() {
-    result = this.getNumberOfParameters() and
-    // the formatting function either has a definition in the snapshot, or all
+    // The formatting function either has a definition in the snapshot, or all
     // `DeclarationEntry`s agree on the number of parameters (otherwise we don't
     // really know the correct number)
-    (
-      this.hasDefinition()
-      or
-      forall(FunctionDeclarationEntry fde | fde = this.getADeclarationEntry() |
-        result = fde.getNumberOfParameters()
-      )
+    if this.hasDefinition()
+    then result = this.getDefinition().getNumberOfParameters()
+    else result = this.getNumberOfExplicitParameters()
+  }
+
+  /**
+   * Gets a non-implicit function declaration entry.
+   */
+  private FunctionDeclarationEntry getAnExplicitDeclarationEntry() {
+    result = this.getADeclarationEntry() and
+    not result.isImplicit()
+  }
+
+  /**
+   * Gets the number of parameters, excluding any parameters that have been defined
+   * from implicit function declarations. If there is some inconsistency in the number
+   * of parameters, then don't return anything.
+   */
+  private int getNumberOfExplicitParameters() {
+    forex(FunctionDeclarationEntry fde | fde = this.getAnExplicitDeclarationEntry() |
+      result = fde.getNumberOfParameters()
     )
   }
 

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

@@ -72,7 +72,6 @@ module FlowFromFree<FlowFromFreeParamSig P> {
 
     predicate isSource(DataFlow::Node node, FlowState state) { isFree(node, _, state, _) }
 
-    pragma[inline]
     predicate isSink(DataFlow::Node sink, FlowState state) {
       exists(Expr e, DataFlow::Node source, DeallocationExpr dealloc |
         P::isSink(sink, e) and

cpp/ql/src/CHANGELOG.md

@@ -1,9 +1,15 @@
+## 1.2.4
+
+### Minor Analysis Improvements
+
+* Fixed false positives in the `cpp/wrong-number-format-arguments` ("Too few arguments to formatting function") query when the formatting function has been declared implicitly.
+
 ## 1.2.3
 
 ### Minor Analysis Improvements
 
-* Removed false positives caused by buffer accesses in unreachable code.
-* Removed false positives caused by inconsistent type checking.
+* Removed false positives caused by buffer accesses in unreachable code
+* Removed false positives caused by inconsistent type checking
 * Add modeling of C functions that don't throw, thereby increasing the precision of the `cpp/incorrect-allocation-error-handling` ("Incorrect allocation-error handling") query. The query now produces additional true positives.
 
 ## 1.2.2

cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.ql

@@ -13,23 +13,85 @@
  */
 
 import cpp
+import semmle.code.cpp.controlflow.Guards
 
 class WideCharPointerType extends PointerType {
   WideCharPointerType() { this.getBaseType() instanceof WideCharType }
 }
 
+/**
+ * Given type `t`, recurses through and returns all
+ * intermediate base types, including `t`.
+ */
+Type getABaseType(Type t) {
+  result = t
+  or
+  result = getABaseType(t.(DerivedType).getBaseType())
+  or
+  result = getABaseType(t.(TypedefType).getBaseType())
+}
+
 /**
  * A type that may also be `CharPointerType`, but that are likely used as arbitrary buffers.
  */
 class UnlikelyToBeAStringType extends Type {
   UnlikelyToBeAStringType() {
-    this.(PointerType).getBaseType().(CharType).isUnsigned() or
-    this.(PointerType).getBaseType().getName().toLowerCase().matches("%byte") or
-    this.getName().toLowerCase().matches("%byte") or
-    this.(PointerType).getBaseType().hasName("uint8_t")
+    exists(Type targ | getABaseType(this) = targ |
+      // NOTE: not using CharType isUnsigned, but rather look for any explicitly declared unsigned
+      // char types. Assuming these are used for buffers, not strings.
+      targ.(CharType).getName().toLowerCase().matches("unsigned%") or
+      targ.getName().toLowerCase().matches(["uint8_t", "%byte%"])
+    )
   }
 }
 
+// Types that can be wide depending on the UNICODE macro
+// see https://learn.microsoft.com/en-us/windows/win32/winprog/windows-data-types
+class UnicodeMacroDependentWidthType extends Type {
+  UnicodeMacroDependentWidthType() {
+    exists(Type targ | getABaseType(this) = targ |
+      targ.getName() in [
+          "LPCTSTR",
+          "LPTSTR",
+          "PCTSTR",
+          "PTSTR",
+          "TBYTE",
+          "TCHAR"
+        ]
+    )
+  }
+}
+
+class UnicodeMacro extends Macro {
+  UnicodeMacro() { this.getName().toLowerCase().matches("%unicode%") }
+}
+
+class UnicodeMacroInvocation extends MacroInvocation {
+  UnicodeMacroInvocation() { this.getMacro() instanceof UnicodeMacro }
+}
+
+/**
+ * Holds when a expression whose type is UnicodeMacroDependentWidthType and
+ * is observed to be guarded by a check involving a bitwise-and operation
+ * with a UnicodeMacroInvocation.
+ * Such expressions are assumed to be checked dynamically, i.e.,
+ * the flag would indicate if UNICODE typing is set correctly to allow
+ * or disallow a widening cast.
+ */
+predicate isLikelyDynamicallyChecked(Expr e) {
+  e.getType() instanceof UnicodeMacroDependentWidthType and
+  exists(GuardCondition gc, BitwiseAndExpr bai, UnicodeMacroInvocation umi |
+    bai.getAnOperand() = umi.getExpr()
+  |
+    // bai == 0 is false when reaching `e.getBasicBlock()`.
+    // That is, bai != 0 when reaching `e.getBasicBlock()`.
+    gc.ensuresEq(bai, 0, e.getBasicBlock(), false)
+    or
+    // bai == k and k != 0 is true when reaching `e.getBasicBlock()`.
+    gc.ensuresEq(bai, any(int k | k != 0), e.getBasicBlock(), true)
+  )
+}
+
 from Expr e1, Cast e2
 where
   e2 = e1.getConversion() and
@@ -42,7 +104,11 @@ where
   not e1.getType() instanceof UnlikelyToBeAStringType and
   // Avoid castings from 'new' expressions as typically these will be safe
   // Example: `__Type* ret = reinterpret_cast<__Type*>(New(m_pmo) char[num * sizeof(__Type)]);`
-  not exists(NewOrNewArrayExpr newExpr | newExpr.getAChild*() = e1)
+  not exists(NewOrNewArrayExpr newExpr | newExpr.getAChild*() = e1) and
+  // Avoid cases where the cast is guarded by a check to determine if
+  // unicode encoding is enabled in such a way to disallow the dangerous cast
+  // at runtime.
+  not isLikelyDynamicallyChecked(e1)
 select e1,
   "Conversion from " + e1.getType().toString() + " to " + e2.getType().toString() +
     ". Use of invalid string can lead to undefined behavior."

cpp/ql/src/change-notes/2024-09-26-wcharcharconversion-false-positives.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+* The `cpp/incorrect-string-type-conversion` query now produces fewer false positives caused by failure to detect byte arrays.
+* The `cpp/incorrect-string-type-conversion` query now produces fewer false positives caused by failure to recognize dynamic checks prior to possible dangerous widening.

cpp/ql/src/change-notes/released/1.2.4.md

@@ -0,0 +1,5 @@
+## 1.2.4
+
+### Minor Analysis Improvements
+
+* Fixed false positives in the `cpp/wrong-number-format-arguments` ("Too few arguments to formatting function") query when the formatting function has been declared implicitly.

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 1.2.3
+lastReleaseVersion: 1.2.4

cpp/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-queries
-version: 1.2.4-dev
+version: 1.2.5-dev
 groups:
   - cpp
   - queries