Merge branch 'main' into redsun82/rust-perf-measures

.devcontainer/devcontainer.json

@@ -1,4 +1,5 @@
 {
+  "image": "mcr.microsoft.com/devcontainers/base:ubuntu-24.04",
   "extensions": [
     "rust-lang.rust-analyzer",
     "bungcip.better-toml",

.github/workflows/compile-queries.yml

@@ -7,6 +7,11 @@ on:
       - "rc/*"
       - "codeql-cli-*"
   pull_request:
+    paths:
+      - '**.ql'
+      - '**.qll'
+      - '**/qlpack.yml'
+      - '**.dbscheme'
 
 permissions:
   contents: read

CODEOWNERS

@@ -42,3 +42,6 @@ MODULE.bazel @github/codeql-ci-reviewers
 # Misc
 /misc/scripts/accept-expected-changes-from-ci.py @RasmusWL
 /misc/scripts/generate-code-scanning-query-list.py @RasmusWL
+
+# .devcontainer
+/.devcontainer/ @github/codeql-ci-reviewers

Cargo.lock

@@ -384,6 +384,7 @@ dependencies = [
  "chrono",
  "clap",
  "codeql-extractor",
+ "dunce",
  "figment",
  "glob",
  "itertools 0.13.0",
@@ -542,6 +543,12 @@ version = "0.1.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "9bda8e21c04aca2ae33ffc2fd8c23134f3cac46db123ba97bd9d3f3b8a4a85e1"
 
+[[package]]
+name = "dunce"
+version = "1.0.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813"
+
 [[package]]
 name = "either"
 version = "1.13.0"

MODULE.bazel

@@ -68,7 +68,7 @@ use_repo(py_deps, "vendor__anyhow-1.0.44", "vendor__cc-1.0.70", "vendor__clap-2.
 # deps for ruby+rust
 # keep in sync by running `misc/bazel/3rdparty/update_cargo_deps.sh`
 tree_sitter_extractors_deps = use_extension("//misc/bazel/3rdparty:tree_sitter_extractors_extension.bzl", "r")
-use_repo(tree_sitter_extractors_deps, "vendor__anyhow-1.0.93", "vendor__argfile-0.2.1", "vendor__chrono-0.4.38", "vendor__clap-4.5.20", "vendor__encoding-0.2.33", "vendor__figment-0.10.19", "vendor__flate2-1.0.34", "vendor__glob-0.3.1", "vendor__globset-0.4.15", "vendor__itertools-0.10.5", "vendor__itertools-0.13.0", "vendor__lazy_static-1.5.0", "vendor__log-0.4.22", "vendor__num-traits-0.2.19", "vendor__num_cpus-1.16.0", "vendor__proc-macro2-1.0.89", "vendor__quote-1.0.37", "vendor__ra_ap_base_db-0.0.232", "vendor__ra_ap_cfg-0.0.232", "vendor__ra_ap_hir-0.0.232", "vendor__ra_ap_hir_def-0.0.232", "vendor__ra_ap_hir_expand-0.0.232", "vendor__ra_ap_ide_db-0.0.232", "vendor__ra_ap_intern-0.0.232", "vendor__ra_ap_load-cargo-0.0.232", "vendor__ra_ap_parser-0.0.232", "vendor__ra_ap_paths-0.0.232", "vendor__ra_ap_project_model-0.0.232", "vendor__ra_ap_span-0.0.232", "vendor__ra_ap_syntax-0.0.232", "vendor__ra_ap_vfs-0.0.232", "vendor__rand-0.8.5", "vendor__rayon-1.10.0", "vendor__regex-1.11.1", "vendor__serde-1.0.214", "vendor__serde_json-1.0.133", "vendor__serde_with-3.11.0", "vendor__stderrlog-0.6.0", "vendor__syn-2.0.87", "vendor__tracing-0.1.40", "vendor__tracing-subscriber-0.3.18", "vendor__tree-sitter-0.24.4", "vendor__tree-sitter-embedded-template-0.23.2", "vendor__tree-sitter-json-0.24.8", "vendor__tree-sitter-ql-0.23.1", "vendor__tree-sitter-ruby-0.23.1", "vendor__triomphe-0.1.14", "vendor__ungrammar-1.16.1")
+use_repo(tree_sitter_extractors_deps, "vendor__anyhow-1.0.93", "vendor__argfile-0.2.1", "vendor__chrono-0.4.38", "vendor__clap-4.5.20", "vendor__dunce-1.0.5", "vendor__encoding-0.2.33", "vendor__figment-0.10.19", "vendor__flate2-1.0.34", "vendor__glob-0.3.1", "vendor__globset-0.4.15", "vendor__itertools-0.10.5", "vendor__itertools-0.13.0", "vendor__lazy_static-1.5.0", "vendor__log-0.4.22", "vendor__num-traits-0.2.19", "vendor__num_cpus-1.16.0", "vendor__proc-macro2-1.0.89", "vendor__quote-1.0.37", "vendor__ra_ap_base_db-0.0.232", "vendor__ra_ap_cfg-0.0.232", "vendor__ra_ap_hir-0.0.232", "vendor__ra_ap_hir_def-0.0.232", "vendor__ra_ap_hir_expand-0.0.232", "vendor__ra_ap_ide_db-0.0.232", "vendor__ra_ap_intern-0.0.232", "vendor__ra_ap_load-cargo-0.0.232", "vendor__ra_ap_parser-0.0.232", "vendor__ra_ap_paths-0.0.232", "vendor__ra_ap_project_model-0.0.232", "vendor__ra_ap_span-0.0.232", "vendor__ra_ap_syntax-0.0.232", "vendor__ra_ap_vfs-0.0.232", "vendor__rand-0.8.5", "vendor__rayon-1.10.0", "vendor__regex-1.11.1", "vendor__serde-1.0.214", "vendor__serde_json-1.0.133", "vendor__serde_with-3.11.0", "vendor__stderrlog-0.6.0", "vendor__syn-2.0.87", "vendor__tracing-0.1.40", "vendor__tracing-subscriber-0.3.18", "vendor__tree-sitter-0.24.4", "vendor__tree-sitter-embedded-template-0.23.2", "vendor__tree-sitter-json-0.24.8", "vendor__tree-sitter-ql-0.23.1", "vendor__tree-sitter-ruby-0.23.1", "vendor__triomphe-0.1.14", "vendor__ungrammar-1.16.1")
 
 dotnet = use_extension("@rules_dotnet//dotnet:extensions.bzl", "dotnet")
 dotnet.toolchain(dotnet_version = "9.0.100")

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

@@ -545,7 +545,7 @@ module ProductFlow {
     private predicate outImpl1(Flow1::PathNode pred1, Flow1::PathNode succ1, DataFlowCall call) {
       Flow1::PathGraph::edges(pred1, succ1, _, _) and
       exists(ReturnKindExt returnKind |
-        succ1.getNode() = returnKind.getAnOutNode(call) and
+        succ1.getNode() = getAnOutNodeExt(call, returnKind) and
         returnKind = getParamReturnPosition(_, pred1.asParameterReturnNode()).getKind()
       )
     }
@@ -573,7 +573,7 @@ module ProductFlow {
     private predicate outImpl2(Flow2::PathNode pred2, Flow2::PathNode succ2, DataFlowCall call) {
       Flow2::PathGraph::edges(pred2, succ2, _, _) and
       exists(ReturnKindExt returnKind |
-        succ2.getNode() = returnKind.getAnOutNode(call) and
+        succ2.getNode() = getAnOutNodeExt(call, returnKind) and
         returnKind = getParamReturnPosition(_, pred2.asParameterReturnNode()).getKind()
       )
     }

csharp/.vscode/launch.json

@@ -61,5 +61,21 @@
             ],
             "env": {}
         },
+        {
+            "name": "C#: Tracing Debug",
+            "type": "coreclr",
+            "request": "launch",
+            "preLaunchTask": "dotnet: build",
+            "program": "${workspaceFolder}/extractor/Semmle.Extraction.CSharp.Driver/bin/Debug/net9.0/Semmle.Extraction.CSharp.Driver.dll",
+            // Set the path to the folder that should be extracted:
+            "cwd": "${workspaceFolder}/ql/test/library-tests/dataflow/local",
+            "args": [
+                "LocalDataFlow.cs"
+            ],
+            "env": {},
+            "stopAtEntry": true,
+            "justMyCode": false,
+            "suppressJITOptimizations": true
+        },
     ]
 }

csharp/documentation/library-coverage/coverage.csv

@@ -13,6 +13,8 @@ JsonToItemsTaskFactory,,,11,,,,,,,,,,,,,,,,,,,1,10
 Microsoft.Android.Build,,1,14,,,,,,,,,,,,,1,,,,,,12,2
 Microsoft.Apple.Build,,,7,,,,,,,,,,,,,,,,,,,7,
 Microsoft.ApplicationBlocks.Data,28,,,,,,,,,,,,28,,,,,,,,,,
+Microsoft.AspNetCore.Components,,2,1,,,,,,,,,,,,,,,,2,,,1,
+Microsoft.AspNetCore.WebUtilities,,,2,,,,,,,,,,,,,,,,,,,2,
 Microsoft.CSharp,,,2,,,,,,,,,,,,,,,,,,,2,
 Microsoft.Diagnostics.Tools.Pgo,,,25,,,,,,,,,,,,,,,,,,,2,23
 Microsoft.DotNet.Build.Tasks,,,10,,,,,,,,,,,,,,,,,,,8,2
@@ -44,5 +46,5 @@ MySql.Data.MySqlClient,48,,,,,,,,,,,,48,,,,,,,,,,
 Newtonsoft.Json,,,91,,,,,,,,,,,,,,,,,,,73,18
 ServiceStack,194,,7,27,,,,,75,,,,92,,,,,,,,,7,
 SourceGenerators,,,5,,,,,,,,,,,,,,,,,,,,5
-System,54,47,10818,,6,5,5,,,4,1,,33,2,,6,15,17,4,3,,5511,5307
+System,54,47,10819,,6,5,5,,,4,1,,33,2,,6,15,17,4,3,,5512,5307
 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,10818,54,5
+   System,"``System.*``, ``System``",47,10819,54,5
-   Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``JsonToItemsTaskFactory``, ``Microsoft.Android.Build``, ``Microsoft.Apple.Build``, ``Microsoft.ApplicationBlocks.Data``, ``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.NET.Sdk.WebAssembly``, ``Microsoft.NET.WebAssembly.Webcil``, ``Microsoft.VisualBasic``, ``Microsoft.WebAssembly.Build.Tasks``, ``Microsoft.Win32``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",57,2068,150,2
+   Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``JsonToItemsTaskFactory``, ``Microsoft.Android.Build``, ``Microsoft.Apple.Build``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.AspNetCore.Components``, ``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.NET.Sdk.WebAssembly``, ``Microsoft.NET.WebAssembly.Webcil``, ``Microsoft.VisualBasic``, ``Microsoft.WebAssembly.Build.Tasks``, ``Microsoft.Win32``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",59,2071,150,2
-   Totals,,104,12893,398,7
+   Totals,,106,12897,398,7
 

csharp/ql/lib/change-notes/2024-11-27-navigationmanager.uri-and-uri-parsing-utilities.md

@@ -0,0 +1,8 @@
+---
+category: minorAnalysis
+---
+* Added `Microsoft.AspNetCore.Components.NagivationManager::Uri` as a remote flow source, since this value may contain user-specified values.
+* Added the following URI-parsing methods as summaries, as they may be tainted with user-specified values:
+  - `System.Web.HttpUtility::ParseQueryString`
+  - `Microsoft.AspNetCore.WebUtilities.QueryHelpers::ParseQuery`
+  - `Microsoft.AspNetCore.WebUtilities.QueryHelpers::ParseNullableQuery`

csharp/ql/lib/ext/Microsoft.AspNetCore.Components.model.yml

@@ -0,0 +1,12 @@
+extensions:
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: sourceModel
+    data:
+      - ["Microsoft.AspNetCore.Components", "NagivationManager", True, "get_BaseUri", "", "", "ReturnValue", "remote", "manual"]
+      - ["Microsoft.AspNetCore.Components", "NagivationManager", True, "get_Uri", "", "", "ReturnValue", "remote", "manual"]
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: summaryModel
+    data:
+      - ["Microsoft.AspNetCore.Components", "NagivationManager", True, "ToAbsoluteUri", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]

csharp/ql/lib/ext/Microsoft.AspNetCore.WebUtilities.model.yml

@@ -0,0 +1,7 @@
+extensions:
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: summaryModel
+    data:
+      - ["Microsoft.AspNetCore.WebUtilities", "QueryHelpers", False, "ParseQuery", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
+      - ["Microsoft.AspNetCore.WebUtilities", "QueryHelpers", False, "ParseNullableQuery", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]

csharp/ql/lib/ext/System.Web.model.yml

@@ -22,6 +22,7 @@ extensions:
       - ["System.Web", "HttpUtility", False, "HtmlEncode", "(System.String,System.IO.TextWriter)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["System.Web", "HttpUtility", False, "JavaScriptStringEncode", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["System.Web", "HttpUtility", False, "JavaScriptStringEncode", "(System.String,System.Boolean)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
+      - ["System.Web", "HttpUtility", False, "ParseQueryString", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["System.Web", "HttpUtility", False, "UrlEncode", "(System.Byte[])", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["System.Web", "HttpUtility", False, "UrlEncode", "(System.Byte[],System.Int32,System.Int32)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["System.Web", "HttpUtility", False, "UrlEncode", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]

csharp/ql/lib/ext/generated/System.Web.model.yml

@@ -25,8 +25,6 @@ extensions:
       - ["System.Web", "AspNetHostingPermissionAttribute", "AspNetHostingPermissionAttribute", "(System.Security.Permissions.SecurityAction)", "summary", "df-generated"]
       - ["System.Web", "AspNetHostingPermissionAttribute", "CreatePermission", "()", "summary", "df-generated"]
       - ["System.Web", "HttpUtility", "HtmlDecode", "(System.String,System.IO.TextWriter)", "summary", "df-generated"]
-      - ["System.Web", "HttpUtility", "ParseQueryString", "(System.String)", "summary", "df-generated"]
-      - ["System.Web", "HttpUtility", "ParseQueryString", "(System.String,System.Text.Encoding)", "summary", "df-generated"]
       - ["System.Web", "HttpUtility", "UrlDecode", "(System.Byte[],System.Int32,System.Int32,System.Text.Encoding)", "summary", "df-generated"]
       - ["System.Web", "HttpUtility", "UrlDecode", "(System.Byte[],System.Text.Encoding)", "summary", "df-generated"]
       - ["System.Web", "HttpUtility", "UrlDecode", "(System.String)", "summary", "df-generated"]

csharp/ql/src/Telemetry/DatabaseQuality.qll

@@ -68,15 +68,25 @@ module CallTargetStats implements StatsSig {
     )
   }
 
-  private predicate isInitializedWithCollectionInitializer(PropertyCall c) {
+  private predicate isInitializedWithObjectOrCollectionInitializer(PropertyCall c) {
     exists(Property p, AssignExpr assign |
       p = c.getProperty() and
       assign = c.getParent() and
       assign.getLValue() = c and
-      assign.getRValue() instanceof CollectionInitializer
+      assign.getRValue() instanceof ObjectOrCollectionInitializer
     )
   }
 
+  private predicate isEventFieldAccess(EventCall c) {
+    exists(Event e | c.getEvent() = e |
+      forall(Accessor a | e.getAnAccessor() = a | a.isCompilerGenerated())
+    )
+  }
+
+  private predicate isTypeParameterInstantiation(ObjectCreation e) {
+    e.getType() instanceof TypeParameter
+  }
+
   additional predicate isNotOkCall(Call c) {
     not exists(c.getTarget()) and
     not c instanceof DelegateCall and
@@ -84,8 +94,10 @@ module CallTargetStats implements StatsSig {
     not isNoSetterPropertyCallInConstructor(c) and
     not isNoSetterPropertyInitialization(c) and
     not isAnonymousObjectMemberDeclaration(c) and
-    not isInitializedWithCollectionInitializer(c) and
+    not isInitializedWithObjectOrCollectionInitializer(c) and
-    not c.getParent+() instanceof NameOfExpr
+    not c.getParent+() instanceof NameOfExpr and
+    not isEventFieldAccess(c) and
+    not isTypeParameterInstantiation(c)
   }
 
   int getNumberOfNotOk() { result = count(Call c | isNotOkCall(c)) }

csharp/ql/test/library-tests/dataflow/library/FlowSummaries.expected

@@ -1699,6 +1699,8 @@ summary
 | Microsoft.AspNetCore.WebUtilities;HttpResponseStreamWriter;WriteLineAsync;(System.String);Argument[0];ReturnValue;taint;df-generated |
 | Microsoft.AspNetCore.WebUtilities;HttpResponseStreamWriter;WriteLineAsync;(System.String);Argument[this];ReturnValue;taint;df-generated |
 | Microsoft.AspNetCore.WebUtilities;HttpResponseStreamWriter;get_Encoding;();Argument[this];ReturnValue;taint;df-generated |
+| Microsoft.AspNetCore.WebUtilities;QueryHelpers;ParseNullableQuery;(System.String);Argument[0];ReturnValue;taint;manual |
+| Microsoft.AspNetCore.WebUtilities;QueryHelpers;ParseQuery;(System.String);Argument[0];ReturnValue;taint;manual |
 | Microsoft.AspNetCore;WebHost;Start;(Microsoft.AspNetCore.Http.RequestDelegate);Argument[0];Argument[0].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore;WebHost;Start;(System.Action<Microsoft.AspNetCore.Routing.IRouteBuilder>);Argument[0];Argument[0].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore;WebHost;Start;(System.String,Microsoft.AspNetCore.Http.RequestDelegate);Argument[1];Argument[1].Parameter[delegate-self];value;hq-generated |
@@ -18321,6 +18323,8 @@ summary
 | System.Web;HttpUtility;HtmlEncode;(System.String,System.IO.TextWriter);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;JavaScriptStringEncode;(System.String);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;JavaScriptStringEncode;(System.String,System.Boolean);Argument[0];ReturnValue;taint;manual |
+| System.Web;HttpUtility;ParseQueryString;(System.String);Argument[0];ReturnValue;taint;manual |
+| System.Web;HttpUtility;ParseQueryString;(System.String,System.Text.Encoding);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;UrlEncode;(System.Byte[]);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;UrlEncode;(System.Byte[],System.Int32,System.Int32);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;UrlEncode;(System.String);Argument[0];ReturnValue;taint;manual |
@@ -51126,8 +51130,6 @@ neutral
 | System.Web;AspNetHostingPermissionAttribute;CreatePermission;();summary;df-generated |
 | System.Web;HtmlString;ToHtmlString;();summary;df-generated |
 | System.Web;HttpUtility;HtmlDecode;(System.String,System.IO.TextWriter);summary;df-generated |
-| System.Web;HttpUtility;ParseQueryString;(System.String);summary;df-generated |
-| System.Web;HttpUtility;ParseQueryString;(System.String,System.Text.Encoding);summary;df-generated |
 | System.Web;HttpUtility;UrlDecode;(System.Byte[],System.Int32,System.Int32,System.Text.Encoding);summary;df-generated |
 | System.Web;HttpUtility;UrlDecode;(System.Byte[],System.Text.Encoding);summary;df-generated |
 | System.Web;HttpUtility;UrlDecode;(System.String);summary;df-generated |

csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.expected

@@ -650,6 +650,8 @@
 | Microsoft.AspNetCore.WebUtilities;FileBufferingReadStream;FileBufferingReadStream;(System.IO.Stream,System.Int32,System.Nullable<System.Int64>,System.Func<System.String>);Argument[3];Argument[3].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore.WebUtilities;FileBufferingReadStream;FileBufferingReadStream;(System.IO.Stream,System.Int32,System.Nullable<System.Int64>,System.Func<System.String>,System.Buffers.ArrayPool<System.Byte>);Argument[3];Argument[3].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore.WebUtilities;FileBufferingWriteStream;FileBufferingWriteStream;(System.Int32,System.Nullable<System.Int64>,System.Func<System.String>);Argument[2];Argument[2].Parameter[delegate-self];value;hq-generated |
+| Microsoft.AspNetCore.WebUtilities;QueryHelpers;ParseNullableQuery;(System.String);Argument[0];ReturnValue;taint;manual |
+| Microsoft.AspNetCore.WebUtilities;QueryHelpers;ParseQuery;(System.String);Argument[0];ReturnValue;taint;manual |
 | Microsoft.AspNetCore;WebHost;Start;(Microsoft.AspNetCore.Http.RequestDelegate);Argument[0];Argument[0].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore;WebHost;Start;(System.Action<Microsoft.AspNetCore.Routing.IRouteBuilder>);Argument[0];Argument[0].Parameter[delegate-self];value;hq-generated |
 | Microsoft.AspNetCore;WebHost;Start;(System.String,Microsoft.AspNetCore.Http.RequestDelegate);Argument[1];Argument[1].Parameter[delegate-self];value;hq-generated |
@@ -13988,6 +13990,8 @@
 | System.Web;HttpUtility;HtmlEncode;(System.String,System.IO.TextWriter);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;JavaScriptStringEncode;(System.String);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;JavaScriptStringEncode;(System.String,System.Boolean);Argument[0];ReturnValue;taint;manual |
+| System.Web;HttpUtility;ParseQueryString;(System.String);Argument[0];ReturnValue;taint;manual |
+| System.Web;HttpUtility;ParseQueryString;(System.String,System.Text.Encoding);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;UrlEncode;(System.Byte[]);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;UrlEncode;(System.Byte[],System.Int32,System.Int32);Argument[0];ReturnValue;taint;manual |
 | System.Web;HttpUtility;UrlEncode;(System.String);Argument[0];ReturnValue;taint;manual |

csharp/ql/test/query-tests/Telemetry/DatabaseQuality/IsNotOkayCall.expected

@@ -0,0 +1,4 @@
+| Quality.cs:26:19:26:26 | access to indexer | Call without target $@. | Quality.cs:26:19:26:26 | access to indexer | access to indexer |
+| Quality.cs:29:21:29:27 | access to indexer | Call without target $@. | Quality.cs:29:21:29:27 | access to indexer | access to indexer |
+| Quality.cs:32:9:32:21 | access to indexer | Call without target $@. | Quality.cs:32:9:32:21 | access to indexer | access to indexer |
+| Quality.cs:34:21:34:25 | object creation of type null | Call without target $@. | Quality.cs:34:21:34:25 | object creation of type null | object creation of type null |

csharp/ql/test/query-tests/Telemetry/DatabaseQuality/NoTarget.expected

@@ -4,4 +4,12 @@
 | Quality.cs:15:24:15:34 | access to property MyProperty3 | Call without target $@. | Quality.cs:15:24:15:34 | access to property MyProperty3 | access to property MyProperty3 |
 | Quality.cs:15:24:15:46 | access to property MyProperty2 | Call without target $@. | Quality.cs:15:24:15:46 | access to property MyProperty2 | access to property MyProperty2 |
 | Quality.cs:19:13:19:23 | access to property MyProperty4 | Call without target $@. | Quality.cs:19:13:19:23 | access to property MyProperty4 | access to property MyProperty4 |
-| Quality.cs:24:16:24:26 | access to property MyProperty2 | Call without target $@. | Quality.cs:24:16:24:26 | access to property MyProperty2 | access to property MyProperty2 |
+| Quality.cs:20:13:20:23 | access to property MyProperty6 | Call without target $@. | Quality.cs:20:13:20:23 | access to property MyProperty6 | access to property MyProperty6 |
+| Quality.cs:23:9:23:14 | access to event Event1 | Call without target $@. | Quality.cs:23:9:23:14 | access to event Event1 | access to event Event1 |
+| Quality.cs:23:9:23:30 | delegate call | Call without target $@. | Quality.cs:23:9:23:30 | delegate call | delegate call |
+| Quality.cs:26:19:26:26 | access to indexer | Call without target $@. | Quality.cs:26:19:26:26 | access to indexer | access to indexer |
+| Quality.cs:29:21:29:27 | access to indexer | Call without target $@. | Quality.cs:29:21:29:27 | access to indexer | access to indexer |
+| Quality.cs:32:9:32:21 | access to indexer | Call without target $@. | Quality.cs:32:9:32:21 | access to indexer | access to indexer |
+| Quality.cs:34:21:34:25 | object creation of type null | Call without target $@. | Quality.cs:34:21:34:25 | object creation of type null | object creation of type null |
+| Quality.cs:38:16:38:26 | access to property MyProperty2 | Call without target $@. | Quality.cs:38:16:38:26 | access to property MyProperty2 | access to property MyProperty2 |
+| Quality.cs:50:20:50:26 | object creation of type T | Call without target $@. | Quality.cs:50:20:50:26 | object creation of type T | object creation of type T |

csharp/ql/test/query-tests/Telemetry/DatabaseQuality/Quality.cs

@@ -16,8 +16,22 @@ public class Test
 
         new Test()
         {
-            MyProperty4 = { 1, 2, 3 }
+            MyProperty4 = { 1, 2, 3 },
+            MyProperty6 = { [1] = "" }
         };
+
+        Event1.Invoke(this, 5);
+
+        var str = "abcd";
+        var sub = str[..3];         // TODO: this is not an indexer call, but rather a `str.Substring(0, 3)` call.
+
+        Span<int> sp = null;
+        var slice = sp[..3];        // TODO: this is not an indexer call, but rather a `sp.Slice(0, 3)` call.
+
+        Span<byte> guidBytes = stackalloc byte[16];
+        guidBytes[08] = 1;          // TODO: this indexer call has no target, because the target is a `ref` returning getter.
+
+        new MyList([new(), new Test()]);        // TODO: the `new()` call has no target, which is unexpected, as we know at compile time, that this is a `new Test()` call.
     }
 
     public int MyProperty1 { get; }
@@ -25,4 +39,20 @@ public class Test
     public Test MyProperty3 { get; set; }
     public List<int> MyProperty4 { get; }
     static int MyProperty5 { get; }
+    public Dictionary<int, string> MyProperty6 { get; }
+
+    public event EventHandler<int> Event1;
+
+    class Gen<T> where T : new()
+    {
+        public static T Factory()
+        {
+            return new T();
+        }
+    }
+
+    class MyList
+    {
+        public MyList(IEnumerable<Test> init) { }
+    }
 }

misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.bazel

@@ -55,6 +55,12 @@ alias(
     tags = ["manual"],
 )
 
+alias(
+    name = "dunce",
+    actual = "@vendor__dunce-1.0.5//:dunce",
+    tags = ["manual"],
+)
+
 alias(
     name = "encoding",
     actual = "@vendor__encoding-0.2.33//:encoding",

misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.dunce-1.0.5.bazel

@@ -0,0 +1,81 @@
+###############################################################################
+# @generated
+# DO NOT MODIFY: This file is auto-generated by a crate_universe tool. To
+# regenerate this file, run the following:
+#
+#     bazel run @@//misc/bazel/3rdparty:vendor_tree_sitter_extractors
+###############################################################################
+
+load("@rules_rust//rust:defs.bzl", "rust_library")
+
+package(default_visibility = ["//visibility:public"])
+
+rust_library(
+    name = "dunce",
+    srcs = glob(
+        include = ["**/*.rs"],
+        allow_empty = True,
+    ),
+    compile_data = glob(
+        include = ["**"],
+        allow_empty = True,
+        exclude = [
+            "**/* *",
+            ".tmp_git_root/**/*",
+            "BUILD",
+            "BUILD.bazel",
+            "WORKSPACE",
+            "WORKSPACE.bazel",
+        ],
+    ),
+    crate_root = "src/lib.rs",
+    edition = "2021",
+    rustc_flags = [
+        "--cap-lints=allow",
+    ],
+    tags = [
+        "cargo-bazel",
+        "crate-name=dunce",
+        "manual",
+        "noclippy",
+        "norustfmt",
+    ],
+    target_compatible_with = select({
+        "@rules_rust//rust/platform:aarch64-apple-darwin": [],
+        "@rules_rust//rust/platform:aarch64-apple-ios": [],
+        "@rules_rust//rust/platform:aarch64-apple-ios-sim": [],
+        "@rules_rust//rust/platform:aarch64-fuchsia": [],
+        "@rules_rust//rust/platform:aarch64-linux-android": [],
+        "@rules_rust//rust/platform:aarch64-pc-windows-msvc": [],
+        "@rules_rust//rust/platform:aarch64-unknown-linux-gnu": [],
+        "@rules_rust//rust/platform:aarch64-unknown-nixos-gnu": [],
+        "@rules_rust//rust/platform:aarch64-unknown-nto-qnx710": [],
+        "@rules_rust//rust/platform:arm-unknown-linux-gnueabi": [],
+        "@rules_rust//rust/platform:armv7-linux-androideabi": [],
+        "@rules_rust//rust/platform:armv7-unknown-linux-gnueabi": [],
+        "@rules_rust//rust/platform:i686-apple-darwin": [],
+        "@rules_rust//rust/platform:i686-linux-android": [],
+        "@rules_rust//rust/platform:i686-pc-windows-msvc": [],
+        "@rules_rust//rust/platform:i686-unknown-freebsd": [],
+        "@rules_rust//rust/platform:i686-unknown-linux-gnu": [],
+        "@rules_rust//rust/platform:powerpc-unknown-linux-gnu": [],
+        "@rules_rust//rust/platform:riscv32imc-unknown-none-elf": [],
+        "@rules_rust//rust/platform:riscv64gc-unknown-none-elf": [],
+        "@rules_rust//rust/platform:s390x-unknown-linux-gnu": [],
+        "@rules_rust//rust/platform:thumbv7em-none-eabi": [],
+        "@rules_rust//rust/platform:thumbv8m.main-none-eabi": [],
+        "@rules_rust//rust/platform:wasm32-unknown-unknown": [],
+        "@rules_rust//rust/platform:wasm32-wasi": [],
+        "@rules_rust//rust/platform:x86_64-apple-darwin": [],
+        "@rules_rust//rust/platform:x86_64-apple-ios": [],
+        "@rules_rust//rust/platform:x86_64-fuchsia": [],
+        "@rules_rust//rust/platform:x86_64-linux-android": [],
+        "@rules_rust//rust/platform:x86_64-pc-windows-msvc": [],
+        "@rules_rust//rust/platform:x86_64-unknown-freebsd": [],
+        "@rules_rust//rust/platform:x86_64-unknown-linux-gnu": [],
+        "@rules_rust//rust/platform:x86_64-unknown-nixos-gnu": [],
+        "@rules_rust//rust/platform:x86_64-unknown-none": [],
+        "//conditions:default": ["@platforms//:incompatible"],
+    }),
+    version = "1.0.5",
+)

misc/bazel/3rdparty/tree_sitter_extractors_deps/defs.bzl

@@ -322,6 +322,7 @@ _NORMAL_DEPENDENCIES = {
             "argfile": Label("@vendor__argfile-0.2.1//:argfile"),
             "chrono": Label("@vendor__chrono-0.4.38//:chrono"),
             "clap": Label("@vendor__clap-4.5.20//:clap"),
+            "dunce": Label("@vendor__dunce-1.0.5//:dunce"),
             "figment": Label("@vendor__figment-0.10.19//:figment"),
             "glob": Label("@vendor__glob-0.3.1//:glob"),
             "itertools": Label("@vendor__itertools-0.13.0//:itertools"),
@@ -1119,6 +1120,16 @@ def crate_repositories():
         build_file = Label("//misc/bazel/3rdparty/tree_sitter_extractors_deps:BUILD.drop_bomb-0.1.5.bazel"),
     )
 
+    maybe(
+        http_archive,
+        name = "vendor__dunce-1.0.5",
+        sha256 = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813",
+        type = "tar.gz",
+        urls = ["https://static.crates.io/crates/dunce/1.0.5/download"],
+        strip_prefix = "dunce-1.0.5",
+        build_file = Label("//misc/bazel/3rdparty/tree_sitter_extractors_deps:BUILD.dunce-1.0.5.bazel"),
+    )
+
     maybe(
         http_archive,
         name = "vendor__either-1.13.0",
@@ -3313,6 +3324,7 @@ def crate_repositories():
         struct(repo = "vendor__argfile-0.2.1", is_dev_dep = False),
         struct(repo = "vendor__chrono-0.4.38", is_dev_dep = False),
         struct(repo = "vendor__clap-4.5.20", is_dev_dep = False),
+        struct(repo = "vendor__dunce-1.0.5", is_dev_dep = False),
         struct(repo = "vendor__encoding-0.2.33", is_dev_dep = False),
         struct(repo = "vendor__figment-0.10.19", is_dev_dep = False),
         struct(repo = "vendor__flate2-1.0.34", is_dev_dep = False),

ql/ql/src/queries/style/ValidatePredicateGetReturns.ql

@@ -0,0 +1,37 @@
+/**
+ * @name Predicates starting with "get" or "as" should return a value
+ * @description Checks if predicates that start with "get" or "as" actually return a value.
+ * @kind problem
+ * @problem.severity warning
+ * @id ql/predicates-get-should-return-value
+ * @tags correctness
+ *       maintainability
+ * @precision high
+ */
+
+import ql
+import codeql_ql.ast.Ast
+
+/**
+ * Identifies predicates whose names start with "get", "as" followed by an uppercase letter.
+ * This ensures that only predicates like "getValue" are matched, excluding names like "getter".
+ */
+predicate isGetPredicate(Predicate pred, string prefix) {
+  prefix = pred.getName().regexpCapture("(get|as)[A-Z].*", 1)
+}
+
+/**
+ * Checks if a predicate has a return type. This is phrased negatively to not flag unresolved aliases.
+ */
+predicate hasNoReturnType(Predicate pred) {
+  not exists(pred.getReturnTypeExpr()) and
+  not pred.(ClasslessPredicate).getAlias() instanceof PredicateExpr
+  or
+  hasNoReturnType(pred.(ClasslessPredicate).getAlias().(PredicateExpr).getResolvedPredicate())
+}
+
+from Predicate pred, string prefix
+where
+  isGetPredicate(pred, prefix) and
+  hasNoReturnType(pred)
+select pred, "This predicate starts with '" + prefix + "' but does not return a value."

ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.expected

@@ -0,0 +1,6 @@
+| test.qll:4:11:4:18 | ClasslessPredicate getValue | This predicate starts with 'get' but does not return a value. |
+| test.qll:25:11:25:28 | ClasslessPredicate getImplementation2 | This predicate starts with 'get' but does not return a value. |
+| test.qll:28:11:28:19 | ClasslessPredicate getAlias2 | This predicate starts with 'get' but does not return a value. |
+| test.qll:31:11:31:17 | ClasslessPredicate asValue | This predicate starts with 'as' but does not return a value. |
+| test.qll:48:11:48:19 | ClasslessPredicate getAlias4 | This predicate starts with 'get' but does not return a value. |
+| test.qll:61:11:61:22 | ClasslessPredicate getDistance2 | This predicate starts with 'get' but does not return a value. |

ql/ql/test/queries/style/ValidatePredicateGetReturns/ValidatePredicateGetReturns.qlref

@@ -0,0 +1 @@
+queries/style/ValidatePredicateGetReturns.ql

ql/ql/test/queries/style/ValidatePredicateGetReturns/test.qll

@@ -0,0 +1,67 @@
+import ql
+
+// NOT OK -- Predicate starts with "get" but does not return a value
+predicate getValue() { none() }
+
+// OK -- starts with get and returns a value
+string getData() { result = "data" }
+
+// OK -- starts with get but followed by a lowercase letter, probably should be ignored
+predicate getterFunction() { none() }
+
+// OK -- starts with get and returns a value
+string getImplementation() { result = "implementation" }
+
+// OK -- is an alias
+predicate getAlias = getImplementation/0;
+
+// OK -- Starts with "get" but followed by a lowercase letter, probably be ignored
+predicate getvalue() { none() }
+
+// OK -- Does not start with "get", should be ignored
+predicate retrieveValue() { none() }
+
+// NOT OK -- starts with get and does not return value
+predicate getImplementation2() { none() }
+
+// NOT OK -- is an alias for a predicate which does not have a return value
+predicate getAlias2 = getImplementation2/0;
+
+// NOT OK -- starts with as and does not return value
+predicate asValue() { none() }
+
+// OK -- starts with as but followed by a lowercase letter, probably should be ignored
+predicate assessment() { none() }
+
+// OK -- starts with as and returns a value
+string asString() { result = "string" }
+
+// OK -- starts with get and returns a value
+HiddenType getInjectableCompositeActionNode() {
+  exists(HiddenType hidden | result = hidden.toString())
+}
+
+// OK
+predicate implementation4() { none() }
+
+// NOT OK -- is an alias
+predicate getAlias4 = implementation4/0;
+
+// OK -- is an alias
+predicate alias5 = implementation4/0;
+
+int root() { none() }
+
+predicate edge(int x, int y) { none() }
+
+// OK -- Higher-order predicate
+int getDistance(int x) = shortestDistances(root/0, edge/2)(_, x, result)
+
+// NOT OK -- Higher-order predicate that does not return a value even though has 'get' in the name
+predicate getDistance2(int x, int y) = shortestDistances(root/0, edge/2)(_, x, y)
+
+// OK
+predicate unresolvedAlias = unresolved/0;
+
+// OK -- unresolved alias
+predicate getUnresolvedAlias = unresolved/0;

rust/extractor/Cargo.toml

@@ -35,3 +35,4 @@ itertools = "0.13.0"
 glob = "0.3.1"
 chrono = { version = "0.4.38", features = ["serde"] }
 serde_json = "1.0.133"
+dunce = "1.0.5"

rust/extractor/src/rust_analyzer.rs

@@ -17,6 +17,7 @@ use ra_ap_vfs::Vfs;
 use ra_ap_vfs::VfsPath;
 use ra_ap_vfs::{AbsPathBuf, FileId};
 use std::borrow::Cow;
+use std::iter;
 use std::path::{Path, PathBuf};
 use triomphe::Arc;
 
@@ -188,10 +189,28 @@ fn from_utf8_lossy(v: &[u8]) -> (Cow<'_, str>, Option<SyntaxError>) {
     (Cow::Owned(res), Some(error))
 }
 
-pub(crate) fn path_to_file_id(path: &Path, vfs: &Vfs) -> Option<FileId> {
+fn canonicalize_if_on_windows(path: &Path) -> Option<PathBuf> {
-    Utf8PathBuf::from_path_buf(path.to_path_buf())
+    if cfg!(windows) {
-        .ok()
+        dunce::canonicalize(path).ok()
-        .and_then(|x| AbsPathBuf::try_from(x).ok())
+    } else {
-        .map(VfsPath::from)
+        None
-        .and_then(|x| vfs.file_id(&x))
+    }
+}
+
+pub(crate) fn path_to_file_id(path: &Path, vfs: &Vfs) -> Option<FileId> {
+    // There seems to be some flaky inconsistencies around paths on Windows, where sometimes paths
+    // are registered in `vfs` without the `//?/` long path prefix. Then it happens that paths with
+    // that prefix are not found. To work around that, on Windows after failing to find `path` as
+    // is, we then try to canonicalize it using dunce. Dunce will be able to losslessly convert a
+    // `//?/` path into its equivalent one in `vfs` without the prefix, if there is one.
+    iter::once(path.to_path_buf())
+        .chain(canonicalize_if_on_windows(path))
+        .filter_map(|p| {
+            Utf8PathBuf::from_path_buf(p)
+                .ok()
+                .and_then(|x| AbsPathBuf::try_from(x).ok())
+                .map(VfsPath::from)
+                .and_then(|x| vfs.file_id(&x))
+        })
+        .next()
 }

rust/ql/consistency-queries/DataFlowConsistency.ql

@@ -5,12 +5,4 @@
  * @id rust/diagnostics/data-flow-consistency
  */
 
-import codeql.rust.dataflow.DataFlow::DataFlow as DataFlow
+import codeql.rust.dataflow.internal.DataFlowConsistency
-private import rust
-private import codeql.rust.dataflow.internal.DataFlowImpl
-private import codeql.rust.dataflow.internal.TaintTrackingImpl
-private import codeql.dataflow.internal.DataFlowImplConsistency
-
-private module Input implements InputSig<Location, RustDataFlow> { }
-
-import MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>

rust/ql/lib/codeql/rust/AstConsistency.qll

@@ -21,6 +21,11 @@ query predicate multipleToStrings(Element e, string cls, string s) {
  */
 query predicate multipleLocations(Locatable e) { strictcount(e.getLocation()) > 1 }
 
+/**
+ * Holds if `e` does not have a `Location`.
+ */
+query predicate noLocation(Locatable e) { not exists(e.getLocation()) }
+
 private predicate multiplePrimaryQlClasses(Element e) {
   strictcount(string cls | cls = e.getAPrimaryQlClass() and cls != "VariableAccess") > 1
 }
@@ -58,6 +63,9 @@ int getAstInconsistencyCounts(string type) {
   type = "Multiple locations" and
   result = count(Element e | multipleLocations(e) | e)
   or
+  type = "No location" and
+  result = count(Element e | noLocation(e) | e)
+  or
   type = "Multiple primary QL classes" and
   result = count(Element e | multiplePrimaryQlClasses(e) | e)
   or

rust/ql/lib/codeql/rust/Concepts.qll

@@ -105,7 +105,7 @@ module RemoteSource {
 }
 
 /**
- * A data-flow node that constructs a SQL statement.
+ * A data-flow node that constructs a SQL statement (for later execution).
  *
  * Often, it is worthy of an alert if a SQL statement is constructed such that
  * executing it would be a security risk.
@@ -133,10 +133,10 @@ module SqlConstruction {
 }
 
 /**
- * A data-flow node that executes SQL statements.
+ * A data-flow node that constructs and executes SQL statements.
  *
  * If the context of interest is such that merely constructing a SQL statement
- * would be valuable to report, consider using `SqlConstruction`.
+ * would be valuable to report, consider also using `SqlConstruction`.
  *
  * Extend this class to refine existing API models. If you want to model new APIs,
  * extend `SqlExecution::Range` instead.

rust/ql/lib/codeql/rust/Frameworks.qll

@@ -4,3 +4,4 @@
 
 private import codeql.rust.frameworks.Reqwest
 private import codeql.rust.frameworks.stdlib.Env
+private import codeql.rust.frameworks.Sqlx

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowConsistency.qll

@@ -0,0 +1,17 @@
+import codeql.rust.dataflow.DataFlow::DataFlow as DataFlow
+private import rust
+private import codeql.rust.dataflow.internal.DataFlowImpl
+private import codeql.rust.dataflow.internal.TaintTrackingImpl
+private import codeql.dataflow.internal.DataFlowImplConsistency
+
+private module Input implements InputSig<Location, RustDataFlow> {
+  predicate uniqueNodeLocationExclude(RustDataFlow::Node n) {
+    // Exclude nodes where the missing location can be explained by the
+    // underlying AST node not having a location.
+    not exists(n.asExpr().getLocation())
+  }
+
+  predicate missingLocationExclude(RustDataFlow::Node n) { not exists(n.asExpr().getLocation()) }
+}
+
+import MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>

rust/ql/lib/codeql/rust/frameworks/Sqlx.qll

@@ -0,0 +1,40 @@
+/**
+ * Provides modeling for the `SQLx` library.
+ */
+
+private import rust
+private import codeql.rust.Concepts
+private import codeql.rust.dataflow.DataFlow
+
+/**
+ * A call to `sqlx::query` and variations.
+ */
+private class SqlxQuery extends SqlConstruction::Range {
+  CallExpr call;
+
+  SqlxQuery() {
+    this.asExpr().getExpr() = call and
+    call.getFunction().(PathExpr).getPath().getResolvedPath() =
+      [
+        "crate::query::query", "crate::query_as::query_as", "crate::query_with::query_with",
+        "crate::query_as_with::query_as_with", "crate::query_scalar::query_scalar",
+        "crate::query_scalar_with::query_scalar_with", "crate::raw_sql::raw_sql"
+      ]
+  }
+
+  override DataFlow::Node getSql() { result.asExpr().getExpr() = call.getArgList().getArg(0) }
+}
+
+/**
+ * A call to `sqlx::Executor::execute`.
+ */
+private class SqlxExecute extends SqlExecution::Range {
+  MethodCallExpr call;
+
+  SqlxExecute() {
+    this.asExpr().getExpr() = call and
+    call.(Resolvable).getResolvedPath() = "crate::executor::Executor::execute"
+  }
+
+  override DataFlow::Node getSql() { result.asExpr().getExpr() = call.getArgList().getArg(0) }
+}

rust/ql/src/queries/diagnostics/DataFlowConsistencyCounts.ql

@@ -5,17 +5,10 @@
  * @id rust/diagnostics/data-flow-consistency-counts
  */
 
-private import rust
+import codeql.rust.dataflow.internal.DataFlowConsistency as Consistency
-private import codeql.rust.dataflow.internal.DataFlowImpl
-private import codeql.rust.dataflow.internal.TaintTrackingImpl
-private import codeql.dataflow.internal.DataFlowImplConsistency
-
-private module Input implements InputSig<Location, RustDataFlow> { }
 
 // see also `rust/diagnostics/data-flow-consistency`, which lists the
 // individual inconsistency results.
 from string type, int num
-where
+where num = Consistency::getInconsistencyCounts(type)
-  num =
-    MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>::getInconsistencyCounts(type)
 select type, num

rust/ql/src/queries/security/CWE-696/BadCtorInitialization.qhelp

@@ -0,0 +1,44 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+
+<p>
+Calling functions and methods in the Rust <code>std</code> library from a <code>#[ctor]</code> or <code>#[dtor]</code> function is not safe. This is because the <code>std</code> library only guarantees stability and portability between the beginning and the end of <code>main</code>, whereas <code>#[ctor]</code> functions are called before <code>main</code>, and <code>#[dtor]</code> functions are called after it.
+</p>
+
+</overview>
+<recommendation>
+
+<p>
+Do not call any part of the <code>std</code> library from a <code>#[ctor]</code> or <code>#[dtor]</code> function. Instead either:
+</p>
+<ul>
+  <li>Move the code to a different location, such as inside your program's <code>main</code> function.</li>
+  <li>Rewrite the code using an alternative library.</li>
+</ul>
+
+</recommendation>
+<example>
+
+<p>
+In the following example, a <code>#[ctor]</code> function uses the <code>println!</code> macro which calls <code>std</code> library functions. This may cause unexpected behavior at runtime.
+</p>
+
+<sample src="BadCtorInitializationBad.rs" />
+
+<p>
+The issue can be fixed by replacing <code>println!</code> with something that does not rely on the <code>std</code> library. In the fixed code below, we used the <code>libc_println!</code> macro from the <code>libc-print</code> library:
+</p>
+
+<sample src="BadCtorInitializationGood.rs" />
+
+</example>
+<references>
+
+<li>GitHub: <a href="https://github.com/mmastrac/rust-ctor?tab=readme-ov-file#warnings">rust-ctor - Warnings</a>.</li>
+<li>Rust Programming Language: <a href="https://doc.rust-lang.org/std/#use-before-and-after-main">Crate std - Use before and after main()</a>.</li>
+
+</references>
+</qhelp>

rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql

@@ -0,0 +1,58 @@
+/**
+ * @name Bad 'ctor' initialization
+ * @description Calling functions in the Rust std library from a ctor or dtor function is not safe.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id rust/ctor-initialization
+ * @tags reliability
+ *       correctness
+ *       external/cwe/cwe-696
+ *       external/cwe/cwe-665
+ */
+
+import rust
+
+/**
+ * A `#[ctor]` or `#[dtor]` attribute.
+ */
+class CtorAttr extends Attr {
+  string whichAttr;
+
+  CtorAttr() {
+    whichAttr = this.getMeta().getPath().getPart().getNameRef().getText() and
+    whichAttr = ["ctor", "dtor"]
+  }
+
+  string getWhichAttr() { result = whichAttr }
+}
+
+/**
+ * A call into the Rust standard library.
+ */
+class StdCall extends Expr {
+  StdCall() {
+    this.(CallExpr).getFunction().(PathExpr).getPath().getResolvedCrateOrigin() = "lang:std" or
+    this.(MethodCallExpr).getResolvedCrateOrigin() = "lang:std"
+  }
+}
+
+class PathElement = AstNode;
+
+query predicate edges(PathElement pred, PathElement succ) {
+  // starting edge
+  exists(CtorAttr ctor, Function f, StdCall call |
+    f.getAnAttr() = ctor and
+    call.getEnclosingCallable() = f and
+    pred = ctor and // source
+    succ = call // sink
+  )
+  // or
+  // transitive edge
+  // TODO
+}
+
+from CtorAttr ctor, StdCall call
+where edges*(ctor, call)
+select call, ctor, call,
+  "Call to " + call.toString() + " in a function with the " + ctor.getWhichAttr() + " attribute."

rust/ql/src/queries/security/CWE-696/BadCtorInitializationBad.rs

@@ -0,0 +1,5 @@
+
+#[ctor::ctor]
+fn bad_example() {
+    println!("Hello, world!"); // BAD: the println! macro calls std library functions
+}

rust/ql/src/queries/security/CWE-696/BadCtorInitializationGood.rs

@@ -0,0 +1,5 @@
+
+#[ctor::ctor]
+fn good_example() {
+    libc_print::libc_println!("Hello, world!"); // GOOD: libc-print does not use the std library
+}

rust/ql/src/queries/summary/Stats.qll

@@ -7,7 +7,7 @@ private import codeql.rust.dataflow.internal.DataFlowImpl
 private import codeql.rust.dataflow.internal.TaintTrackingImpl
 private import codeql.rust.AstConsistency as AstConsistency
 private import codeql.rust.controlflow.internal.CfgConsistency as CfgConsistency
-private import codeql.dataflow.internal.DataFlowImplConsistency as DataFlowImplConsistency
+private import codeql.rust.dataflow.internal.DataFlowConsistency as DataFlowConsistency
 
 /**
  * Gets a count of the total number of lines of code in the database.
@@ -35,15 +35,9 @@ int getTotalCfgInconsistencies() {
   result = sum(string type | | CfgConsistency::getCfgInconsistencyCounts(type))
 }
 
-private module Input implements DataFlowImplConsistency::InputSig<Location, RustDataFlow> { }
-
 /**
  * Gets a count of the total number of data flow inconsistencies in the database.
  */
 int getTotalDataFlowInconsistencies() {
-  result =
+  result = sum(string type | | DataFlowConsistency::getInconsistencyCounts(type))
-    sum(string type |
-      |
-      DataFlowImplConsistency::MakeConsistency<Location, RustDataFlow, RustTaintTracking, Input>::getInconsistencyCounts(type)
-    )
 }

rust/ql/test/extractor-tests/generated/MacroItems/CONSISTENCY/AstConsistency.expected

@@ -0,0 +1,42 @@
+noLocation
+| file://:0:0:0:0 | ... .parent(...) |
+| file://:0:0:0:0 | ... .unwrap(...) |
+| file://:0:0:0:0 | ...: ... |
+| file://:0:0:0:0 | ...::Path |
+| file://:0:0:0:0 | ...::path |
+| file://:0:0:0:0 | ArgList |
+| file://:0:0:0:0 | ArgList |
+| file://:0:0:0:0 | MacroItems |
+| file://:0:0:0:0 | ParamList |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | Path |
+| file://:0:0:0:0 | RefType |
+| file://:0:0:0:0 | RefType |
+| file://:0:0:0:0 | RetType |
+| file://:0:0:0:0 | StmtList |
+| file://:0:0:0:0 | Use |
+| file://:0:0:0:0 | UseTree |
+| file://:0:0:0:0 | fn get_parent |
+| file://:0:0:0:0 | get_parent |
+| file://:0:0:0:0 | parent |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | path |
+| file://:0:0:0:0 | std |
+| file://:0:0:0:0 | std |
+| file://:0:0:0:0 | std |
+| file://:0:0:0:0 | unwrap |
+| file://:0:0:0:0 | { ... } |

rust/ql/test/extractor-tests/generated/MacroItems/CONSISTENCY/DataFlowConsistency.expected

@@ -1,11 +0,0 @@
-uniqueNodeLocation
-| file://:0:0:0:0 | ... .parent(...) | Node should have one location but has 0. |
-| file://:0:0:0:0 | ... .parent(...) | Node should have one location but has 0. |
-| file://:0:0:0:0 | ... .unwrap(...) | Node should have one location but has 0. |
-| file://:0:0:0:0 | ...: ... | Node should have one location but has 0. |
-| file://:0:0:0:0 | path | Node should have one location but has 0. |
-| file://:0:0:0:0 | path | Node should have one location but has 0. |
-| file://:0:0:0:0 | path | Node should have one location but has 0. |
-| file://:0:0:0:0 | { ... } | Node should have one location but has 0. |
-missingLocation
-| Nodes without location: 8 |

rust/ql/test/query-tests/diagnostics/AstConsistencyCounts.expected

@@ -2,3 +2,4 @@
 | Multiple parents | 0 |
 | Multiple primary QL classes | 0 |
 | Multiple toStrings | 0 |
+| No location | 0 |

rust/ql/test/query-tests/security/CWE-089/SqlSinks.expected

@@ -0,0 +1,2 @@
+testFailures
+failures

rust/ql/test/query-tests/security/CWE-089/SqlSinks.ql

@@ -0,0 +1,19 @@
+import rust
+import codeql.rust.security.SqlInjectionExtensions
+import utils.InlineExpectationsTest
+
+module SqlSinksTest implements TestSig {
+  string getARelevantTag() { result = "sql-sink" }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(SqlInjection::Sink sink |
+      location = sink.getLocation() and
+      location.getFile().getBaseName() != "" and
+      element = sink.toString() and
+      tag = "sql-sink" and
+      value = ""
+    )
+  }
+}
+
+import MakeTest<SqlSinksTest>

rust/ql/test/query-tests/security/CWE-089/sqlx.rs

@@ -44,8 +44,8 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err
 
     // construct queries (with extra variants)
     let const_string = String::from("Alice");
-    let arg_string = std::env::args().nth(1).unwrap_or(String::from("Alice")); // $ MISSING Source=args1
+    let arg_string = std::env::args().nth(1).unwrap_or(String::from("Alice")); // $ MISSING: Source=args1
-    let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING Source=remote1
+    let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING: Source=remote1
     let remote_number = remote_string.parse::<i32>().unwrap_or(0);
     let safe_query_1 = String::from("SELECT * FROM people WHERE firstname='Alice'");
     let safe_query_2 = String::from("SELECT * FROM people WHERE firstname='") + &const_string + "'";
@@ -57,31 +57,31 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err
     let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=?"); // (prepared arguments are safe)
 
     // direct execution
-    let _ = conn.execute(safe_query_1.as_str()).await?;
+    let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink
-    let _ = conn.execute(safe_query_2.as_str()).await?;
+    let _ = conn.execute(safe_query_2.as_str()).await?; // $ sql-sink
-    let _ = conn.execute(safe_query_3.as_str()).await?;
+    let _ = conn.execute(safe_query_3.as_str()).await?; // $ sql-sink
-    let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING Alert[sql-injection]=args1
+    let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=args1
     if enable_remote {
-        let _ = conn.execute(unsafe_query_2.as_str()).await?; // $ MISSING Alert[sql-injection]=remote1
+        let _ = conn.execute(unsafe_query_2.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
-        let _ = conn.execute(unsafe_query_3.as_str()).await?; // $ MISSING Alert[sql-injection]=remote1
+        let _ = conn.execute(unsafe_query_3.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
-        let _ = conn.execute(unsafe_query_4.as_str()).await?; // $ MISSING Alert[sql-injection]=remote1
+        let _ = conn.execute(unsafe_query_4.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
     }
 
     // prepared queries
-    let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?;
+    let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?; // $ sql-sink
-    let _ = sqlx::query(safe_query_2.as_str()).execute(&pool).await?;
+    let _ = sqlx::query(safe_query_2.as_str()).execute(&pool).await?; // $ sql-sink
-    let _ = sqlx::query(safe_query_3.as_str()).execute(&pool).await?;
+    let _ = sqlx::query(safe_query_3.as_str()).execute(&pool).await?; // $ sql-sink
-    let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=args1
+    let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=args1
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_2.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=remote1
+        let _ = sqlx::query(unsafe_query_2.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
-        let _ = sqlx::query(unsafe_query_3.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=remote1
+        let _ = sqlx::query(unsafe_query_3.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
-        let _ = sqlx::query(unsafe_query_4.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=remote1
+        let _ = sqlx::query(unsafe_query_4.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote1
     }
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(const_string).execute(&pool).await?;
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(const_string).execute(&pool).await?; // $ sql-sink
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(arg_string).execute(&pool).await?;
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(arg_string).execute(&pool).await?; // $ sql-sink
     if enable_remote {
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_string).execute(&pool).await?;
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_string).execute(&pool).await?; // $ sql-sink
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_number).execute(&pool).await?;
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(remote_number).execute(&pool).await?; // $ sql-sink
     }
 
     Ok(())
@@ -93,67 +93,67 @@ async fn test_sqlx_sqlite(url: &str, enable_remote: bool) -> Result<(), sqlx::Er
 
     // construct queries
     let const_string = String::from("Alice");
-    let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING Source=remote2
+    let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING: Source=remote2
     let safe_query_1 = String::from("SELECT * FROM people WHERE firstname='") + &const_string + "'";
     let unsafe_query_1 = String::from("SELECT * FROM people WHERE firstname='") + &remote_string + "'";
     let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=?"); // (prepared arguments are safe)
 
     // direct execution (with extra variants)
-    let _ = conn.execute(safe_query_1.as_str()).await?;
+    let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink
     if enable_remote {
-        let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING Alert[sql-injection]=remote2
+        let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
     }
     // ...
-    let _ = sqlx::raw_sql(safe_query_1.as_str()).execute(&mut conn).await?;
+    let _ = sqlx::raw_sql(safe_query_1.as_str()).execute(&mut conn).await?; // $ sql-sink
     if enable_remote {
-        let _ = sqlx::raw_sql(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING Alert[sql-injection]=remote2
+        let _ = sqlx::raw_sql(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
     }
 
     // prepared queries (with extra variants)
-    let _ = sqlx::query(safe_query_1.as_str()).execute(&mut conn).await?;
+    let _ = sqlx::query(safe_query_1.as_str()).execute(&mut conn).await?; // $ sql-sink
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&mut conn).await?;
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&mut conn).await?; // $ sql-sink
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ MISSING Alert[sql-injection]=remote2
+        let _ = sqlx::query(unsafe_query_1.as_str()).execute(&mut conn).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&mut conn).await?;
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&mut conn).await?; // $ sql-sink
     }
     // ...
-    let _ = sqlx::query(safe_query_1.as_str()).fetch(&mut conn);
+    let _ = sqlx::query(safe_query_1.as_str()).fetch(&mut conn); // $ sql-sink
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch(&mut conn);
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch(&mut conn); // $ sql-sink
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_1.as_str()).fetch(&mut conn); // $ MISSING Alert[sql-injection]=remote2
+        let _ = sqlx::query(unsafe_query_1.as_str()).fetch(&mut conn); // $ sql-sink MISSING: Alert[sql-injection]=remote2
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch(&mut conn);
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch(&mut conn); // $ sql-sink
     }
     // ...
-    let row1: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_one(&mut conn).await?;
+    let row1: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_one(&mut conn).await?; // $ sql-sink
     println!("  row1 = {:?}", row1);
-    let row2: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_one(&mut conn).await?;
+    let row2: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_one(&mut conn).await?; // $ sql-sink
     println!("  row2 = {:?}", row2);
     if enable_remote {
-        let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_one(&mut conn).await?; // $ MISSING Alert[sql-injection]=remote2
+        let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_one(&mut conn).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
-        let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_one(&mut conn).await?;
+        let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_one(&mut conn).await?; // $ sql-sink
     }
     // ...
-    let row3: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data");
+    let row3: (i64, String, String) = sqlx::query_as(safe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data"); // $ sql-sink
     println!("  row3 = {:?}", row3);
-    let row4: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_optional(&mut conn).await?.expect("no data");
+    let row4: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&const_string).fetch_optional(&mut conn).await?.expect("no data"); // $ sql-sink
     println!("  row4 = {:?}", row4);
     if enable_remote {
-        let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data"); // $ MISSING Alert[sql-injection]=remote2
+        let _: (i64, String, String) = sqlx::query_as(unsafe_query_1.as_str()).fetch_optional(&mut conn).await?.expect("no data"); // $ sql-sink $ MISSING: Alert[sql-injection]=remote2
-        let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_optional(&mut conn).await?.expect("no data");
+        let _: (i64, String, String) = sqlx::query_as(prepared_query_1.as_str()).bind(&remote_string).fetch_optional(&mut conn).await?.expect("no data"); // $ sql-sink
     }
     // ...
-    let _ = sqlx::query(safe_query_1.as_str()).fetch_all(&mut conn).await?;
+    let _ = sqlx::query(safe_query_1.as_str()).fetch_all(&mut conn).await?; // $ sql-sink
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch_all(&mut conn).await?;
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).fetch_all(&mut conn).await?; // $ sql-sink
-    let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&const_string).fetch_all(&mut conn).await?;
+    let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&const_string).fetch_all(&mut conn).await?; // $ sql-sink
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_1.as_str()).fetch_all(&mut conn).await?; // $ MISSING Alert[sql-injection]=remote2
+        let _ = sqlx::query(unsafe_query_1.as_str()).fetch_all(&mut conn).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote2
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch_all(&mut conn).await?;
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).fetch_all(&mut conn).await?; // $ sql-sink
-        let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&remote_string).fetch_all(&mut conn).await?;
+        let _ = sqlx::query("SELECT * FROM people WHERE firstname=?").bind(&remote_string).fetch_all(&mut conn).await?; // $ sql-sink
     }
     // ...
-    let _ = sqlx::query!("SELECT * FROM people WHERE firstname=$1", const_string).fetch_all(&mut conn).await?; // (only takes string literals, so can't be vulnerable)
+    let _ = sqlx::query!("SELECT * FROM people WHERE firstname=$1", const_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink (only takes string literals, so can't be vulnerable)
     if enable_remote {
-        let _ = sqlx::query!("SELECT * FROM people WHERE firstname=$1", remote_string).fetch_all(&mut conn).await?;
+        let _ = sqlx::query!("SELECT * FROM people WHERE firstname=$1", remote_string).fetch_all(&mut conn).await?; // $ MISSING: sql-sink
     }
 
     Ok(())
@@ -166,23 +166,23 @@ async fn test_sqlx_postgres(url: &str, enable_remote: bool) -> Result<(), sqlx::
 
     // construct queries
     let const_string = String::from("Alice");
-    let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING Source=remote3
+    let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ MISSING: Source=remote3
     let safe_query_1 = String::from("SELECT * FROM people WHERE firstname='") + &const_string + "'";
     let unsafe_query_1 = String::from("SELECT * FROM people WHERE firstname='") + &remote_string + "'";
     let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=$1"); // (prepared arguments are safe)
 
     // direct execution
-    let _ = conn.execute(safe_query_1.as_str()).await?;
+    let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink
     if enable_remote {
-        let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING Alert[sql-injection]=remote3
+        let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote3
     }
 
     // prepared queries
-    let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?;
+    let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?; // $ sql-sink
-    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&pool).await?;
+    let _ = sqlx::query(prepared_query_1.as_str()).bind(&const_string).execute(&pool).await?; // $ sql-sink
     if enable_remote {
-        let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ MISSING Alert[sql-injection]=remote3
+        let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[sql-injection]=remote3
-        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&pool).await?;
+        let _ = sqlx::query(prepared_query_1.as_str()).bind(&remote_string).execute(&pool).await?; // $ sql-sink
     }
 
     Ok(())

rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.expected

@@ -0,0 +1,22 @@
+#select
+| test.rs:31:9:31:25 | ...::stdout(...) | test.rs:29:1:29:13 | Attr | test.rs:31:9:31:25 | ...::stdout(...) | Call to ...::stdout(...) in a function with the ctor attribute. |
+| test.rs:36:9:36:25 | ...::stdout(...) | test.rs:34:1:34:13 | Attr | test.rs:36:9:36:25 | ...::stdout(...) | Call to ...::stdout(...) in a function with the dtor attribute. |
+| test.rs:43:9:43:25 | ...::stdout(...) | test.rs:40:1:40:13 | Attr | test.rs:43:9:43:25 | ...::stdout(...) | Call to ...::stdout(...) in a function with the dtor attribute. |
+| test.rs:53:9:53:16 | stdout(...) | test.rs:51:1:51:7 | Attr | test.rs:53:9:53:16 | stdout(...) | Call to stdout(...) in a function with the ctor attribute. |
+| test.rs:58:9:58:16 | stderr(...) | test.rs:56:1:56:7 | Attr | test.rs:58:9:58:16 | stderr(...) | Call to stderr(...) in a function with the ctor attribute. |
+| test.rs:63:14:63:28 | ...::_print(...) | test.rs:61:1:61:7 | Attr | test.rs:63:14:63:28 | ...::_print(...) | Call to ...::_print(...) in a function with the ctor attribute. |
+| test.rs:69:9:69:24 | ...::stdin(...) | test.rs:66:1:66:7 | Attr | test.rs:69:9:69:24 | ...::stdin(...) | Call to ...::stdin(...) in a function with the ctor attribute. |
+| test.rs:90:5:90:35 | ...::sleep(...) | test.rs:88:1:88:7 | Attr | test.rs:90:5:90:35 | ...::sleep(...) | Call to ...::sleep(...) in a function with the ctor attribute. |
+| test.rs:97:5:97:23 | ...::exit(...) | test.rs:95:1:95:7 | Attr | test.rs:97:5:97:23 | ...::exit(...) | Call to ...::exit(...) in a function with the ctor attribute. |
+| test.rs:166:5:166:15 | ...::stdout(...) | test.rs:164:1:164:7 | Attr | test.rs:166:5:166:15 | ...::stdout(...) | Call to ...::stdout(...) in a function with the ctor attribute. |
+edges
+| test.rs:29:1:29:13 | Attr | test.rs:31:9:31:25 | ...::stdout(...) |
+| test.rs:34:1:34:13 | Attr | test.rs:36:9:36:25 | ...::stdout(...) |
+| test.rs:40:1:40:13 | Attr | test.rs:43:9:43:25 | ...::stdout(...) |
+| test.rs:51:1:51:7 | Attr | test.rs:53:9:53:16 | stdout(...) |
+| test.rs:56:1:56:7 | Attr | test.rs:58:9:58:16 | stderr(...) |
+| test.rs:61:1:61:7 | Attr | test.rs:63:14:63:28 | ...::_print(...) |
+| test.rs:66:1:66:7 | Attr | test.rs:69:9:69:24 | ...::stdin(...) |
+| test.rs:88:1:88:7 | Attr | test.rs:90:5:90:35 | ...::sleep(...) |
+| test.rs:95:1:95:7 | Attr | test.rs:97:5:97:23 | ...::exit(...) |
+| test.rs:164:1:164:7 | Attr | test.rs:166:5:166:15 | ...::stdout(...) |

rust/ql/test/query-tests/security/CWE-696/BadCTorInitialization.qlref

@@ -0,0 +1,2 @@
+query: queries/security/CWE-696/BadCtorInitialization.ql
+postprocess: utils/InlineExpectationsTestQuery.ql

rust/ql/test/query-tests/security/CWE-696/options.yml

@@ -0,0 +1,4 @@
+qltest_cargo_check: true
+qltest_dependencies:
+    - ctor = { version = "0.2.9" }
+    - libc-print = { version = "0.1.23" }

rust/ql/test/query-tests/security/CWE-696/test.rs

@@ -0,0 +1,167 @@
+
+// --- attribute variants ---
+
+use std::io::Write;
+
+fn harmless1_1() {
+    // ...
+}
+
+#[ctor::ctor]
+fn harmless1_2() {
+    // ...
+}
+
+#[ctor::dtor]
+fn harmless1_3() {
+    // ...
+}
+
+fn harmless1_4() {
+    _ = std::io::stdout().write(b"Hello, world!");
+}
+
+#[rustfmt::skip]
+fn harmless1_5() {
+    _ = std::io::stdout().write(b"Hello, world!");
+}
+
+#[ctor::ctor] // $ Source=source1_1
+fn bad1_1() {
+    _ = std::io::stdout().write(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source1_1
+}
+
+#[ctor::dtor] // $ Source=source1_2
+fn bad1_2() {
+    _ = std::io::stdout().write(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source1_2
+}
+
+#[rustfmt::skip]
+#[ctor::dtor] // $ Source=source1_3
+#[rustfmt::skip]
+fn bad1_3() {
+    _ = std::io::stdout().write(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source1_3
+}
+
+// --- code variants ---
+
+use ctor::ctor;
+use std::io::*;
+
+#[ctor] // $ Source=source2_1
+fn bad2_1() {
+    _ = stdout().write(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source2_1
+}
+
+#[ctor] // $ Source=source2_2
+fn bad2_2() {
+    _ = stderr().write_all(b"Hello, world!"); // $ Alert[rust/ctor-initialization]=source2_2
+}
+
+#[ctor] // $ Source=source2_3
+fn bad2_3() {
+    println!("Hello, world!"); // $ Alert[rust/ctor-initialization]=source2_3
+}
+
+#[ctor] // $ Source=source2_4
+fn bad2_4() {
+    let mut buff = String::new();
+    _ = std::io::stdin().read_line(&mut buff); // $ Alert[rust/ctor-initialization]=source2_4
+}
+
+use std::fs;
+
+#[ctor] // $ MISSING: Source=source2_5
+fn bad2_5() {
+    let _buff = fs::File::create("hello.txt").unwrap(); // $ MISSING: Alert[rust/ctor-initialization]=source2_5
+}
+
+#[ctor] // $ MISSING: Source=source2_6
+fn bad2_6() {
+    let _t = std::time::Instant::now(); // $ MISSING: Alert[rust/ctor-initialization]=source2_6
+}
+
+use std::time::Duration;
+
+const DURATION2_7: Duration = Duration::new(1, 0);
+
+#[ctor] // $ Source=source2_7
+fn bad2_7() {
+    std::thread::sleep(DURATION2_7); // $ Alert[rust/ctor-initialization]=source2_7
+}
+
+use std::process;
+
+#[ctor] // $ Source=source2_8
+fn bad2_8() {
+    process::exit(1234); // $ Alert[rust/ctor-initialization]=source2_8
+}
+
+#[ctor::ctor]
+fn harmless2_9() {
+    libc_print::libc_println!("Hello, world!"); // does not use the std library
+}
+
+#[ctor::ctor]
+fn harmless2_10() {
+    core::assert!(true); // core library should be OK in this context
+}
+
+extern crate alloc;
+use alloc::alloc::{alloc, dealloc, Layout};
+
+#[ctor::ctor]
+unsafe fn harmless2_11() {
+    let layout = Layout::new::<u64>();
+    let ptr = alloc(layout); // alloc library should be OK in this context
+
+    if !ptr.is_null() {
+        dealloc(ptr, layout);
+    }
+}
+
+// --- transitive cases ---
+
+fn call_target3_1() {
+    _ = stderr().write_all(b"Hello, world!"); // $ MISSING: Alert=source3_1 Alert=source3_3 Alert=source3_4
+}
+
+#[ctor] // $ MISSING: Source=source3_1
+fn bad3_1() {
+    call_target3_1();
+}
+
+fn call_target3_2() {
+    for _x in 0..10 {
+        // ...
+    }
+}
+
+#[ctor] // $ MISSING: Source=source3_2
+fn harmless3_2() {
+    call_target3_2();
+}
+
+#[ctor]
+fn bad3_3() {
+    call_target3_1();
+    call_target3_2();
+}
+
+#[ctor] // $ MISSING: Source=source3_4
+fn bad3_4() {
+    bad3_3();
+}
+
+// --- macros ---
+
+macro_rules! macro4_1 {
+    () => {
+        _ = std::io::stdout().write(b"Hello, world!");
+    };
+}
+
+#[ctor] // $ Source=source4_1
+fn bad4_1() {
+    macro4_1!(); // $ Alert[rust/ctor-initialization]=source4_1
+}

shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll

@@ -343,7 +343,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
     bindingset[n, cc]
     pragma[inline_late]
     private predicate isUnreachableInCall1(NodeEx n, LocalCallContextSpecificCall cc) {
-      cc.unreachable(n.asNode())
+      cc.unreachable(n)
     }
 
     /**
@@ -423,7 +423,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
     pragma[nomagic]
     private predicate readSetEx(NodeEx node1, ContentSet c, NodeEx node2) {
-      readSet(pragma[only_bind_into](node1.asNode()), c, pragma[only_bind_into](node2.asNode())) and
+      readEx(node1, c, node2) and
       stepFilter(node1, node2)
       or
       exists(Node n |
@@ -450,20 +450,19 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
     bindingset[c]
     private predicate expectsContentEx(NodeEx n, Content c) {
       exists(ContentSet cs |
-        expectsContentCached(n.asNode(), cs) and
+        expectsContentSet(n, cs) and
         pragma[only_bind_out](c) = pragma[only_bind_into](cs).getAReadContent()
       )
     }
 
     pragma[nomagic]
-    private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
+    private predicate notExpectsContent(NodeEx n) { not expectsContentSet(n, _) }
 
     pragma[nomagic]
-    private predicate storeExUnrestricted(
+    private predicate storeUnrestricted(
       NodeEx node1, Content c, NodeEx node2, DataFlowType contentType, DataFlowType containerType
     ) {
-      store(pragma[only_bind_into](node1.asNode()), c, pragma[only_bind_into](node2.asNode()),
+      storeEx(node1, c, node2, contentType, containerType) and
-        contentType, containerType) and
       stepFilter(node1, node2)
     }
 
@@ -471,23 +470,13 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
     private predicate hasReadStep(Content c) { read(_, c, _) }
 
     pragma[nomagic]
-    private predicate storeEx(
+    private predicate store(
       NodeEx node1, Content c, NodeEx node2, DataFlowType contentType, DataFlowType containerType
     ) {
-      storeExUnrestricted(node1, c, node2, contentType, containerType) and
+      storeUnrestricted(node1, c, node2, contentType, containerType) and
       hasReadStep(c)
     }
 
-    pragma[nomagic]
-    private predicate viableReturnPosOutEx(DataFlowCall call, ReturnPosition pos, NodeEx out) {
-      viableReturnPosOut(call, pos, out.asNode())
-    }
-
-    pragma[nomagic]
-    private predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx arg) {
-      viableParamArg(call, p.asNode(), arg.asNode())
-    }
-
     /**
      * Holds if field flow should be used for the given configuration.
      */
@@ -520,7 +509,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       exists(ParameterPosition pos | p.isParameterOf(_, pos) |
         not kind.(ParamUpdateReturnKind).getPosition() = pos
         or
-        allowParameterReturnInSelfCached(p.asNode())
+        allowParameterReturnInSelfEx(p)
       )
     }
 
@@ -558,7 +547,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
         exists(NodeEx mid |
           useFieldFlow() and
           fwdFlow(mid, cc) and
-          storeEx(mid, _, node, _, _)
+          store(mid, _, node, _, _)
         )
         or
         // read
@@ -653,7 +642,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
           not fullBarrier(node) and
           useFieldFlow() and
           fwdFlow(mid, _) and
-          storeEx(mid, c, node, _, _)
+          store(mid, c, node, _, _)
         )
       }
 
@@ -796,7 +785,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
         exists(NodeEx mid |
           revFlow(mid, toReturn) and
           fwdFlowConsCand(c) and
-          storeEx(node, c, mid, _, _)
+          store(node, c, mid, _, _)
         )
       }
 
@@ -893,7 +882,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       ) {
         revFlowIsReadAndStored(c) and
         revFlow(node2) and
-        storeEx(node1, c, node2, contentType, containerType) and
+        store(node1, c, node2, contentType, containerType) and
         exists(ap1)
       }
 
@@ -1152,7 +1141,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
         flowOutOfCallNodeCand1(call, ret, _, out) and
         c = ret.getEnclosingCallable()
       |
-        scope = getSecondLevelScopeCached(ret.asNode())
+        scope = getSecondLevelScopeEx(ret)
         or
         ret = TParamReturnNode(_, scope)
       )
@@ -1496,7 +1485,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
           PrevStage::revFlow(node, state, apa) and
           filter(node, state, t0, ap, t) and
           (
-            if castingNodeEx(node)
+            if node instanceof CastingNodeEx
             then
               ap instanceof ApNil or
               compatibleContainer(getHeadContent(ap), node.getDataFlowType()) or
@@ -2627,10 +2616,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
             FlowCheckNode() {
               revFlow(this, _, _) and
               (
-                castNode(this.asNode()) or
+                flowCheckNode(this) or
-                clearsContentCached(this.asNode(), _) or
-                expectsContentCached(this.asNode(), _) or
-                neverSkipInPathGraph(this.asNode()) or
                 Config::neverSkip(this.asNode())
               )
             }
@@ -2665,7 +2651,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
               or
               node instanceof ParamNodeEx
               or
-              node.asNode() instanceof OutNodeExt
+              node instanceof OutNodeEx
               or
               storeStepCand(_, _, _, node, _, _)
               or
@@ -2899,15 +2885,9 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
             predicate isHidden() {
               not Config::includeHiddenNodes() and
-              (
+              hiddenNode(this.getNodeEx()) and
-                hiddenNode(this.getNodeEx().asNode()) and
+              not this.isSource() and
-                not this.isSource() and
+              not this instanceof PathNodeSink
-                not this instanceof PathNodeSink
-                or
-                this.getNodeEx() instanceof TNodeImplicitRead
-                or
-                hiddenNode(this.getNodeEx().asParamReturnNode())
-              )
             }
 
             /** Gets a textual representation of this element. */
@@ -3770,11 +3750,6 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
     private module Stage2 = MkStage<Stage1>::Stage<Stage2Param>;
 
-    pragma[nomagic]
-    private predicate castingNodeEx(NodeEx node) {
-      node.asNode() instanceof CastingNode or exists(node.asParamReturnNode())
-    }
-
     private module Stage3Param implements MkStage<Stage2>::StageParam {
       private module PrevStage = Stage2;
 
@@ -3888,7 +3863,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
     bindingset[node, t0]
     private predicate strengthenType(NodeEx node, DataFlowType t0, DataFlowType t) {
-      if castingNodeEx(node)
+      if node instanceof CastingNodeEx
       then
         exists(DataFlowType nt | nt = node.getDataFlowType() |
           if typeStrongerThanFilter(nt, t0)
@@ -3945,7 +3920,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       pragma[nomagic]
       private predicate clearSet(NodeEx node, ContentSet c) {
         PrevStage::revFlow(node) and
-        clearsContentCached(node.asNode(), c)
+        clearsContentSet(node, c)
       }
 
       pragma[nomagic]
@@ -5024,7 +4999,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       bindingset[c]
       private predicate clearsContentEx(NodeEx n, Content c) {
         exists(ContentSet cs |
-          clearsContentCached(n.asNode(), cs) and
+          clearsContentSet(n, cs) and
           pragma[only_bind_out](c) = pragma[only_bind_into](cs).getAReadContent()
         )
       }
@@ -5377,7 +5352,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
           midNode = mid.getNodeEx() and
           t1 = mid.getType() and
           ap1 = mid.getAp() and
-          storeExUnrestricted(midNode, c, node, contentType, t2) and
+          storeUnrestricted(midNode, c, node, contentType, t2) and
           ap2.getHead() = c and
           ap2.len() = unbindInt(ap1.len() + 1) and
           compatibleTypesFilter(t1, contentType)
@@ -5442,9 +5417,8 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
         PartialAccessPath ap
       ) {
         exists(ReturnKindExt kind, DataFlowCall call |
-          partialPathOutOfCallable1(mid, call, kind, state, cc, t, ap)
+          partialPathOutOfCallable1(mid, call, kind, state, cc, t, ap) and
-        |
+          out = kind.getAnOutNodeEx(call)
-          out.asNode() = kind.getAnOutNode(call)
         )
       }
 
@@ -5529,7 +5503,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       ) {
         exists(DataFlowCall call, ReturnKindExt kind |
           partialPathThroughCallable0(call, mid, kind, state, cc, t, ap) and
-          out.asNode() = kind.getAnOutNode(call)
+          out = kind.getAnOutNodeEx(call)
         )
       }
 
@@ -5549,7 +5523,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
           not outBarrier(node, state) and
           // if a node is not the target of a store, we can check `clearsContent` immediately
           (
-            storeExUnrestricted(_, _, node, _, _)
+            storeUnrestricted(_, _, node, _, _)
             or
             not clearsContentEx(node, ap.getHead())
           )
@@ -5690,7 +5664,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
         exists(NodeEx midNode |
           midNode = mid.getNodeEx() and
           ap = mid.getAp() and
-          storeExUnrestricted(node, c, midNode, _, _) and
+          storeUnrestricted(node, c, midNode, _, _) and
           ap.getHead() = c
         )
       }
@@ -5745,7 +5719,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       ) {
         exists(DataFlowCall call, ArgumentPosition pos |
           revPartialPathThroughCallable0(call, mid, pos, state, ap) and
-          node.asNode().(ArgNode).argumentOf(call, pos)
+          node.argumentOf(call, pos)
         )
       }
 

shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll

@@ -286,7 +286,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     ) {
       revLambdaFlow0(lambdaCall, kind, node, t, toReturn, toJump, lastCall) and
       not expectsContent(node, _) and
-      if castNode(node) or node instanceof ArgNode or node instanceof ReturnNode
+      if node instanceof CastNode or node instanceof ArgNode or node instanceof ReturnNode
       then compatibleTypesFilter(t, getNodeDataFlowType(node))
       else any()
     }
@@ -372,7 +372,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     ) {
       revLambdaFlow(lambdaCall, kind, out, t, _, toJump, lastCall) and
       exists(ReturnKindExt rk |
-        out = rk.getAnOutNode(call) and
+        out = getAnOutNodeExt(call, rk) and
         lambdaCall(call, _, _)
       )
     }
@@ -901,20 +901,36 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     Location getLocation() { result = this.projectToNode().getLocation() }
   }
 
-  final class ArgNodeEx extends NodeEx {
+  /**
-    ArgNodeEx() { this.asNode() instanceof ArgNode }
+   * A `Node` at which a cast can occur such that the type should be checked.
+   */
+  final class CastingNodeEx extends NodeEx {
+    CastingNodeEx() { castingNodeEx(this) }
+  }
 
-    DataFlowCall getCall() { this.asNode().(ArgNode).argumentOf(result, _) }
+  final class ArgNodeEx extends NodeEx {
+    private DataFlowCall call_;
+    private ArgumentPosition pos_;
+
+    ArgNodeEx() { this.asNode().(ArgNode).argumentOf(call_, pos_) }
+
+    predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
+      call = call_ and
+      pos = pos_
+    }
+
+    DataFlowCall getCall() { result = call_ }
   }
 
   final class ParamNodeEx extends NodeEx {
-    ParamNodeEx() { this.asNode() instanceof ParamNode }
+    private DataFlowCallable c_;
+    private ParameterPosition pos_;
 
-    predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
+    ParamNodeEx() { this.asNode().(ParamNode).isParameterOf(c_, pos_) }
-      this.asNode().(ParamNode).isParameterOf(c, pos)
-    }
 
-    ParameterPosition getPosition() { this.isParameterOf(_, result) }
+    predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { c = c_ and pos = pos_ }
+
+    ParameterPosition getPosition() { result = pos_ }
   }
 
   /**
@@ -931,6 +947,18 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     ReturnKindExt getKind() { result = pos.getKind() }
   }
 
+  final class OutNodeEx extends NodeEx {
+    OutNodeEx() { this.asNode() instanceof OutNodeExt }
+  }
+
+  pragma[nomagic]
+  private SndLevelScopeOption getSecondLevelScope0(Node n) {
+    result = SndLevelScopeOption::some(getSecondLevelScope(n))
+    or
+    result instanceof SndLevelScopeOption::None and
+    not exists(getSecondLevelScope(n))
+  }
+
   cached
   private module Cached {
     /**
@@ -942,11 +970,8 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     predicate forceCachingInSameStage() { any() }
 
     cached
-    SndLevelScopeOption getSecondLevelScopeCached(Node n) {
+    SndLevelScopeOption getSecondLevelScopeEx(NodeEx n) {
-      result = SndLevelScopeOption::some(getSecondLevelScope(n))
+      result = getSecondLevelScope0(n.asNode())
-      or
-      result instanceof SndLevelScopeOption::None and
-      not exists(getSecondLevelScope(n))
     }
 
     cached
@@ -978,11 +1003,14 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     predicate jumpStepCached(Node node1, Node node2) { jumpStep(node1, node2) }
 
     cached
-    predicate clearsContentCached(Node n, ContentSet c) { clearsContent(n, c) }
+    predicate clearsContentSet(NodeEx n, ContentSet c) { clearsContent(n.asNode(), c) }
 
     cached
     predicate expectsContentCached(Node n, ContentSet c) { expectsContent(n, c) }
 
+    cached
+    predicate expectsContentSet(NodeEx n, ContentSet c) { expectsContent(n.asNode(), c) }
+
     cached
     predicate isUnreachableInCallCached(NodeRegion nr, DataFlowCall call) {
       isUnreachableInCall(nr, call)
@@ -996,16 +1024,15 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     }
 
     cached
-    predicate hiddenNode(Node n) { nodeIsHidden(n) }
+    predicate hiddenNode(NodeEx n) {
+      nodeIsHidden([n.asNode(), n.asParamReturnNode()])
+      or
+      n instanceof TNodeImplicitRead
+    }
 
     cached
-    OutNodeExt getAnOutNodeExt(DataFlowCall call, ReturnKindExt k) {
+    OutNodeEx getAnOutNodeEx(DataFlowCall call, ReturnKindExt k) {
-      result = getAnOutNode(call, k.(ValueReturnKind).getKind())
+      result.asNode() = getAnOutNodeExt(call, k)
-      or
-      exists(ArgNode arg |
-        result.(PostUpdateNode).getPreUpdateNode() = arg and
-        arg.argumentOf(call, k.(ParamUpdateReturnKind).getAMatchingArgumentPosition())
-      )
     }
 
     pragma[nomagic]
@@ -1016,22 +1043,22 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
         parameterValueFlowsToPreUpdate(p, n) and
         p.isParameterOf(_, pos) and
         k = TParamUpdate(pos) and
-        scope = getSecondLevelScopeCached(n)
+        scope = getSecondLevelScope0(n)
       )
     }
 
     cached
-    predicate castNode(Node n) { n instanceof CastNode }
+    predicate flowCheckNode(NodeEx n) {
+      n.asNode() instanceof CastNode or
+      clearsContentSet(n, _) or
+      expectsContentSet(n, _) or
+      neverSkipInPathGraph(n.asNode())
+    }
 
     cached
-    predicate castingNode(Node n) {
+    predicate castingNodeEx(NodeEx n) {
-      castNode(n) or
+      n.asNode() instanceof CastingNode or
-      n instanceof ParamNode or
+      exists(n.asParamReturnNode())
-      n instanceof OutNodeExt or
-      // For reads, `x.f`, we want to check that the tracked type after the read (which
-      // is obtained by popping the head of the access path stack) is compatible with
-      // the type of `x.f`.
-      readSet(_, _, n)
     }
 
     cached
@@ -1208,6 +1235,15 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
       )
     }
 
+    /**
+     * Holds if `arg` is a possible argument to `p` in `call`, taking virtual
+     * dispatch into account.
+     */
+    cached
+    predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx arg) {
+      viableParamArg(call, p.asNode(), arg.asNode())
+    }
+
     pragma[nomagic]
     private ReturnPosition viableReturnPos(DataFlowCall call, ReturnKindExt kind) {
       viableCallableExt(call) = result.getCallable() and
@@ -1219,13 +1255,22 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
      * taking virtual dispatch into account.
      */
     cached
-    predicate viableReturnPosOut(DataFlowCall call, ReturnPosition pos, Node out) {
+    predicate viableReturnPosOut(DataFlowCall call, ReturnPosition pos, OutNodeExt out) {
       exists(ReturnKindExt kind |
         pos = viableReturnPos(call, kind) and
-        out = kind.getAnOutNode(call)
+        out = getAnOutNodeExt(call, kind)
       )
     }
 
+    /**
+     * Holds if a value at return position `pos` can be returned to `out` via `call`,
+     * taking virtual dispatch into account.
+     */
+    cached
+    predicate viableReturnPosOutEx(DataFlowCall call, ReturnPosition pos, OutNodeEx out) {
+      viableReturnPosOut(call, pos, out.asNode())
+    }
+
     /** Provides predicates for calculating flow-through summaries. */
     private module FlowThrough {
       /**
@@ -1559,6 +1604,11 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     cached
     predicate readSet(Node node1, ContentSet c, Node node2) { readStep(node1, c, node2) }
 
+    cached
+    predicate readEx(NodeEx node1, ContentSet c, NodeEx node2) {
+      readSet(pragma[only_bind_into](node1.asNode()), c, pragma[only_bind_into](node2.asNode()))
+    }
+
     cached
     predicate storeSet(
       Node node1, ContentSet c, Node node2, DataFlowType contentType, DataFlowType containerType
@@ -1587,11 +1637,13 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
      * been stored into, in order to handle cases like `x.f1.f2 = y`.
      */
     cached
-    predicate store(
+    predicate storeEx(
-      Node node1, Content c, Node node2, DataFlowType contentType, DataFlowType containerType
+      NodeEx node1, Content c, NodeEx node2, DataFlowType contentType, DataFlowType containerType
     ) {
       exists(ContentSet cs |
-        c = cs.getAStoreContent() and storeSet(node1, cs, node2, contentType, containerType)
+        c = cs.getAStoreContent() and
+        storeSet(pragma[only_bind_into](node1.asNode()), cs, pragma[only_bind_into](node2.asNode()),
+          contentType, containerType)
       )
     }
 
@@ -1627,7 +1679,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     }
 
     cached
-    predicate allowParameterReturnInSelfCached(ParamNode p) { allowParameterReturnInSelf(p) }
+    predicate allowParameterReturnInSelfEx(ParamNodeEx p) { allowParameterReturnInSelf(p.asNode()) }
 
     cached
     predicate paramMustFlow(ParamNode p, ArgNode arg) { localMustFlowStep+(p, arg) }
@@ -1657,7 +1709,9 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
       string toString() { result = "Unreachable" }
 
       cached
-      predicate contains(Node n) { exists(NodeRegion nr | super.contains(nr) and nr.contains(n)) }
+      predicate contains(NodeEx n) {
+        exists(NodeRegion nr | super.contains(nr) and nr.contains(n.asNode()))
+      }
 
       cached
       DataFlowCallable getEnclosingCallable() {
@@ -2209,7 +2263,15 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
    * A `Node` at which a cast can occur such that the type should be checked.
    */
   class CastingNode extends NodeFinal {
-    CastingNode() { castingNode(this) }
+    CastingNode() {
+      this instanceof CastNode or
+      this instanceof ParamNode or
+      this instanceof OutNodeExt or
+      // For reads, `x.f`, we want to check that the tracked type after the read (which
+      // is obtained by popping the head of the access path stack) is compatible with
+      // the type of `x.f`.
+      readSet(_, _, this)
+    }
   }
 
   private predicate readStepWithTypes(
@@ -2266,7 +2328,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     }
 
     /** Holds if this call context makes `n` unreachable. */
-    predicate unreachable(Node n) { ns.contains(n) }
+    predicate unreachable(NodeEx n) { ns.contains(n) }
   }
 
   private DataFlowCallable getNodeRegionEnclosingCallable(NodeRegion nr) {
@@ -2307,6 +2369,16 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     OutNodeExt() { outNodeExt(this) }
   }
 
+  pragma[nomagic]
+  OutNodeExt getAnOutNodeExt(DataFlowCall call, ReturnKindExt k) {
+    result = getAnOutNode(call, k.(ValueReturnKind).getKind())
+    or
+    exists(ArgNode arg |
+      result.(PostUpdateNode).getPreUpdateNode() = arg and
+      arg.argumentOf(call, k.(ParamUpdateReturnKind).getAMatchingArgumentPosition())
+    )
+  }
+
   /**
    * An extended return kind. A return kind describes how data can be returned
    * from a callable. This can either be through a returned value or an updated
@@ -2317,7 +2389,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     abstract string toString();
 
     /** Gets a node corresponding to data flow out of `call`. */
-    final OutNodeExt getAnOutNode(DataFlowCall call) { result = getAnOutNodeExt(call, this) }
+    final OutNodeEx getAnOutNodeEx(DataFlowCall call) { result = getAnOutNodeEx(call, this) }
   }
 
   class ValueReturnKind extends ReturnKindExt, TValueReturn {

shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll

@@ -1355,7 +1355,7 @@ module Make<
         exists(DataFlowCall call, ReturnKindExt rk |
           result = summaryArgParam(call, arg, sc) and
           summaryReturnNodeExt(ret, pragma[only_bind_into](rk)) and
-          out = pragma[only_bind_into](rk).getAnOutNode(call)
+          out = getAnOutNodeExt(call, pragma[only_bind_into](rk))
         )
       }
 

shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll

@@ -464,8 +464,10 @@ module MakeModelGenerator<
     predicate isAdditionalFlowStep(
       DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
     ) {
-      exists(DataFlow::ContentSet c |
+      exists(DataFlow::NodeEx n1, DataFlow::NodeEx n2, DataFlow::ContentSet c |
-        DataFlow::store(node1, c.getAStoreContent(), node2, _, _) and
+        node1 = n1.asNode() and
+        node2 = n2.asNode() and
+        DataFlow::storeEx(n1, c.getAStoreContent(), n2, _, _) and
         isRelevantContent0(c) and
         (
           state1 instanceof TaintRead and state2.(TaintStore).getStep() = 1