Merge branch 'main' into marcogario-patch-1

Author: marcogario

actions/ql/src/Security/CWE-829/UntrustedCheckoutCritical.ql

@@ -1,5 +1,5 @@
 /**
- * @name Checkout of untrusted code in trusted context
+ * @name Checkout of untrusted code in a priviledged context
  * @description Privileged workflows have read/write access to the base repository and access to secrets.
  *              By explicitly checking out and running the build script from a fork the untrusted code is running in an environment
  *              that is able to push to the base repository and to access secrets.

cpp/ql/lib/change-notes/2025-03-31-calling-convention.md

@@ -0,0 +1,5 @@
+---
+category: feature
+---
+* Calling conventions explicitly specified on function declarations (`__cdecl`, `__stdcall`, `__fastcall`, etc.)  are now represented as specifiers of those declarations.
+* A new class `CallingConventionSpecifier` extending the `Specifier` class was introduced, which represents explicitly specified calling conventions.

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

@@ -97,6 +97,18 @@ class AccessSpecifier extends Specifier {
   override string getAPrimaryQlClass() { result = "AccessSpecifier" }
 }
 
+/**
+ * A C/C++ calling convention specifier: `cdecl`, `fastcall`, `stdcall`, `thiscall`,
+ * `vectorcall`, or `clrcall`.
+ */
+class CallingConventionSpecifier extends Specifier {
+  CallingConventionSpecifier() {
+    this.hasName(["cdecl", "fastcall", "stdcall", "thiscall", "vectorcall", "clrcall"])
+  }
+
+  override string getAPrimaryQlClass() { result = "CallingConventionSpecifier" }
+}
+
 /**
  * An attribute introduced by GNU's `__attribute__((name))` syntax,
  * Microsoft's `__declspec(name)` syntax, Microsoft's `[name]` syntax, the

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

@@ -1834,15 +1834,55 @@ module IteratorFlow {
 
   private module IteratorSsa = SsaImpl::Make<Location, SsaInput>;
 
-  private class Def extends IteratorSsa::DefinitionExt {
+  private module DataFlowIntegrationInput implements IteratorSsa::DataFlowIntegrationInputSig {
+    private import codeql.util.Void
+
+    class Expr extends Instruction {
+      Expr() {
+        exists(IRBlock bb, int i |
+          SsaInput::variableRead(bb, i, _, true) and
+          this = bb.getInstruction(i)
+        )
+      }
+
+      predicate hasCfgNode(SsaInput::BasicBlock bb, int i) { bb.getInstruction(i) = this }
+    }
+
+    predicate ssaDefHasSource(IteratorSsa::WriteDefinition def) { none() }
+
+    predicate allowFlowIntoUncertainDef(IteratorSsa::UncertainWriteDefinition def) { any() }
+
+    class Guard extends Void {
+      predicate controlsBranchEdge(
+        SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch
+      ) {
+        none()
+      }
+    }
+
+    predicate guardDirectlyControlsBlock(Guard guard, SsaInput::BasicBlock bb, boolean branch) {
+      none()
+    }
+
+    predicate supportBarrierGuardsOnPhiEdges() { none() }
+  }
+
+  private module DataFlowIntegrationImpl =
+    IteratorSsa::DataFlowIntegration<DataFlowIntegrationInput>;
+
+  private class IteratorSynthNode extends DataFlowIntegrationImpl::SsaNode {
+    IteratorSynthNode() { not this.asDefinition() instanceof IteratorSsa::WriteDefinition }
+  }
+
+  private class Def extends IteratorSsa::Definition {
     final override Location getLocation() { result = this.getImpl().getLocation() }
 
     /**
      * Holds if this definition (or use) has index `index` in block `block`,
      * and is a definition (or use) of the variable `sv`.
      */
     predicate hasIndexInBlock(IRBlock block, int index, SourceVariable sv) {
-      super.definesAt(sv, block, index, _)
+      super.definesAt(sv, block, index)
     }
 
     private Ssa::DefImpl getImpl() {
@@ -1859,46 +1899,15 @@ module IteratorFlow {
     int getIndirectionIndex() { result = this.getImpl().getIndirectionIndex() }
   }
 
-  private class PhiNode extends IteratorSsa::DefinitionExt {
-    PhiNode() {
-      this instanceof IteratorSsa::PhiNode or
-      this instanceof IteratorSsa::PhiReadNode
-    }
-
-    SsaIteratorNode getNode() { result.getIteratorFlowNode() = this }
-  }
-
-  cached
-  private module IteratorSsaCached {
-    cached
-    predicate adjacentDefRead(IRBlock bb1, int i1, SourceVariable sv, IRBlock bb2, int i2) {
-      IteratorSsa::adjacentDefReadExt(_, sv, bb1, i1, bb2, i2)
-      or
-      exists(PhiNode phi |
-        IteratorSsa::lastRefRedefExt(_, sv, bb1, i1, phi) and
-        phi.definesAt(sv, bb2, i2, _)
-      )
-    }
-
-    cached
-    Node getAPriorDefinition(IteratorSsa::DefinitionExt next) {
-      exists(IRBlock bb, int i, SourceVariable sv, IteratorSsa::DefinitionExt def |
-        IteratorSsa::lastRefRedefExt(pragma[only_bind_into](def), pragma[only_bind_into](sv),
-          pragma[only_bind_into](bb), pragma[only_bind_into](i), next) and
-        nodeToDefOrUse(result, sv, bb, i, _)
-      )
-    }
-  }
-
   /** The set of nodes necessary for iterator flow. */
-  class IteratorFlowNode instanceof PhiNode {
+  class IteratorFlowNode instanceof IteratorSynthNode {
     /** Gets a textual representation of this node. */
     string toString() { result = super.toString() }
 
     /** Gets the type of this node. */
     DataFlowType getType() {
       exists(Ssa::SourceVariable sv |
-        super.definesAt(sv, _, _, _) and
+        super.getSourceVariable() = sv and
         result = sv.getType()
       )
     }
@@ -1910,60 +1919,33 @@ module IteratorFlow {
     Location getLocation() { result = super.getBasicBlock().getLocation() }
   }
 
-  private import IteratorSsaCached
-
-  private predicate defToNode(Node node, Def def, boolean uncertain) {
-    (
-      nodeHasOperand(node, def.getValue().asOperand(), def.getIndirectionIndex())
-      or
-      nodeHasInstruction(node, def.getValue().asInstruction(), def.getIndirectionIndex())
-    ) and
-    uncertain = false
-  }
-
-  private predicate nodeToDefOrUse(
-    Node node, SourceVariable sv, IRBlock bb, int i, boolean uncertain
-  ) {
-    exists(Def def |
-      def.hasIndexInBlock(bb, i, sv) and
-      defToNode(node, def, uncertain)
-    )
+  private predicate defToNode(Node node, Def def) {
+    nodeHasOperand(node, def.getValue().asOperand(), def.getIndirectionIndex())
     or
-    useToNode(bb, i, sv, node) and
-    uncertain = false
+    nodeHasInstruction(node, def.getValue().asInstruction(), def.getIndirectionIndex())
   }
 
-  private predicate useToNode(IRBlock bb, int i, SourceVariable sv, Node nodeTo) {
-    exists(PhiNode phi |
-      phi.definesAt(sv, bb, i, _) and
-      nodeTo = phi.getNode()
-    )
+  bindingset[result, v]
+  pragma[inline_late]
+  private DataFlowIntegrationImpl::Node fromDfNode(Node n, SourceVariable v) {
+    result = n.(SsaIteratorNode).getIteratorFlowNode()
     or
-    exists(Ssa::UseImpl use |
-      use.hasIndexInBlock(bb, i, sv) and
-      nodeTo = use.getNode()
+    exists(Ssa::UseImpl use, IRBlock bb, int i |
+      result.(DataFlowIntegrationImpl::ExprNode).getExpr().hasCfgNode(bb, i) and
+      use.hasIndexInBlock(bb, i, v) and
+      use.getNode() = n
     )
+    or
+    defToNode(n, result.(DataFlowIntegrationImpl::SsaDefinitionNode).getDefinition())
   }
 
   /**
    * Holds if `nodeFrom` flows to `nodeTo` in a single step.
    */
   predicate localFlowStep(Node nodeFrom, Node nodeTo) {
-    exists(
-      Node nFrom, SourceVariable sv, IRBlock bb1, int i1, IRBlock bb2, int i2, boolean uncertain
-    |
-      adjacentDefRead(bb1, i1, sv, bb2, i2) and
-      nodeToDefOrUse(nFrom, sv, bb1, i1, uncertain) and
-      useToNode(bb2, i2, sv, nodeTo)
-    |
-      if uncertain = true
-      then
-        nodeFrom =
-          [
-            nFrom,
-            getAPriorDefinition(any(IteratorSsa::DefinitionExt next | next.definesAt(sv, bb1, i1, _)))
-          ]
-      else nFrom = nodeFrom
+    exists(SourceVariable v |
+      nodeFrom != nodeTo and
+      DataFlowIntegrationImpl::localFlowStep(v, fromDfNode(nodeFrom, v), fromDfNode(nodeTo, v), _)
     )
   }
 }

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

@@ -1069,7 +1069,7 @@ module BarrierGuard<guardChecksNodeSig/3 guardChecksNode> {
 
 bindingset[result, v]
 pragma[inline_late]
-DataFlowIntegrationImpl::Node fromDfNode(Node n, SourceVariable v) {
+private DataFlowIntegrationImpl::Node fromDfNode(Node n, SourceVariable v) {
   result = n.(SsaSynthNode).getSynthNode()
   or
   exists(UseImpl use, IRBlock bb, int i |

cpp/ql/test/library-tests/calling-convention/calling-convention.expected

@@ -0,0 +1,7 @@
+| test.cpp:4:21:4:35 | definition of thiscall_method | thiscall |
+| test.cpp:7:14:7:23 | definition of func_cdecl | cdecl |
+| test.cpp:9:16:9:27 | definition of func_stdcall | stdcall |
+| test.cpp:11:17:11:29 | definition of func_fastcall | fastcall |
+| test.cpp:13:20:13:34 | definition of func_vectorcall | vectorcall |
+| test.cpp:15:13:15:25 | definition of func_overload | cdecl |
+| test.cpp:16:15:16:27 | definition of func_overload | stdcall |

cpp/ql/test/library-tests/calling-convention/calling-convention.ql

@@ -0,0 +1,5 @@
+import cpp
+
+from FunctionDeclarationEntry func, CallingConventionSpecifier ccs
+where ccs.hasName(func.getASpecifier())
+select func, func.getASpecifier()

cpp/ql/test/library-tests/calling-convention/test.cpp

@@ -0,0 +1,16 @@
+// semmle-extractor-options: --microsoft
+
+struct call_conventions {
+    void __thiscall thiscall_method() {}
+};
+
+void __cdecl func_cdecl() {}
+
+void __stdcall func_stdcall() {}
+
+void __fastcall func_fastcall() {}
+
+void __vectorcall  func_vectorcall() {}
+
+int __cdecl func_overload() {}
+int __stdcall func_overload(int x) {}

csharp/documentation/library-coverage/coverage.csv

@@ -42,5 +42,5 @@ MySql.Data.MySqlClient,48,,,,,,,,,,,,48,,,,,,,,,,
 Newtonsoft.Json,,,91,,,,,,,,,,,,,,,,,,,73,18
 ServiceStack,194,,7,27,,,,,75,,,,92,,,,,,,,,7,
 SourceGenerators,,,5,,,,,,,,,,,,,,,,,,,,5
-System,54,47,12241,,6,5,5,,,4,1,,33,2,,6,15,17,4,3,,5941,6300
+System,54,47,12255,,6,5,5,,,4,1,,33,2,,6,15,17,4,3,,5955,6300
 Windows.Security.Cryptography.Core,1,,,,,,,1,,,,,,,,,,,,,,,

csharp/documentation/library-coverage/coverage.rst

@@ -8,7 +8,7 @@ C# framework & library support
 
    Framework / library,Package,Flow sources,Taint & value steps,Sinks (total),`CWE-079` :sub:`Cross-site scripting`
    `ServiceStack <https://servicestack.net/>`_,"``ServiceStack.*``, ``ServiceStack``",,7,194,
-   System,"``System.*``, ``System``",47,12241,54,5
+   System,"``System.*``, ``System``",47,12255,54,5
    Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.AspNetCore.Components``, ``Microsoft.AspNetCore.Http``, ``Microsoft.AspNetCore.Mvc``, ``Microsoft.AspNetCore.WebUtilities``, ``Microsoft.CSharp``, ``Microsoft.Diagnostics.Tools.Pgo``, ``Microsoft.DotNet.Build.Tasks``, ``Microsoft.DotNet.PlatformAbstractions``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``Microsoft.Extensions.Diagnostics.Metrics``, ``Microsoft.Extensions.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.JSInterop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.VisualBasic``, ``Microsoft.Win32``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",60,2272,152,4
-   Totals,,107,14520,400,9
+   Totals,,107,14534,400,9