Complete SQL injection barriers implementation with change notes

Co-authored-by: geoffw0 <40627776+geoffw0@users.noreply.github.com>

.github/copilot-instructions.md

@@ -1,4 +0,0 @@
-When reviewing code:
-* do not review changes in files with `.expected` extension (they are automatically ensured to be correct).
-* in `.ql` and `.qll` files, do not try to review the code itself as you don't understand the programming language
-  well enough to make comments in these languages. You can still check for typos or comment improvements.

.github/instructions/expected-files.instructions.md

@@ -0,0 +1,4 @@
+---
+applyTo: "**/*.expected"
+---
+Ignore these files. These are generated to be correct by running the tests, and should not be edited manually.

.github/instructions/ql-files.instructions.md

@@ -0,0 +1,6 @@
+---
+applyTo: "**/*.ql,**/*.qll"
+---
+When reviewing these QL files, do not make any comment on the code itself. You don't understand the QL programming language well enough yet. You can only make comments about the following aspects:
+* comments (lines starting with `//` or block comments enclosed in `/* ... */`): you can suggest improvements to the clarity of comments, or point out spelling mistakes
+* typos in identifiers

CODEOWNERS

@@ -1,3 +1,7 @@
+# Catch-all for anything which isn't matched by a line lower down
+* @github/code-scanning-alert-coverage
+
+# CodeQL language libraries
 /actions/ @github/codeql-dynamic
 /cpp/ @github/codeql-c-analysis
 /csharp/ @github/codeql-csharp
@@ -7,8 +11,10 @@
 /java/ @github/codeql-java
 /javascript/ @github/codeql-javascript
 /python/ @github/codeql-python
+/ql/ @github/codeql-ql-for-ql-reviewers
 /ruby/ @github/codeql-ruby
 /rust/ @github/codeql-rust
+/shared/ @github/codeql-shared-libraries-reviewers
 /swift/ @github/codeql-swift
 /misc/codegen/ @github/codeql-swift
 /java/kotlin-extractor/ @github/codeql-kotlin
@@ -25,9 +31,6 @@
 /docs/codeql/ql-language-reference/ @github/codeql-frontend-reviewers
 /docs/query-*-style-guide.md @github/codeql-analysis-reviewers
 
-# QL for QL reviewers
-/ql/ @github/codeql-ql-for-ql-reviewers
-
 # Bazel (excluding BUILD.bazel files)
 MODULE.bazel @github/codeql-ci-reviewers
 .bazelversion @github/codeql-ci-reviewers

Cargo.lock

@@ -84,9 +84,9 @@ dependencies = [
 
 [[package]]
 name = "anyhow"
-version = "1.0.99"
+version = "1.0.100"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b0674a1ddeecb70197781e945de4b3b8ffb61fa939a5597bcf48503737663100"
+checksum = "a23eb6b1614318a8071c9b2521f36b424b2c83db5eb3a0fead4a6c0809af6e61"
 
 [[package]]
 name = "argfile"
@@ -328,7 +328,7 @@ dependencies = [
  "chalk-derive 0.103.0",
  "chalk-ir 0.103.0",
  "ena",
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
  "itertools 0.12.1",
  "petgraph",
  "rustc-hash 1.1.0",
@@ -351,9 +351,9 @@ dependencies = [
 
 [[package]]
 name = "clap"
-version = "4.5.47"
+version = "4.5.48"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7eac00902d9d136acd712710d71823fb8ac8004ca445a89e73a41d45aa712931"
+checksum = "e2134bb3ea021b78629caa971416385309e0131b351b25e01dc16fb54e1b5fae"
 dependencies = [
  "clap_builder",
  "clap_derive",
@@ -361,9 +361,9 @@ dependencies = [
 
 [[package]]
 name = "clap_builder"
-version = "4.5.47"
+version = "4.5.48"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2ad9bbf750e73b5884fb8a211a9424a1906c1e156724260fdae972f31d70e1d6"
+checksum = "c2ba64afa3c0a6df7fa517765e31314e983f51dda798ffba27b988194fb65dc9"
 dependencies = [
  "anstream",
  "anstyle",
@@ -472,7 +472,7 @@ dependencies = [
  "serde",
  "serde_json",
  "serde_with",
- "toml 0.9.5",
+ "toml 0.9.7",
  "tracing",
  "tracing-flame",
  "tracing-subscriber",
@@ -557,9 +557,9 @@ checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28"
 
 [[package]]
 name = "darling"
-version = "0.20.11"
+version = "0.21.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "fc7f46116c46ff9ab3eb1597a45688b6715c6e628b5c133e288e709a29bcb4ee"
+checksum = "9cdf337090841a411e2a7f3deb9187445851f91b309c0c0a29e05f74a00a48c0"
 dependencies = [
  "darling_core",
  "darling_macro",
@@ -567,9 +567,9 @@ dependencies = [
 
 [[package]]
 name = "darling_core"
-version = "0.20.11"
+version = "0.21.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0d00b9596d185e565c2207a0b01f8bd1a135483d02d9b7b0a54b11da8d53412e"
+checksum = "1247195ecd7e3c85f83c8d2a366e4210d588e802133e1e355180a9870b517ea4"
 dependencies = [
  "fnv",
  "ident_case",
@@ -581,9 +581,9 @@ dependencies = [
 
 [[package]]
 name = "darling_macro"
-version = "0.20.11"
+version = "0.21.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "fc34b93ccb385b40dc71c6fceac4b2ad23662c7eeb248cf10d529b7e055b6ead"
+checksum = "d38308df82d1080de0afee5d069fa14b0326a88c14f15c5ccda35b4a6c414c81"
 dependencies = [
  "darling_core",
  "quote",
@@ -1059,13 +1059,14 @@ dependencies = [
 
 [[package]]
 name = "indexmap"
-version = "2.11.1"
+version = "2.11.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "206a8042aec68fa4a62e8d3f7aa4ceb508177d9324faf261e1959e495b7a1921"
+checksum = "4b0f83760fb341a774ed326568e19f5a863af4a952def8c39f9ab92fd95b88e5"
 dependencies = [
  "equivalent",
  "hashbrown 0.15.5",
  "serde",
+ "serde_core",
 ]
 
 [[package]]
@@ -1490,7 +1491,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "b4c5cc86750666a3ed20bdaf5ca2a0344f9c67674cae0515bec2da16fbaa47db"
 dependencies = [
  "fixedbitset",
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
 ]
 
 [[package]]
@@ -1559,9 +1560,9 @@ dependencies = [
 
 [[package]]
 name = "quote"
-version = "1.0.40"
+version = "1.0.41"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1885c039570dc00dcb4ff087a89e185fd56bae234ddc7f056a945bf36467248d"
+checksum = "ce25767e7b499d1b604768e7cde645d14cc8584231ea6b295e9c9eb22c02e1d1"
 dependencies = [
  "proc-macro2",
 ]
@@ -1666,7 +1667,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "e876bb2c3e52a8d4e6684526a2d4e81f9d028b939ee4dc5dc775fe10deb44d59"
 dependencies = [
  "dashmap",
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
  "la-arena",
  "ra_ap_cfg",
  "ra_ap_intern",
@@ -1708,7 +1709,7 @@ checksum = "ebffdc134eccabc17209d7760cfff7fd12ed18ab6e21188c5e084b97aa38504c"
 dependencies = [
  "arrayvec",
  "either",
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
  "itertools 0.14.0",
  "ra_ap_base_db",
  "ra_ap_cfg",
@@ -1738,7 +1739,7 @@ dependencies = [
  "drop_bomb",
  "either",
  "fst",
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
  "itertools 0.14.0",
  "la-arena",
  "ra-ap-rustc_abi",
@@ -1807,7 +1808,7 @@ dependencies = [
  "cov-mark",
  "either",
  "ena",
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
  "itertools 0.14.0",
  "la-arena",
  "oorandom",
@@ -1845,7 +1846,7 @@ dependencies = [
  "crossbeam-channel",
  "either",
  "fst",
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
  "itertools 0.14.0",
  "line-index",
  "memchr",
@@ -1947,7 +1948,7 @@ version = "0.0.301"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "45db9e2df587d56f0738afa89fb2c100ff7c1e9cbe49e07f6a8b62342832211b"
 dependencies = [
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
  "ra_ap_intern",
  "ra_ap_paths",
  "ra_ap_span",
@@ -2106,7 +2107,7 @@ checksum = "6c174d6b9b7a7f54687df7e00c3e75ed6f082a7943a9afb1d54f33c0c12773de"
 dependencies = [
  "crossbeam-channel",
  "fst",
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
  "nohash-hasher",
  "ra_ap_paths",
  "ra_ap_stdx",
@@ -2211,9 +2212,9 @@ dependencies = [
 
 [[package]]
 name = "regex"
-version = "1.11.2"
+version = "1.11.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "23d7fd106d8c02486a8d64e778353d1cffe08ce79ac2e82f540c86d0facf6912"
+checksum = "8b5288124840bee7b386bc413c487869b360b2b4ec421ea56425128692f2a82c"
 dependencies = [
  "aho-corasick",
  "memchr",
@@ -2223,9 +2224,9 @@ dependencies = [
 
 [[package]]
 name = "regex-automata"
-version = "0.4.10"
+version = "0.4.11"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6b9458fa0bfeeac22b5ca447c63aaf45f28439a709ccd244698632f9aa6394d6"
+checksum = "833eb9ce86d40ef33cb1306d8accf7bc8ec2bfea4355cbdebb3df68b40925cad"
 dependencies = [
  "aho-corasick",
  "memchr",
@@ -2316,7 +2317,7 @@ dependencies = [
  "crossbeam-utils",
  "hashbrown 0.15.5",
  "hashlink",
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
  "intrusive-collections",
  "papaya",
  "parking_lot",
@@ -2414,10 +2415,11 @@ dependencies = [
 
 [[package]]
 name = "serde"
-version = "1.0.219"
+version = "1.0.228"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5f0e2c6ed6606019b4e29e69dbaba95b11854410e5347d525002456dbbb786b6"
+checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e"
 dependencies = [
+ "serde_core",
  "serde_derive",
 ]
 
@@ -2443,10 +2445,19 @@ dependencies = [
 ]
 
 [[package]]
-name = "serde_derive"
-version = "1.0.219"
+name = "serde_core"
+version = "1.0.228"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5b0276cf7f2c73365f7157c8123c21cd9a50fbbd844757af28ca1f5925fc2a00"
+checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad"
+dependencies = [
+ "serde_derive",
+]
+
+[[package]]
+name = "serde_derive"
+version = "1.0.228"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -2455,15 +2466,16 @@ dependencies = [
 
 [[package]]
 name = "serde_json"
-version = "1.0.143"
+version = "1.0.145"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d401abef1d108fbd9cbaebc3e46611f4b1021f714a0597a71f41ee463f5f4a5a"
+checksum = "402a6f66d8c709116cf22f558eab210f5a50187f702eb4d7e5ef38d9a7f1c79c"
 dependencies = [
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
  "itoa",
  "memchr",
  "ryu",
  "serde",
+ "serde_core",
 ]
 
 [[package]]
@@ -2477,24 +2489,24 @@ dependencies = [
 
 [[package]]
 name = "serde_spanned"
-version = "1.0.0"
+version = "1.0.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "40734c41988f7306bb04f0ecf60ec0f3f1caa34290e4e8ea471dcd3346483b83"
+checksum = "5417783452c2be558477e104686f7de5dae53dba813c28435e0e70f82d9b04ee"
 dependencies = [
- "serde",
+ "serde_core",
 ]
 
 [[package]]
 name = "serde_with"
-version = "3.14.0"
+version = "3.14.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f2c45cd61fefa9db6f254525d46e392b852e0e61d9a1fd36e5bd183450a556d5"
+checksum = "c522100790450cf78eeac1507263d0a350d4d5b30df0c8e1fe051a10c22b376e"
 dependencies = [
  "base64",
  "chrono",
  "hex",
  "indexmap 1.9.3",
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
  "schemars 0.9.0",
  "schemars 1.0.4",
  "serde",
@@ -2506,9 +2518,9 @@ dependencies = [
 
 [[package]]
 name = "serde_with_macros"
-version = "3.14.0"
+version = "3.14.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "de90945e6565ce0d9a25098082ed4ee4002e047cb59892c318d66821e14bb30f"
+checksum = "327ada00f7d64abaac1e55a6911e90cf665aa051b9a561c7006c157f4633135e"
 dependencies = [
  "darling",
  "proc-macro2",
@@ -2522,7 +2534,7 @@ version = "0.9.34+deprecated"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "6a8b1a1a2ebf674015cc02edccce75287f1a0130d394307b36743c2f5d504b47"
 dependencies = [
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
  "itoa",
  "ryu",
  "serde",
@@ -2701,14 +2713,14 @@ dependencies = [
 
 [[package]]
 name = "toml"
-version = "0.9.5"
+version = "0.9.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "75129e1dc5000bfbaa9fee9d1b21f974f9fbad9daec557a521ee6e080825f6e8"
+checksum = "00e5e5d9bf2475ac9d4f0d9edab68cc573dc2fd644b0dba36b0c30a92dd9eaa0"
 dependencies = [
- "indexmap 2.11.1",
- "serde",
- "serde_spanned 1.0.0",
- "toml_datetime 0.7.0",
+ "indexmap 2.11.4",
+ "serde_core",
+ "serde_spanned 1.0.2",
+ "toml_datetime 0.7.2",
  "toml_parser",
  "toml_writer",
  "winnow",
@@ -2725,11 +2737,11 @@ dependencies = [
 
 [[package]]
 name = "toml_datetime"
-version = "0.7.0"
+version = "0.7.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "bade1c3e902f58d73d3f294cd7f20391c1cb2fbcb643b73566bc773971df91e3"
+checksum = "32f1085dec27c2b6632b04c80b3bb1b4300d6495d1e129693bdda7d91e72eec1"
 dependencies = [
- "serde",
+ "serde_core",
 ]
 
 [[package]]
@@ -2738,7 +2750,7 @@ version = "0.22.27"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "41fe8c660ae4257887cf66394862d21dbca4a6ddd26f04a3560410406a2f819a"
 dependencies = [
- "indexmap 2.11.1",
+ "indexmap 2.11.4",
  "serde",
  "serde_spanned 0.6.9",
  "toml_datetime 0.6.11",
@@ -2748,9 +2760,9 @@ dependencies = [
 
 [[package]]
 name = "toml_parser"
-version = "1.0.2"
+version = "1.0.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b551886f449aa90d4fe2bdaa9f4a2577ad2dde302c61ecf262d80b116db95c10"
+checksum = "4cf893c33be71572e0e9aa6dd15e6677937abd686b066eac3f8cd3531688a627"
 dependencies = [
  "winnow",
 ]
@@ -2763,9 +2775,9 @@ checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801"
 
 [[package]]
 name = "toml_writer"
-version = "1.0.2"
+version = "1.0.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "fcc842091f2def52017664b53082ecbbeb5c7731092bad69d2c63050401dfd64"
+checksum = "d163a63c116ce562a22cda521fcc4d79152e7aba014456fb5eb442f6d6a10109"
 
 [[package]]
 name = "tracing"
@@ -2855,9 +2867,9 @@ dependencies = [
 
 [[package]]
 name = "tree-sitter-embedded-template"
-version = "0.23.2"
+version = "0.25.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "790063ef14e5b67556abc0b3be0ed863fb41d65ee791cf8c0b20eb42a1fa46af"
+checksum = "833d528e8fcb4e49ddb04d4d6450ddb8ac08f282a58fec94ce981c9c5dbf7e3a"
 dependencies = [
  "cc",
  "tree-sitter-language",

Cargo.toml

@@ -10,4 +10,3 @@ members = [
     "rust/ast-generator",
     "rust/autobuild",
 ]
-exclude = ["mad-generation-build"]

MODULE.bazel

@@ -19,8 +19,8 @@ bazel_dep(name = "rules_go", version = "0.56.1")
 bazel_dep(name = "rules_pkg", version = "1.0.1")
 bazel_dep(name = "rules_nodejs", version = "6.2.0-codeql.1")
 bazel_dep(name = "rules_python", version = "0.40.0")
-bazel_dep(name = "rules_shell", version = "0.3.0")
-bazel_dep(name = "bazel_skylib", version = "1.7.1")
+bazel_dep(name = "rules_shell", version = "0.5.0")
+bazel_dep(name = "bazel_skylib", version = "1.8.1")
 bazel_dep(name = "abseil-cpp", version = "20240116.1", repo_name = "absl")
 bazel_dep(name = "nlohmann_json", version = "3.11.3", repo_name = "json")
 bazel_dep(name = "fmt", version = "10.0.0")
@@ -28,7 +28,7 @@ bazel_dep(name = "rules_kotlin", version = "2.1.3-codeql.1")
 bazel_dep(name = "gazelle", version = "0.40.0")
 bazel_dep(name = "rules_dotnet", version = "0.19.2-codeql.1")
 bazel_dep(name = "googletest", version = "1.14.0.bcr.1")
-bazel_dep(name = "rules_rust", version = "0.63.0")
+bazel_dep(name = "rules_rust", version = "0.66.0")
 bazel_dep(name = "zstd", version = "1.5.5.bcr.1")
 
 bazel_dep(name = "buildifier_prebuilt", version = "6.4.0", dev_dependency = True)
@@ -98,11 +98,11 @@ use_repo(
 tree_sitter_extractors_deps = use_extension("//misc/bazel/3rdparty:tree_sitter_extractors_extension.bzl", "r")
 use_repo(
     tree_sitter_extractors_deps,
-    "vendor_ts__anyhow-1.0.99",
+    "vendor_ts__anyhow-1.0.100",
     "vendor_ts__argfile-0.2.1",
     "vendor_ts__chalk-ir-0.104.0",
     "vendor_ts__chrono-0.4.42",
-    "vendor_ts__clap-4.5.47",
+    "vendor_ts__clap-4.5.48",
     "vendor_ts__dunce-1.0.5",
     "vendor_ts__either-1.15.0",
     "vendor_ts__encoding-0.2.33",
@@ -116,7 +116,7 @@ use_repo(
     "vendor_ts__num-traits-0.2.19",
     "vendor_ts__num_cpus-1.17.0",
     "vendor_ts__proc-macro2-1.0.101",
-    "vendor_ts__quote-1.0.40",
+    "vendor_ts__quote-1.0.41",
     "vendor_ts__ra_ap_base_db-0.0.301",
     "vendor_ts__ra_ap_cfg-0.0.301",
     "vendor_ts__ra_ap_hir-0.0.301",
@@ -135,17 +135,17 @@ use_repo(
     "vendor_ts__ra_ap_vfs-0.0.301",
     "vendor_ts__rand-0.9.2",
     "vendor_ts__rayon-1.11.0",
-    "vendor_ts__regex-1.11.2",
-    "vendor_ts__serde-1.0.219",
-    "vendor_ts__serde_json-1.0.143",
-    "vendor_ts__serde_with-3.14.0",
+    "vendor_ts__regex-1.11.3",
+    "vendor_ts__serde-1.0.228",
+    "vendor_ts__serde_json-1.0.145",
+    "vendor_ts__serde_with-3.14.1",
     "vendor_ts__syn-2.0.106",
-    "vendor_ts__toml-0.9.5",
+    "vendor_ts__toml-0.9.7",
     "vendor_ts__tracing-0.1.41",
     "vendor_ts__tracing-flame-0.2.0",
     "vendor_ts__tracing-subscriber-0.3.20",
     "vendor_ts__tree-sitter-0.25.9",
-    "vendor_ts__tree-sitter-embedded-template-0.23.2",
+    "vendor_ts__tree-sitter-embedded-template-0.25.0",
     "vendor_ts__tree-sitter-json-0.24.8",
     "vendor_ts__tree-sitter-ql-0.23.1",
     "vendor_ts__tree-sitter-ruby-0.23.1",

actions/ql/lib/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.4.19
+
+No user-facing changes.
+
 ## 0.4.18
 
 No user-facing changes.

actions/ql/lib/change-notes/released/0.4.19.md

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

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.4.18
+lastReleaseVersion: 0.4.19

actions/ql/lib/codeql/actions/security/ArgumentInjectionQuery.qll

@@ -100,8 +100,6 @@ private module ArgumentInjectionConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     result = sink.getLocation()
     or

actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll

@@ -333,8 +333,6 @@ private module ArtifactPoisoningConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     result = sink.getLocation()
     or

actions/ql/lib/codeql/actions/security/CodeInjectionQuery.qll

@@ -80,8 +80,6 @@ private module CodeInjectionConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     result = sink.getLocation()
     or

actions/ql/lib/codeql/actions/security/EnvPathInjectionQuery.qll

@@ -130,8 +130,6 @@ private module EnvPathInjectionConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     result = sink.getLocation()
     or

actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll

@@ -184,8 +184,6 @@ private module EnvVarInjectionConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     result = sink.getLocation()
     or

actions/ql/lib/codeql/actions/security/OutputClobberingQuery.qll

@@ -212,8 +212,6 @@ private module OutputClobberingConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 /** Tracks flow of unsafe user input that is used to construct and evaluate an environment variable. */

actions/ql/lib/codeql/actions/security/RequestForgeryQuery.qll

@@ -18,8 +18,6 @@ private module RequestForgeryConfig implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { sink instanceof RequestForgerySink }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 /** Tracks flow of unsafe user input that is used to construct and evaluate a system command. */

actions/ql/lib/codeql/actions/security/SecretExfiltrationQuery.qll

@@ -17,8 +17,6 @@ private module SecretExfiltrationConfig implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { sink instanceof SecretExfiltrationSink }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 /** Tracks flow of unsafe user input that is used in a context where it may lead to a secret exfiltration. */

actions/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/actions-all
-version: 0.4.18
+version: 0.4.20-dev
 library: true
 warnOnImplicitThis: true
 dependencies:

actions/ql/src/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.6.11
+
+No user-facing changes.
+
 ## 0.6.10
 
 No user-facing changes.

actions/ql/src/Models/CompositeActionsSinks.ql

@@ -26,8 +26,6 @@ private module MyConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;

actions/ql/src/Models/CompositeActionsSources.ql

@@ -36,8 +36,6 @@ private module MyConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;

actions/ql/src/Models/CompositeActionsSummaries.ql

@@ -27,8 +27,6 @@ private module MyConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;

actions/ql/src/Models/ReusableWorkflowsSinks.ql

@@ -26,8 +26,6 @@ private module MyConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;

actions/ql/src/Models/ReusableWorkflowsSources.ql

@@ -36,8 +36,6 @@ private module MyConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;

actions/ql/src/Models/ReusableWorkflowsSummaries.ql

@@ -27,8 +27,6 @@ private module MyConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;

actions/ql/src/change-notes/released/0.6.11.md

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

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.6.10
+lastReleaseVersion: 0.6.11

actions/ql/src/experimental/Security/CWE-200/SecretExfiltration.ql

@@ -19,5 +19,5 @@ import SecretExfiltrationFlow::PathGraph
 from SecretExfiltrationFlow::PathNode source, SecretExfiltrationFlow::PathNode sink
 where SecretExfiltrationFlow::flowPath(source, sink)
 select sink.getNode(), source, sink,
-  "Potential secret exfiltration in $@, which may be be leaked to an attacker-controlled resource.",
+  "Potential secret exfiltration in $@, which may be leaked to an attacker-controlled resource.",
   sink, sink.getNode().asExpr().(Expression).getRawExpression()

actions/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/actions-queries
-version: 0.6.10
+version: 0.6.12-dev
 library: false
 warnOnImplicitThis: true
 groups: [actions, queries]

actions/ql/test/query-tests/Security/CWE-200/SecretExfiltration.expected

@@ -3,4 +3,4 @@ nodes
 | .github/workflows/test1.yml:15:11:16:75 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
 subpaths
 #select
-| .github/workflows/test1.yml:15:11:16:75 | github.event.pull_request.title | .github/workflows/test1.yml:15:11:16:75 | github.event.pull_request.title | .github/workflows/test1.yml:15:11:16:75 | github.event.pull_request.title | Potential secret exfiltration in $@, which may be be leaked to an attacker-controlled resource. | .github/workflows/test1.yml:15:11:16:75 | github.event.pull_request.title | ${{ github.event.pull_request.title }} |
+| .github/workflows/test1.yml:15:11:16:75 | github.event.pull_request.title | .github/workflows/test1.yml:15:11:16:75 | github.event.pull_request.title | .github/workflows/test1.yml:15:11:16:75 | github.event.pull_request.title | Potential secret exfiltration in $@, which may be leaked to an attacker-controlled resource. | .github/workflows/test1.yml:15:11:16:75 | github.event.pull_request.title | ${{ github.event.pull_request.title }} |

config/dbscheme-fragments.json

@@ -9,6 +9,7 @@
   "fragments": [
     "/*- Compilations -*/",
     "/*- External data -*/",
+    "/*- Overlay support -*/",
     "/*- Files and folders -*/",
     "/*- Diagnostic messages -*/",
     "/*- Diagnostic messages: severity -*/",

cpp/ql/lib/CHANGELOG.md

@@ -1,3 +1,13 @@
+## 6.0.0
+
+### Breaking Changes
+
+* The "Guards" libraries (`semmle.code.cpp.controlflow.Guards` and `semmle.code.cpp.controlflow.IRGuards`) have been totally rewritten to recognize many more guards. The API remains unchanged, but the `GuardCondition` class now extends `Element` instead of `Expr`.
+
+### New Features
+
+* C/C++ `build-mode: none` support is now generally available.
+
 ## 5.6.1
 
 No user-facing changes.

cpp/ql/lib/change-notes/2025-10-16-range-analysis-performance.md

@@ -0,0 +1,4 @@
+---
+category: fix
+---
+* Improve performance of the range analysis in cases where it would otherwise take an exorbitant amount of time.

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

@@ -0,0 +1,9 @@
+## 6.0.0
+
+### Breaking Changes
+
+* The "Guards" libraries (`semmle.code.cpp.controlflow.Guards` and `semmle.code.cpp.controlflow.IRGuards`) have been totally rewritten to recognize many more guards. The API remains unchanged, but the `GuardCondition` class now extends `Element` instead of `Expr`.
+
+### New Features
+
+* C/C++ `build-mode: none` support is now generally available.

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 5.6.1
+lastReleaseVersion: 6.0.0

cpp/ql/lib/experimental/quantum/Language.qll

@@ -14,8 +14,8 @@ module CryptoInput implements InputSig<Language::Location> {
     result = node.asExpr() or
     result = node.asParameter() or
     result = node.asVariable() or
-    result = node.asDefiningArgument()
-    // TODO: do we need asIndirectExpr()?
+    result = node.asDefiningArgument() or
+    result = node.asIndirectExpr()
   }
 
   string locationToFileBaseNameAndLineNumberString(Location location) {
@@ -53,7 +53,7 @@ module ArtifactFlowConfig implements DataFlow::ConfigSig {
   }
 }
 
-module ArtifactFlow = DataFlow::Global<ArtifactFlowConfig>;
+module ArtifactFlow = TaintTracking::Global<ArtifactFlowConfig>;
 
 /**
  * An artifact output to node input configuration
@@ -93,7 +93,13 @@ module GenericDataSourceFlow = TaintTracking::Global<GenericDataSourceFlowConfig
 
 private class ConstantDataSource extends Crypto::GenericConstantSourceInstance instanceof OpenSslGenericSourceCandidateLiteral
 {
-  override DataFlow::Node getOutputNode() { result.asExpr() = this }
+  override DataFlow::Node getOutputNode() {
+    // OpenSSL algorithms may be referenced either by string name or by numeric ID:
+    // String names (e.g. "AES-256-CBC") appear in the AST as character pointer
+    // literals. For these we must use `asIndirectExpr`. Numeric IDs (e.g. NID_aes_256_cbc)
+    // appear as integer literals. For these, we must use `asExpr` to get the "value" node.
+    [result.asIndirectExpr(), result.asExpr()] = this
+  }
 
   override predicate flowsTo(Crypto::FlowAwareElement other) {
     // TODO: separate config to avoid blowing up data-flow analysis
@@ -103,28 +109,4 @@ private class ConstantDataSource extends Crypto::GenericConstantSourceInstance i
   override string getAdditionalDescription() { result = this.toString() }
 }
 
-module ArtifactUniversalFlowConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) {
-    source = any(Crypto::ArtifactInstance artifact).getOutputNode()
-  }
-
-  predicate isSink(DataFlow::Node sink) {
-    sink = any(Crypto::FlowAwareElement other).getInputNode()
-  }
-
-  predicate isBarrierOut(DataFlow::Node node) {
-    node = any(Crypto::FlowAwareElement element).getInputNode()
-  }
-
-  predicate isBarrierIn(DataFlow::Node node) {
-    node = any(Crypto::FlowAwareElement element).getOutputNode()
-  }
-
-  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-    node1.(AdditionalFlowInputStep).getOutput() = node2
-  }
-}
-
-module ArtifactUniversalFlow = DataFlow::Global<ArtifactUniversalFlowConfig>;
-
 import OpenSSL.OpenSSL

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/AlgToAVCFlow.qll

@@ -14,9 +14,13 @@ private import PaddingAlgorithmInstance
  */
 module KnownOpenSslAlgorithmToAlgorithmValueConsumerConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) {
-    source.asExpr() instanceof KnownOpenSslAlgorithmExpr and
+    (
+      source.asExpr() instanceof KnownOpenSslAlgorithmExpr or
+      source.asIndirectExpr() instanceof KnownOpenSslAlgorithmExpr
+    ) and
     // No need to flow direct operations to AVCs
-    not source.asExpr() instanceof OpenSslDirectAlgorithmOperationCall
+    not source.asExpr() instanceof OpenSslDirectAlgorithmOperationCall and
+    not source.asIndirectExpr() instanceof OpenSslDirectAlgorithmOperationCall
   }
 
   predicate isSink(DataFlow::Node sink) {
@@ -46,10 +50,12 @@ module KnownOpenSslAlgorithmToAlgorithmValueConsumerConfig implements DataFlow::
 }
 
 module KnownOpenSslAlgorithmToAlgorithmValueConsumerFlow =
-  DataFlow::Global<KnownOpenSslAlgorithmToAlgorithmValueConsumerConfig>;
+  TaintTracking::Global<KnownOpenSslAlgorithmToAlgorithmValueConsumerConfig>;
 
 module RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) { source.asExpr() instanceof OpenSslPaddingLiteral }
+  predicate isSource(DataFlow::Node source) {
+    source.asExpr() instanceof OpenSslSpecialPaddingLiteral
+  }
 
   predicate isSink(DataFlow::Node sink) {
     exists(PaddingAlgorithmValueConsumer c | c.getInputNode() = sink)
@@ -61,7 +67,7 @@ module RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig implements DataF
 }
 
 module RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerFlow =
-  DataFlow::Global<RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig>;
+  TaintTracking::Global<RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig>;
 
 class OpenSslAlgorithmAdditionalFlowStep extends AdditionalFlowInputStep {
   OpenSslAlgorithmAdditionalFlowStep() { exists(AlgorithmPassthroughCall c | c.getInNode() = this) }

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/BlockAlgorithmInstance.qll

@@ -53,7 +53,8 @@ class KnownOpenSslBlockModeConstantAlgorithmInstance extends OpenSslAlgorithmIns
       // Sink is an argument to a CipherGetterCall
       sink = getterCall.getInputNode() and
       // Source is `this`
-      src.asExpr() = this and
+      // NOTE: src literals can be ints or strings, so need to consider asExpr and asIndirectExpr
+      this = [src.asExpr(), src.asIndirectExpr()] and
       // This traces to a getter
       KnownOpenSslAlgorithmToAlgorithmValueConsumerFlow::flow(src, sink)
     )

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/CipherAlgorithmInstance.qll

@@ -2,12 +2,10 @@ import cpp
 private import experimental.quantum.Language
 private import KnownAlgorithmConstants
 private import Crypto::KeyOpAlg as KeyOpAlg
-private import OpenSSLAlgorithmInstanceBase
-private import PaddingAlgorithmInstance
-private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
-private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
+private import experimental.quantum.OpenSSL.Operations.OpenSSLOperationBase
+private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
+private import OpenSSLAlgorithmInstances
 private import AlgToAVCFlow
-private import BlockAlgorithmInstance
 
 /**
  * Given a `KnownOpenSslCipherAlgorithmExpr`, converts this to a cipher family type.
@@ -79,7 +77,8 @@ class KnownOpenSslCipherConstantAlgorithmInstance extends OpenSslAlgorithmInstan
       // Sink is an argument to a CipherGetterCall
       sink = getterCall.getInputNode() and
       // Source is `this`
-      src.asExpr() = this and
+      // NOTE: src literals can be ints or strings, so need to consider asExpr and asIndirectExpr
+      this = [src.asExpr(), src.asIndirectExpr()] and
       // This traces to a getter
       KnownOpenSslAlgorithmToAlgorithmValueConsumerFlow::flow(src, sink)
     )
@@ -97,10 +96,13 @@ class KnownOpenSslCipherConstantAlgorithmInstance extends OpenSslAlgorithmInstan
   }
 
   override Crypto::PaddingAlgorithmInstance getPaddingAlgorithm() {
-    //TODO: the padding is either self, or it flows through getter ctx to a set padding call
-    // like EVP_PKEY_CTX_set_rsa_padding
     result = this
-    // TODO or trace through getter ctx to set padding
+    or
+    exists(OperationStep s |
+      this.getAvc().(AvcContextCreationStep).flowsToOperationStep(s) and
+      s.getAlgorithmValueConsumerForInput(PaddingAlgorithmIO()) =
+        result.(OpenSslAlgorithmInstance).getAvc()
+    )
   }
 
   override string getRawAlgorithmName() {
@@ -117,7 +119,7 @@ class KnownOpenSslCipherConstantAlgorithmInstance extends OpenSslAlgorithmInstan
     knownOpenSslConstantToCipherFamilyType(this, result)
     or
     not knownOpenSslConstantToCipherFamilyType(this, _) and
-    result = Crypto::KeyOpAlg::TUnknownKeyOperationAlgorithmType()
+    result = Crypto::KeyOpAlg::TOtherKeyOperationAlgorithmType()
   }
 
   override OpenSslAlgorithmValueConsumer getAvc() { result = getterCall }

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/EllipticCurveAlgorithmInstance.qll

@@ -21,7 +21,8 @@ class KnownOpenSslEllipticCurveConstantAlgorithmInstance extends OpenSslAlgorith
       // Sink is an argument to a CipherGetterCall
       sink = getterCall.getInputNode() and
       // Source is `this`
-      src.asExpr() = this and
+      // NOTE: src literals can be ints or strings, so need to consider asExpr and asIndirectExpr
+      this = [src.asExpr(), src.asIndirectExpr()] and
       // This traces to a getter
       KnownOpenSslAlgorithmToAlgorithmValueConsumerFlow::flow(src, sink)
     )
@@ -39,7 +40,7 @@ class KnownOpenSslEllipticCurveConstantAlgorithmInstance extends OpenSslAlgorith
     result = this.(Call).getTarget().getName()
   }
 
-  override Crypto::EllipticCurveFamilyType getEllipticCurveFamilyType() {
+  override Crypto::EllipticCurveType getEllipticCurveType() {
     if
       Crypto::ellipticCurveNameToKnownKeySizeAndFamilyMapping(this.getParsedEllipticCurveName(), _,
         _)

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/HashAlgorithmInstance.qll

@@ -59,7 +59,8 @@ class KnownOpenSslHashConstantAlgorithmInstance extends OpenSslAlgorithmInstance
       // Sink is an argument to a CipherGetterCall
       sink = getterCall.getInputNode() and
       // Source is `this`
-      src.asExpr() = this and
+      // NOTE: src literals can be ints or strings, so need to consider asExpr and asIndirectExpr
+      this = [src.asExpr(), src.asIndirectExpr()] and
       // This traces to a getter
       KnownOpenSslAlgorithmToAlgorithmValueConsumerFlow::flow(src, sink)
     )
@@ -71,7 +72,7 @@ class KnownOpenSslHashConstantAlgorithmInstance extends OpenSslAlgorithmInstance
 
   override OpenSslAlgorithmValueConsumer getAvc() { result = getterCall }
 
-  override Crypto::THashType getHashFamily() {
+  override Crypto::THashType getHashType() {
     knownOpenSslConstantToHashFamilyType(this, result)
     or
     not knownOpenSslConstantToHashFamilyType(this, _) and result = Crypto::OtherHashType()

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/KeyAgreementAlgorithmInstance.qll

@@ -37,7 +37,8 @@ class KnownOpenSslKeyAgreementConstantAlgorithmInstance extends OpenSslAlgorithm
       // Sink is an argument to a CipherGetterCall
       sink = getterCall.getInputNode() and
       // Source is `this`
-      src.asExpr() = this and
+      // NOTE: src literals can be ints or strings, so need to consider asExpr and asIndirectExpr
+      this = [src.asExpr(), src.asIndirectExpr()] and
       // This traces to a getter
       KnownOpenSslAlgorithmToAlgorithmValueConsumerFlow::flow(src, sink)
     )

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/KnownAlgorithmConstants.qll

@@ -171,9 +171,15 @@ class KnownOpenSslKeyAgreementAlgorithmExpr extends Expr instanceof KnownOpenSsl
 }
 
 predicate knownOpenSslAlgorithmOperationCall(Call c, string normalized, string algType) {
-  c.getTarget().getName() in ["EVP_RSA_gen", "RSA_generate_key_ex", "RSA_generate_key", "RSA_new"] and
+  c.getTarget().getName() in [
+      "EVP_RSA_gen", "RSA_generate_key_ex", "RSA_generate_key", "RSA_new", "RSA_sign", "RSA_verify"
+    ] and
   normalized = "RSA" and
   algType = "ASYMMETRIC_ENCRYPTION"
+  or
+  c.getTarget().getName() in ["DSA_do_sign", "DSA_do_verify"] and
+  normalized = "DSA" and
+  algType = "SIGNATURE"
 }
 
 /**

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/MACAlgorithmInstance.qll

@@ -2,12 +2,13 @@ import cpp
 private import experimental.quantum.Language
 private import KnownAlgorithmConstants
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
-private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstanceBase
+private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstances
 private import experimental.quantum.OpenSSL.Operations.OpenSSLOperations
+private import Crypto::KeyOpAlg as KeyOpAlg
 private import AlgToAVCFlow
 
 class KnownOpenSslMacConstantAlgorithmInstance extends OpenSslAlgorithmInstance,
-  Crypto::MacAlgorithmInstance instanceof KnownOpenSslMacAlgorithmExpr
+  Crypto::KeyOperationAlgorithmInstance instanceof KnownOpenSslMacAlgorithmExpr
 {
   OpenSslAlgorithmValueConsumer getterCall;
 
@@ -21,7 +22,8 @@ class KnownOpenSslMacConstantAlgorithmInstance extends OpenSslAlgorithmInstance,
       // Sink is an argument to a CipherGetterCall
       sink = getterCall.getInputNode() and
       // Source is `this`
-      src.asExpr() = this and
+      // NOTE: src literals can be ints or strings, so need to consider asExpr and asIndirectExpr
+      this = [src.asExpr(), src.asIndirectExpr()] and
       // This traces to a getter
       KnownOpenSslAlgorithmToAlgorithmValueConsumerFlow::flow(src, sink)
     )
@@ -33,17 +35,34 @@ class KnownOpenSslMacConstantAlgorithmInstance extends OpenSslAlgorithmInstance,
 
   override OpenSslAlgorithmValueConsumer getAvc() { result = getterCall }
 
-  override string getRawMacAlgorithmName() {
+  override string getRawAlgorithmName() {
     result = this.(Literal).getValue().toString()
     or
     result = this.(Call).getTarget().getName()
   }
 
-  override Crypto::MacType getMacType() {
-    this instanceof KnownOpenSslHMacAlgorithmExpr and result = Crypto::HMAC()
-    or
-    this instanceof KnownOpenSslCMacAlgorithmExpr and result = Crypto::CMAC()
+  override Crypto::KeyOpAlg::AlgorithmType getAlgorithmType() {
+    if this instanceof KnownOpenSslHMacAlgorithmExpr
+    then result = KeyOpAlg::TMac(KeyOpAlg::HMAC())
+    else
+      if this instanceof KnownOpenSslCMacAlgorithmExpr
+      then result = KeyOpAlg::TMac(KeyOpAlg::CMAC())
+      else result = KeyOpAlg::TMac(KeyOpAlg::OtherMacAlgorithmType())
   }
+
+  override Crypto::ConsumerInputDataFlowNode getKeySizeConsumer() {
+    // TODO: trace to any key size initializer?
+    none()
+  }
+
+  override int getKeySizeFixed() {
+    // TODO: are there known fixed key sizes to consider?
+    none()
+  }
+
+  override Crypto::ModeOfOperationAlgorithmInstance getModeOfOperationAlgorithm() { none() }
+
+  override Crypto::PaddingAlgorithmInstance getPaddingAlgorithm() { none() }
 }
 
 class KnownOpenSslHMacConstantAlgorithmInstance extends Crypto::HmacAlgorithmInstance,
@@ -60,9 +79,13 @@ class KnownOpenSslHMacConstantAlgorithmInstance extends Crypto::HmacAlgorithmIns
       // where the current AVC traces to a HashAlgorithmIO consuming operation step.
       // TODO: need to consider getting reset values, tracing down to the first set for now
       exists(OperationStep s, AvcContextCreationStep avc |
-        avc = this.getAvc() and
+        avc = super.getAvc() and
         avc.flowsToOperationStep(s) and
         s.getAlgorithmValueConsumerForInput(HashAlgorithmIO()) = result
       )
   }
+
+  override Crypto::ModeOfOperationAlgorithmInstance getModeOfOperationAlgorithm() { none() }
+
+  override Crypto::PaddingAlgorithmInstance getPaddingAlgorithm() { none() }
 }

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/PaddingAlgorithmInstance.qll

@@ -1,10 +1,10 @@
 import cpp
 private import experimental.quantum.Language
 private import OpenSSLAlgorithmInstanceBase
+private import experimental.quantum.OpenSSL.Operations.OpenSSLOperationBase
 private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
 private import AlgToAVCFlow
-private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
-private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
+private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
 private import codeql.quantum.experimental.Standardization::Types::KeyOpAlg as KeyOpAlg
 
 /**
@@ -18,13 +18,14 @@ private import codeql.quantum.experimental.Standardization::Types::KeyOpAlg as K
  *     # define RSA_PKCS1_WITH_TLS_PADDING 7
  *     # define RSA_PKCS1_NO_IMPLICIT_REJECT_PADDING 8
  */
-class OpenSslPaddingLiteral extends Literal {
+class OpenSslSpecialPaddingLiteral extends Literal {
   // TODO: we can be more specific about where the literal is in a larger expression
   // to avoid literals that are clealy not representing an algorithm, e.g., array indices.
-  OpenSslPaddingLiteral() { this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8] }
+  OpenSslSpecialPaddingLiteral() { this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8] }
 }
 
 /**
+ * Holds if `e` has the given `type`.
  * Given a `KnownOpenSslPaddingAlgorithmExpr`, converts this to a padding family type.
  * Does not bind if there is no mapping (no mapping to 'unknown' or 'other').
  */
@@ -45,9 +46,6 @@ predicate knownOpenSslConstantToPaddingFamilyType(
   )
 }
 
-//abstract class OpenSslPaddingAlgorithmInstance extends OpenSslAlgorithmInstance, Crypto::PaddingAlgorithmInstance{}
-// TODO: need to alter this to include known padding constants which don't have the
-// same mechanics as those with known nids
 class KnownOpenSslPaddingConstantAlgorithmInstance extends OpenSslAlgorithmInstance,
   Crypto::PaddingAlgorithmInstance instanceof Expr
 {
@@ -66,7 +64,8 @@ class KnownOpenSslPaddingConstantAlgorithmInstance extends OpenSslAlgorithmInsta
       // Sink is an argument to a CipherGetterCall
       sink = getterCall.getInputNode() and
       // Source is `this`
-      src.asExpr() = this and
+      // NOTE: src literals can be ints or strings, so need to consider asExpr and asIndirectExpr
+      this = [src.asExpr(), src.asIndirectExpr()] and
       // This traces to a getter
       KnownOpenSslAlgorithmToAlgorithmValueConsumerFlow::flow(src, sink) and
       isPaddingSpecificConsumer = false
@@ -79,12 +78,13 @@ class KnownOpenSslPaddingConstantAlgorithmInstance extends OpenSslAlgorithmInsta
     isPaddingSpecificConsumer = false
     or
     // Possibility 3: padding-specific literal
-    this instanceof OpenSslPaddingLiteral and
+    this instanceof OpenSslSpecialPaddingLiteral and
     exists(DataFlow::Node src, DataFlow::Node sink |
       // Sink is an argument to a CipherGetterCall
       sink = getterCall.getInputNode() and
       // Source is `this`
-      src.asExpr() = this and
+      // NOTE: src literals can be ints or strings, so need to consider asExpr and asIndirectExpr
+      this = [src.asExpr(), src.asIndirectExpr()] and
       // This traces to a padding-specific consumer
       RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerFlow::flow(src, sink)
     ) and
@@ -124,44 +124,6 @@ class KnownOpenSslPaddingConstantAlgorithmInstance extends OpenSslAlgorithmInsta
   }
 }
 
-// // Values used for EVP_PKEY_CTX_set_rsa_padding, these are
-// // not the same as 'typical' constants found in the set of known algorithm constants
-// // they do not have an NID
-// // TODO: what about setting the padding directly?
-// class KnownRSAPaddingConstant extends OpenSslPaddingAlgorithmInstance, Crypto::PaddingAlgorithmInstance instanceof Literal
-// {
-//   KnownRSAPaddingConstant() {
-//     // from rsa.h in openssl:
-//     // # define RSA_PKCS1_PADDING          1
-//     // # define RSA_NO_PADDING             3
-//     // # define RSA_PKCS1_OAEP_PADDING     4
-//     // # define RSA_X931_PADDING           5
-//     // /* EVP_PKEY_ only */
-//     // # define RSA_PKCS1_PSS_PADDING      6
-//     // # define RSA_PKCS1_WITH_TLS_PADDING 7
-//     // /* internal RSA_ only */
-//     // # define RSA_PKCS1_NO_IMPLICIT_REJECT_PADDING 8
-//     this instanceof Literal and
-//     this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8]
-//     // TODO: trace to padding-specific consumers
-//     RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerFlow
-//   }
-//   override string getRawPaddingAlgorithmName() { result = this.(Literal).getValue().toString() }
-//   override Crypto::TPaddingType getPaddingType() {
-//     if this.(Literal).getValue().toInt() in [1, 6, 7, 8]
-//     then result = Crypto::PKCS1_v1_5()
-//     else
-//       if this.(Literal).getValue().toInt() = 3
-//       then result = Crypto::NoPadding()
-//       else
-//         if this.(Literal).getValue().toInt() = 4
-//         then result = Crypto::OAEP()
-//         else
-//           if this.(Literal).getValue().toInt() = 5
-//           then result = Crypto::ANSI_X9_23()
-//           else result = Crypto::OtherPadding()
-//   }
-// }
 class OaepPaddingAlgorithmInstance extends Crypto::OaepPaddingAlgorithmInstance,
   KnownOpenSslPaddingConstantAlgorithmInstance
 {
@@ -170,10 +132,18 @@ class OaepPaddingAlgorithmInstance extends Crypto::OaepPaddingAlgorithmInstance,
   }
 
   override Crypto::HashAlgorithmInstance getOaepEncodingHashAlgorithm() {
-    none() //TODO
+    exists(OperationStep s |
+      this.getAvc().(AvcContextCreationStep).flowsToOperationStep(s) and
+      s.getAlgorithmValueConsumerForInput(HashAlgorithmOaepIO()) =
+        result.(OpenSslAlgorithmInstance).getAvc()
+    )
   }
 
   override Crypto::HashAlgorithmInstance getMgf1HashAlgorithm() {
-    none() //TODO
+    exists(OperationStep s |
+      this.getAvc().(AvcContextCreationStep).flowsToOperationStep(s) and
+      s.getAlgorithmValueConsumerForInput(HashAlgorithmMgf1IO()) =
+        result.(OpenSslAlgorithmInstance).getAvc()
+    )
   }
 }

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/SignatureAlgorithmInstance.qll

@@ -47,7 +47,8 @@ class KnownOpenSslSignatureConstantAlgorithmInstance extends OpenSslAlgorithmIns
       // Sink is an argument to a signature getter call
       sink = getterCall.getInputNode() and
       // Source is `this`
-      src.asExpr() = this and
+      // NOTE: src literals can be ints or strings, so need to consider asExpr and asIndirectExpr
+      this = [src.asExpr(), src.asIndirectExpr()] and
       // This traces to a getter
       KnownOpenSslAlgorithmToAlgorithmValueConsumerFlow::flow(src, sink)
     )

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/CipherAlgorithmValueConsumer.qll

@@ -12,15 +12,17 @@ class EvpCipherAlgorithmValueConsumer extends CipherAlgorithmValueConsumer {
   DataFlow::Node resultNode;
 
   EvpCipherAlgorithmValueConsumer() {
-    resultNode.asExpr() = this and
+    resultNode.asIndirectExpr() = this and
     (
-      this.(Call).getTarget().getName() in [
-          "EVP_get_cipherbyname", "EVP_get_cipherbyobj", "EVP_get_cipherbynid"
-        ] and
+      this.(Call).getTarget().getName() in ["EVP_get_cipherbyname", "EVP_get_cipherbyobj"] and
+      valueArgNode.asIndirectExpr() = this.(Call).getArgument(0)
+      or
+      this.(Call).getTarget().getName() = "EVP_get_cipherbynid" and
+      // algorithm is an NID (int), use asExpr()
       valueArgNode.asExpr() = this.(Call).getArgument(0)
       or
       this.(Call).getTarget().getName() in ["EVP_CIPHER_fetch", "EVP_ASYM_CIPHER_fetch"] and
-      valueArgNode.asExpr() = this.(Call).getArgument(1)
+      valueArgNode.asIndirectExpr() = this.(Call).getArgument(1)
     )
   }
 

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/DirectAlgorithmValueConsumer.qll

@@ -23,7 +23,7 @@ class DirectAlgorithmValueConsumer extends OpenSslAlgorithmValueConsumer instanc
    */
   override DataFlow::Node getResultNode() {
     this instanceof OpenSslDirectAlgorithmFetchCall and
-    result.asExpr() = this
+    result.asIndirectExpr() = this
     // NOTE: if instanceof OpenSslDirectAlgorithmOperationCall then there is no algorithm generated
     // the algorithm is directly used
   }

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/EllipticCurveAlgorithmValueConsumer.qll

@@ -12,14 +12,19 @@ class EvpEllipticCurveAlgorithmConsumer extends EllipticCurveValueConsumer {
   DataFlow::Node resultNode;
 
   EvpEllipticCurveAlgorithmConsumer() {
-    resultNode.asExpr() = this.(Call) and // in all cases the result is the return
+    resultNode.asIndirectExpr() = this.(Call) and // in all cases the result is the return
     (
-      this.(Call).getTarget().getName() in ["EVP_EC_gen", "EC_KEY_new_by_curve_name"] and
+      this.(Call).getTarget().getName() = "EVP_EC_gen" and
+      valueArgNode.asIndirectExpr() = this.(Call).getArgument(0)
+      or
+      this.(Call).getTarget().getName() = "EC_KEY_new_by_curve_name" and
+      // algorithm is an NID (int), use asExpr()
       valueArgNode.asExpr() = this.(Call).getArgument(0)
       or
       this.(Call).getTarget().getName() in [
           "EC_KEY_new_by_curve_name_ex", "EVP_PKEY_CTX_set_ec_paramgen_curve_nid"
         ] and
+      // algorithm is an NID (int), use asExpr
       valueArgNode.asExpr() = this.(Call).getArgument(2)
     )
   }

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/HashAlgorithmValueConsumer.qll

@@ -9,11 +9,11 @@ abstract class HashAlgorithmValueConsumer extends OpenSslAlgorithmValueConsumer
 /**
  * An EVP_Q_Digest directly consumes algorithm constant values
  */
-class Evp_Q_Digest_Algorithm_Consumer extends HashAlgorithmValueConsumer {
-  Evp_Q_Digest_Algorithm_Consumer() { this.(Call).getTarget().getName() = "EVP_Q_digest" }
+class Evp_Q_Digest_Algorithm_Consumer extends HashAlgorithmValueConsumer instanceof Call {
+  Evp_Q_Digest_Algorithm_Consumer() { super.getTarget().getName() = "EVP_Q_digest" }
 
   override Crypto::ConsumerInputDataFlowNode getInputNode() {
-    result.asExpr() = this.(Call).getArgument(1)
+    result.asIndirectExpr() = super.getArgument(1)
   }
 
   override Crypto::AlgorithmInstance getAKnownAlgorithmSource() {
@@ -42,7 +42,7 @@ class EvpPkeySetCtxALgorithmConsumer extends HashAlgorithmValueConsumer {
         "EVP_PKEY_CTX_set_rsa_mgf1_md_name", "EVP_PKEY_CTX_set_rsa_oaep_md_name",
         "EVP_PKEY_CTX_set_dsa_paramgen_md_props"
       ] and
-    valueArgNode.asExpr() = this.(Call).getArgument(1)
+    valueArgNode.asIndirectExpr() = this.(Call).getArgument(1)
   }
 
   override DataFlow::Node getResultNode() { none() }
@@ -64,18 +64,18 @@ class EvpDigestAlgorithmValueConsumer extends HashAlgorithmValueConsumer {
   DataFlow::Node resultNode;
 
   EvpDigestAlgorithmValueConsumer() {
-    resultNode.asExpr() = this and
+    resultNode.asIndirectExpr() = this and
     (
       this.(Call).getTarget().getName() in [
           "EVP_get_digestbyname", "EVP_get_digestbynid", "EVP_get_digestbyobj"
         ] and
-      valueArgNode.asExpr() = this.(Call).getArgument(0)
+      valueArgNode.asIndirectExpr() = this.(Call).getArgument(0)
       or
       this.(Call).getTarget().getName() = "EVP_MD_fetch" and
-      valueArgNode.asExpr() = this.(Call).getArgument(1)
+      valueArgNode.asIndirectExpr() = this.(Call).getArgument(1)
       or
       this.(Call).getTarget().getName() = "EVP_DigestSignInit_ex" and
-      valueArgNode.asExpr() = this.(Call).getArgument(2)
+      valueArgNode.asIndirectExpr() = this.(Call).getArgument(2)
     )
   }
 
@@ -87,3 +87,21 @@ class EvpDigestAlgorithmValueConsumer extends HashAlgorithmValueConsumer {
     exists(OpenSslAlgorithmInstance i | i.getAvc() = this and result = i)
   }
 }
+
+class RsaSignOrVerifyHashAlgorithmValueConsumer extends HashAlgorithmValueConsumer {
+  DataFlow::Node valueArgNode;
+
+  RsaSignOrVerifyHashAlgorithmValueConsumer() {
+    this.(Call).getTarget().getName() in ["RSA_sign", "RSA_verify"] and
+    // arg 0 is an int, use asExpr
+    valueArgNode.asExpr() = this.(Call).getArgument(0)
+  }
+
+  override DataFlow::Node getResultNode() { none() }
+
+  override Crypto::ConsumerInputDataFlowNode getInputNode() { result = valueArgNode }
+
+  override Crypto::AlgorithmInstance getAKnownAlgorithmSource() {
+    exists(OpenSslAlgorithmInstance i | i.getAvc() = this and result = i)
+  }
+}

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/KEMAlgorithmValueConsumer.qll

@@ -11,10 +11,10 @@ class EvpKemAlgorithmValueConsumer extends KemAlgorithmValueConsumer {
   DataFlow::Node resultNode;
 
   EvpKemAlgorithmValueConsumer() {
-    resultNode.asExpr() = this and
+    resultNode.asIndirectExpr() = this and
     (
       this.(Call).getTarget().getName() = "EVP_KEM_fetch" and
-      valueArgNode.asExpr() = this.(Call).getArgument(1)
+      valueArgNode.asIndirectExpr() = this.(Call).getArgument(1)
     )
   }
 

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/KeyExchangeAlgorithmValueConsumer.qll

@@ -11,10 +11,10 @@ class EvpKeyExchangeAlgorithmValueConsumer extends KeyExchangeAlgorithmValueCons
   DataFlow::Node resultNode;
 
   EvpKeyExchangeAlgorithmValueConsumer() {
-    resultNode.asExpr() = this and
+    resultNode.asIndirectExpr() = this and
     (
       this.(Call).getTarget().getName() = "EVP_KEYEXCH_fetch" and
-      valueArgNode.asExpr() = this.(Call).getArgument(1)
+      valueArgNode.asIndirectExpr() = this.(Call).getArgument(1)
     )
   }
 

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/PKeyAlgorithmValueConsumer.qll

@@ -11,7 +11,7 @@ class EvpPKeyAlgorithmConsumer extends PKeyValueConsumer {
   DataFlow::Node resultNode;
 
   EvpPKeyAlgorithmConsumer() {
-    resultNode.asExpr() = this.(Call) and // in all cases the result is the return
+    resultNode.asIndirectExpr() = this.(Call) and // in all cases the result is the return
     (
       // NOTE: some of these consumers are themselves key gen operations,
       // in these cases, the operation will be created separately for the same function.
@@ -19,6 +19,7 @@ class EvpPKeyAlgorithmConsumer extends PKeyValueConsumer {
           "EVP_PKEY_CTX_new_id", "EVP_PKEY_new_raw_private_key", "EVP_PKEY_new_raw_public_key",
           "EVP_PKEY_new_mac_key"
         ] and
+      // Algorithm is an int, use asExpr
       valueArgNode.asExpr() = this.(Call).getArgument(0)
       or
       this.(Call).getTarget().getName() in [
@@ -26,7 +27,8 @@ class EvpPKeyAlgorithmConsumer extends PKeyValueConsumer {
           "EVP_PKEY_new_raw_public_key_ex", "EVP_PKEY_CTX_ctrl", "EVP_PKEY_CTX_ctrl_uint64",
           "EVP_PKEY_CTX_ctrl_str", "EVP_PKEY_CTX_set_group_name"
         ] and
-      valueArgNode.asExpr() = this.(Call).getArgument(1)
+      // AAlgorithm is a char*, use asIndirectExpr
+      valueArgNode.asIndirectExpr() = this.(Call).getArgument(1)
       or
       // argInd 2 is 'type' which can be RSA, or EC
       // if RSA argInd 3 is the key size, else if EC argInd 3 is the curve name
@@ -38,10 +40,10 @@ class EvpPKeyAlgorithmConsumer extends PKeyValueConsumer {
         // Elliptic curve case
         // If the argInd 3 is a derived type (pointer or array) then assume it is a curve name
         if this.(Call).getArgument(3).getType().getUnderlyingType() instanceof DerivedType
-        then valueArgNode.asExpr() = this.(Call).getArgument(3)
+        then valueArgNode.asIndirectExpr() = this.(Call).getArgument(3)
         else
           // All other cases
-          valueArgNode.asExpr() = this.(Call).getArgument(2)
+          valueArgNode.asIndirectExpr() = this.(Call).getArgument(2)
       )
     )
   }

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/PaddingAlgorithmValueConsumer.qll

@@ -14,8 +14,9 @@ class Evp_PKey_Ctx_set_rsa_padding_AlgorithmValueConsumer extends PaddingAlgorit
   DataFlow::Node resultNode;
 
   Evp_PKey_Ctx_set_rsa_padding_AlgorithmValueConsumer() {
-    resultNode.asExpr() = this and
+    resultNode.asDefiningArgument() = this.(Call).getArgument(0) and
     this.(Call).getTarget().getName() = "EVP_PKEY_CTX_set_rsa_padding" and
+    // algorithm is an int, use asExpr
     valueArgNode.asExpr() = this.(Call).getArgument(1)
   }
 

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/SignatureAlgorithmValueConsumer.qll

@@ -12,13 +12,13 @@ class EvpSignatureAlgorithmValueConsumer extends SignatureAlgorithmValueConsumer
   DataFlow::Node resultNode;
 
   EvpSignatureAlgorithmValueConsumer() {
-    resultNode.asExpr() = this and
+    resultNode.asIndirectExpr() = this and
     (
       // EVP_SIGNATURE
       this.(Call).getTarget().getName() = "EVP_SIGNATURE_fetch" and
-      valueArgNode.asExpr() = this.(Call).getArgument(1)
+      valueArgNode.asIndirectExpr() = this.(Call).getArgument(1)
       // EVP_PKEY_get1_DSA, EVP_PKEY_get1_RSA
-      // DSA_SIG_new, DSA_SIG_get0, RSA_sign ?
+      // DSA_SIG_new, DSA_SIG_get0 ?
     )
   }
 

cpp/ql/lib/experimental/quantum/OpenSSL/ArtifactPassthrough.qll

@@ -0,0 +1,107 @@
+private import experimental.quantum.Language
+
+/**
+ * A call to `BN_bn2bin`.
+ * Commonly used to extract partial bytes from a signature,
+ * e.g., a signature from DSA_do_sign, passed to DSA_do_verify
+ * - int BN_bn2bin(const BIGNUM *a, unsigned char *to);
+ */
+class BnBn2BinCalStep extends AdditionalFlowInputStep {
+  Call call;
+
+  BnBn2BinCalStep() {
+    call.getTarget().getName() = "BN_bn2bin" and
+    call.getArgument(0) = this.asIndirectExpr()
+  }
+
+  override DataFlow::Node getOutput() { result.asDefiningArgument() = call.getArgument(1) }
+}
+
+/**
+ * A call to `BN_bin2bn`.
+ * Commonly used to convert to a signature for DSA_do_verify
+ * - BIGNUM *BN_bin2bn(const unsigned char *s, int len, BIGNUM *ret);
+ */
+class BnBin2BnCallStep extends AdditionalFlowInputStep {
+  Call call;
+
+  BnBin2BnCallStep() {
+    call.getTarget().getName() = "BN_bin2bn" and
+    call.getArgument(0) = this.asIndirectExpr()
+  }
+
+  override DataFlow::Node getOutput() { result.asDefiningArgument() = call.getArgument(2) }
+}
+
+/**
+ * A call to `RSA_set0_key` or `DSA_SIG_set0`.
+ * Often used in combination with BN_bin2bn, to construct a signature.
+ */
+class RsaSet0KeyCallStep extends AdditionalFlowInputStep {
+  Call call;
+
+  RsaSet0KeyCallStep() {
+    (call.getTarget().getName() = "RSA_set0_key" or call.getTarget().getName() = "DSA_SIG_set0") and
+    this.asIndirectExpr() in [call.getArgument(1), call.getArgument(2), call.getArgument(3)]
+  }
+
+  override DataFlow::Node getOutput() { result.asDefiningArgument() = call.getArgument(0) }
+}
+
+/**
+ * A call to `d2i_DSA_SIG`. This is a pass through of a signature of one form to another.
+ * - DSA_SIG *d2i_DSA_SIG(DSA_SIG **sig, const unsigned char **pp, long length);
+ */
+class D2iDsaSigCallStep extends AdditionalFlowInputStep {
+  Call call;
+
+  D2iDsaSigCallStep() {
+    call.getTarget().getName() = "d2i_DSA_SIG" and
+    this.asIndirectExpr() = call.getArgument(1)
+  }
+
+  override DataFlow::Node getOutput() {
+    // If arg 0 specified, the same pointer is returned, if not specified
+    // a new allocation is returned.
+    result.asDefiningArgument() = call.getArgument(0) or
+    result.asIndirectExpr() = call
+  }
+}
+
+/**
+ * A call to `DSA_SIG_get0`.
+ * Converts a DSA_Sig into its components, which are commonly used with BN_bn2Bin to
+ * construct a char* signature.
+ * - void DSA_SIG_get0(const DSA_SIG *sig, const BIGNUM **pr, const BIGNUM **ps);
+ */
+class DsaSigGet0CallStep extends AdditionalFlowInputStep {
+  Call call;
+
+  DsaSigGet0CallStep() {
+    call.getTarget().getName() = "DSA_SIG_get0" and
+    this.asIndirectExpr() = call.getArgument(0)
+  }
+
+  override DataFlow::Node getOutput() {
+    result.asDefiningArgument() = call.getArgument(1)
+    or
+    result.asDefiningArgument() = call.getArgument(2)
+  }
+}
+
+/**
+ * A call to `EVP_PKEY_get1_RSA` or `EVP_PKEY_get1_DSA`
+ *  - RSA *EVP_PKEY_get1_RSA(EVP_PKEY *pkey);
+ *  - DSA *EVP_PKEY_get1_DSA(EVP_PKEY *pkey);
+ * A key input is converted into a key output, a key is not generated.
+ */
+class EvpPkeyGet1RsaOrDsa extends AdditionalFlowInputStep {
+  Call c;
+
+  EvpPkeyGet1RsaOrDsa() {
+    c.getTarget().getName() = ["EVP_PKEY_get1_RSA", "EVP_PKEY_get1_DSA"] and
+    this.asIndirectExpr() = c.getArgument(0)
+  }
+
+  override DataFlow::Node getOutput() { result.asIndirectExpr() = c }
+}

cpp/ql/lib/experimental/quantum/OpenSSL/AvcFlow.qll

@@ -1,4 +1,4 @@
-import semmle.code.cpp.dataflow.new.DataFlow
+import semmle.code.cpp.dataflow.new.TaintTracking
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
 
 /**
@@ -13,7 +13,9 @@ module AvcToCallArgConfig implements DataFlow::ConfigSig {
    * Trace to any call accepting the algorithm.
    * NOTE: users must restrict this set to the operations they are interested in.
    */
-  predicate isSink(DataFlow::Node sink) { exists(Call c | c.getAnArgument() = sink.asExpr()) }
+  predicate isSink(DataFlow::Node sink) {
+    exists(Call c | c.getAnArgument() = [sink.asIndirectExpr(), sink.asExpr()])
+  }
 }
 
-module AvcToCallArgFlow = DataFlow::Global<AvcToCallArgConfig>;
+module AvcToCallArgFlow = TaintTracking::Global<AvcToCallArgConfig>;

cpp/ql/lib/experimental/quantum/OpenSSL/OpenSSL.qll

@@ -4,4 +4,5 @@ module OpenSslModel {
   import Operations.OpenSSLOperations
   import Random
   import GenericSourceCandidateLiteral
+  import ArtifactPassthrough
 }

cpp/ql/lib/experimental/quantum/OpenSSL/Operations/CipherOperation.qll

@@ -3,24 +3,48 @@ private import OpenSSLOperationBase
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
 import EVPPKeyCtxInitializer
 
+/**
+ * A base class for all final cipher operation steps.
+ */
+abstract class FinalCipherOperationStep extends OperationStep {
+  override OperationStepType getStepType() { result = FinalStep() }
+}
+
+/**
+ * A base configuration for all EVP cipher operations.
+ */
+abstract class EvpCipherOperationFinalStep extends FinalCipherOperationStep {
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+  }
+}
+
 /**
  * A base class for all EVP cipher operations.
  */
 abstract class EvpCipherInitializer extends OperationStep {
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(1) and
+    result.asIndirectExpr() = this.getArgument(1) and
     type = PrimaryAlgorithmIO() and
     // Constants that are not equal to zero or
     // non-constants (e.g., variable accesses, which require data-flow to determine the value)
     // A zero (null) value typically indicates use of this operation step to initialize
     // other out parameters in a multi-step initialization.
-    (exists(result.asExpr().getValue()) implies result.asExpr().getValue().toInt() != 0)
+    (
+      exists(result.asIndirectExpr().getValue())
+      implies
+      result.asIndirectExpr().getValue().toInt() != 0
+    )
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = InitializerStep() }
@@ -38,11 +62,15 @@ abstract class EvpEXInitializer extends EvpCipherInitializer {
       // non-constants (e.g., variable accesses, which require data-flow to determine the value)
       // A zero (null) value typically indicates use of this operation step to initialize
       // other out parameters in a multi-step initialization.
-      result.asExpr() = this.getArgument(3) and type = KeyIO()
+      result.asIndirectExpr() = this.getArgument(3) and type = KeyIO()
       or
-      result.asExpr() = this.getArgument(4) and type = IVorNonceIO()
+      result.asIndirectExpr() = this.getArgument(4) and type = IVorNonceIO()
     ) and
-    (exists(result.asExpr().getValue()) implies result.asExpr().getValue().toInt() != 0)
+    (
+      exists(result.asIndirectExpr().getValue())
+      implies
+      result.asIndirectExpr().getValue().toInt() != 0
+    )
   }
 }
 
@@ -53,9 +81,9 @@ abstract class EvpEX2Initializer extends EvpCipherInitializer {
   override DataFlow::Node getInput(IOType type) {
     result = super.getInput(type)
     or
-    result.asExpr() = this.getArgument(2) and type = KeyIO()
+    result.asIndirectExpr() = this.getArgument(2) and type = KeyIO()
     or
-    result.asExpr() = this.getArgument(3) and type = IVorNonceIO()
+    result.asIndirectExpr() = this.getArgument(3) and type = IVorNonceIO()
   }
 }
 
@@ -90,6 +118,7 @@ class Evp_Cipher_EX2_or_Simple_Init_Call extends EvpEX2Initializer {
     result = super.getInput(type)
     or
     this.getTarget().getName().toLowerCase().matches("%cipherinit%") and
+    // the key op subtype is an int, use asExpr
     result.asExpr() = this.getArgument(4) and
     type = KeyOperationSubtypeIO()
   }
@@ -107,13 +136,13 @@ class EvpPkeyEncryptDecryptInit extends OperationStep {
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(1) and type = OsslParamIO()
+    result.asIndirectExpr() = this.getArgument(1) and type = OsslParamIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = InitializerStep() }
@@ -125,6 +154,7 @@ class EvpCipherInitSKeyCall extends EvpEX2Initializer {
   override DataFlow::Node getInput(IOType type) {
     result = super.getInput(type)
     or
+    // the key op subtype is an int, use asExpr
     result.asExpr() = this.getArgument(5) and
     type = KeyOperationSubtypeIO()
   }
@@ -141,35 +171,20 @@ class EvpCipherUpdateCall extends OperationStep {
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(3) and type = PlaintextIO()
+    result.asIndirectExpr() = this.getArgument(3) and type = PlaintextIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(1) and type = CiphertextIO()
+    result.asDefiningArgument() = this.getArgument(1) and type = CiphertextIO()
     or
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = UpdateStep() }
 }
 
-/**
- * A base configuration for all EVP cipher operations.
- */
-abstract class EvpCipherOperationFinalStep extends OperationStep {
-  override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
-  }
-
-  override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
-  }
-
-  override OperationStepType getStepType() { result = FinalStep() }
-}
-
 /**
  * A Call to EVP_Cipher.
  */
@@ -179,13 +194,13 @@ class EvpCipherCall extends EvpCipherOperationFinalStep {
   override DataFlow::Node getInput(IOType type) {
     super.getInput(type) = result
     or
-    result.asExpr() = this.getArgument(2) and type = PlaintextIO()
+    result.asIndirectExpr() = this.getArgument(2) and type = PlaintextIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
     super.getOutput(type) = result
     or
-    result.asExpr() = this.getArgument(1) and type = CiphertextIO()
+    result.asDefiningArgument() = this.getArgument(1) and type = CiphertextIO()
   }
 }
 
@@ -216,28 +231,50 @@ class EvpCipherFinalCall extends EvpCipherOperationFinalStep {
  */
 class EvpPKeyCipherOperation extends EvpCipherOperationFinalStep {
   EvpPKeyCipherOperation() {
-    this.getTarget().getName() in ["EVP_PKEY_encrypt", "EVP_PKEY_decrypt"]
+    this.getTarget().getName() in ["EVP_PKEY_encrypt", "EVP_PKEY_decrypt"] and
+    // TODO: for now ignore this operation entirely if it is setting the cipher text to null
+    // this needs to be re-evalauted if this scenario sets other values worth tracking
+    (
+      exists(this.(Call).getArgument(1).getValue())
+      implies
+      this.(Call).getArgument(1).getValue().toInt() != 0
+    )
   }
 
   override DataFlow::Node getInput(IOType type) {
     super.getInput(type) = result
     or
-    result.asExpr() = this.getArgument(3) and type = PlaintextIO()
+    result.asIndirectExpr() = this.getArgument(3) and type = PlaintextIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
     super.getOutput(type) = result
     or
-    result.asExpr() = this.getArgument(1) and type = CiphertextIO()
+    result.asDefiningArgument() = this.getArgument(1) and
+    type = CiphertextIO() and
+    this.getStepType() = FinalStep()
     // TODO: could indicate text lengths here, as well
   }
+
+  override OperationStepType getStepType() {
+    // When the output buffer is null, the step is not a final step
+    // it is used to get the buffer size, if 0 consider it an initialization step
+    // NOTE/TODO: not tracing 0 to the arg, just looking for 0 directly in param
+    // the assumption is this is the common case, but we may want to make this more
+    // robust and support a dataflow.
+    result = FinalStep() and
+    (exists(super.getArgument(1).getValue()) implies super.getArgument(1).getValue().toInt() != 0)
+    or
+    result = InitializerStep() and
+    super.getArgument(1).getValue().toInt() = 0
+  }
 }
 
 /**
  * An EVP cipher operation instance.
  * Any operation step that is a final operation step for EVP cipher operation steps.
  */
-class EvpCipherOperationInstance extends Crypto::KeyOperationInstance instanceof EvpCipherOperationFinalStep
+class OpenSslCipherOperationInstance extends Crypto::KeyOperationInstance instanceof FinalCipherOperationStep
 {
   override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
     super.getPrimaryAlgorithmValueConsumer() = result

cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPPKeyCtxInitializer.qll

@@ -1,5 +1,10 @@
 /**
  * Initializers for EVP PKey
+ * These are used to create a Pkey context or set properties on a Pkey context
+ * e.g., key size, hash algorithms, curves, padding schemes, etc.
+ * Meant to capture more general purpose initializers that aren't necessarily
+ * tied to a specific operation. If tied to an operation (i.e., in the docs)
+ * we recommend defining defining all together in the same operation definition qll.
  * including:
  *  https://docs.openssl.org/3.0/man3/EVP_PKEY_CTX_ctrl/
  *  https://docs.openssl.org/3.0/man3/EVP_EncryptInit/#synopsis
@@ -26,14 +31,16 @@ class EvpNewKeyCtx extends OperationStep instanceof Call {
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = keyArg and type = KeyIO()
+    result.asIndirectExpr() = keyArg and type = KeyIO()
     or
     this.getTarget().getName() = "EVP_PKEY_CTX_new_from_pkey" and
-    result.asExpr() = this.getArgument(0) and
+    result.asIndirectExpr() = this.getArgument(0) and
     type = OsslLibContextIO()
   }
 
-  override DataFlow::Node getOutput(IOType type) { result.asExpr() = this and type = ContextIO() }
+  override DataFlow::Node getOutput(IOType type) {
+    result.asIndirectExpr() = this and type = ContextIO()
+  }
 
   override OperationStepType getStepType() { result = ContextCreationStep() }
 }
@@ -47,13 +54,13 @@ class EvpCtxSetEcParamgenCurveNidInitializer extends OperationStep {
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(1) and type = PrimaryAlgorithmIO()
+    result.asIndirectExpr() = this.getArgument(1) and type = PrimaryAlgorithmIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = InitializerStep() }
@@ -71,23 +78,46 @@ class EvpCtxSetEcParamgenCurveNidInitializer extends OperationStep {
  * - `EVP_PKEY_CTX_set_ecdh_kdf_md`
  */
 class EvpCtxSetHashInitializer extends OperationStep {
+  boolean isOaep;
+  boolean isMgf1;
+
   EvpCtxSetHashInitializer() {
     this.getTarget().getName() in [
-        "EVP_PKEY_CTX_set_signature_md", "EVP_PKEY_CTX_set_rsa_mgf1_md_name",
-        "EVP_PKEY_CTX_set_rsa_mgf1_md", "EVP_PKEY_CTX_set_rsa_oaep_md_name",
-        "EVP_PKEY_CTX_set_rsa_oaep_md", "EVP_PKEY_CTX_set_dsa_paramgen_md",
+        "EVP_PKEY_CTX_set_signature_md", "EVP_PKEY_CTX_set_dsa_paramgen_md",
         "EVP_PKEY_CTX_set_dh_kdf_md", "EVP_PKEY_CTX_set_ecdh_kdf_md"
-      ]
+      ] and
+    isOaep = false and
+    isMgf1 = false
+    or
+    this.getTarget().getName() in [
+        "EVP_PKEY_CTX_set_rsa_mgf1_md_name", "EVP_PKEY_CTX_set_rsa_mgf1_md"
+      ] and
+    isOaep = false and
+    isMgf1 = true
+    or
+    this.getTarget().getName() in [
+        "EVP_PKEY_CTX_set_rsa_oaep_md_name",
+        "EVP_PKEY_CTX_set_rsa_oaep_md"
+      ] and
+    isOaep = true and
+    isMgf1 = false
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(1) and type = HashAlgorithmIO()
+    result.asIndirectExpr() = this.getArgument(1) and
+    type = HashAlgorithmIO() and
+    isOaep = false and
+    isMgf1 = false
+    or
+    result.asIndirectExpr() = this.getArgument(1) and type = HashAlgorithmOaepIO() and isOaep = true
+    or
+    result.asIndirectExpr() = this.getArgument(1) and type = HashAlgorithmMgf1IO() and isMgf1 = true
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = InitializerStep() }
@@ -106,13 +136,13 @@ class EvpCtxSetKeySizeInitializer extends OperationStep {
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
     result.asExpr() = this.getArgument(1) and type = KeySizeIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = InitializerStep() }
@@ -122,16 +152,16 @@ class EvpCtxSetMacKeyInitializer extends OperationStep {
   EvpCtxSetMacKeyInitializer() { this.getTarget().getName() = "EVP_PKEY_CTX_set_mac_key" }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
     result.asExpr() = this.getArgument(2) and type = KeySizeIO()
     or
     // the raw key that is configured into the output key
-    result.asExpr() = this.getArgument(1) and type = KeyIO()
+    result.asIndirectExpr() = this.getArgument(1) and type = KeyIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = InitializerStep() }
@@ -143,13 +173,14 @@ class EvpCtxSetPaddingInitializer extends OperationStep {
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
+    // The algorithm is an int: use asExpr
     result.asExpr() = this.getArgument(1) and type = PaddingAlgorithmIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = InitializerStep() }
@@ -161,13 +192,13 @@ class EvpCtxSetSaltLengthInitializer extends OperationStep {
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
     result.asExpr() = this.getArgument(1) and type = SaltLengthIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = InitializerStep() }

cpp/ql/lib/experimental/quantum/OpenSSL/Operations/HashOperation.qll

@@ -6,6 +6,13 @@ private import experimental.quantum.Language
 private import OpenSSLOperationBase
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
 
+/**
+ * A base class for final digest operations.
+ */
+abstract class FinalDigestOperation extends OperationStep {
+  override OperationStepType getStepType() { result = FinalStep() }
+}
+
 /**
  * A call to and EVP digest initializer, such as:
  * - `EVP_DigestInit`
@@ -18,13 +25,13 @@ class EvpDigestInitVariantCalls extends OperationStep instanceof Call {
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(1) and type = PrimaryAlgorithmIO()
+    result.asIndirectExpr() = this.getArgument(1) and type = PrimaryAlgorithmIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and
+    result.asDefiningArgument() = this.getArgument(0) and
     type = ContextIO()
   }
 
@@ -38,56 +45,49 @@ class EvpDigestUpdateCall extends OperationStep instanceof Call {
   EvpDigestUpdateCall() { this.getTarget().getName() = "EVP_DigestUpdate" }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(1) and type = PlaintextIO()
+    result.asIndirectExpr() = this.getArgument(1) and type = PlaintextIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and
+    result.asDefiningArgument() = this.getArgument(0) and
     type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = UpdateStep() }
 }
 
-/**
- * A base class for final digest operations.
- */
-abstract class EvpFinalDigestOperationStep extends OperationStep {
-  override OperationStepType getStepType() { result = FinalStep() }
-}
-
 /**
  * A call to `EVP_Q_digest`
  * https://docs.openssl.org/3.0/man3/EVP_DigestInit/#synopsis
  */
-class EvpQDigestOperation extends EvpFinalDigestOperationStep instanceof Call {
+class EvpQDigestOperation extends FinalDigestOperation instanceof Call {
   EvpQDigestOperation() { this.getTarget().getName() = "EVP_Q_digest" }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(1) and type = PrimaryAlgorithmIO()
+    result.asIndirectExpr() = this.getArgument(1) and type = PrimaryAlgorithmIO()
     or
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(3) and type = PlaintextIO()
+    result.asIndirectExpr() = this.getArgument(3) and type = PlaintextIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and
+    result.asDefiningArgument() = this.getArgument(0) and
     type = ContextIO()
     or
     result.asDefiningArgument() = this.getArgument(5) and type = DigestIO()
   }
 }
 
-class EvpDigestOperation extends EvpFinalDigestOperationStep instanceof Call {
+class EvpDigestOperation extends FinalDigestOperation instanceof Call {
   EvpDigestOperation() { this.getTarget().getName() = "EVP_Digest" }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(4) and type = PrimaryAlgorithmIO()
+    result.asIndirectExpr() = this.getArgument(4) and type = PrimaryAlgorithmIO()
     or
-    result.asExpr() = this.getArgument(0) and type = PlaintextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = PlaintextIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
@@ -98,27 +98,28 @@ class EvpDigestOperation extends EvpFinalDigestOperationStep instanceof Call {
 /**
  * A call to EVP_DigestFinal variants
  */
-class EvpDigestFinalCall extends EvpFinalDigestOperationStep instanceof Call {
+class EvpDigestFinalCall extends FinalDigestOperation instanceof Call {
   EvpDigestFinalCall() {
     this.getTarget().getName() in ["EVP_DigestFinal", "EVP_DigestFinal_ex", "EVP_DigestFinalXOF"]
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and
+    result.asDefiningArgument() = this.getArgument(0) and
     type = ContextIO()
     or
     result.asDefiningArgument() = this.getArgument(1) and type = DigestIO()
+    //result.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = this.getArgument(1)
   }
 }
 
 /**
  * An openssl digest final hash operation instance
  */
-class EvpDigestFinalOperationInstance extends Crypto::HashOperationInstance instanceof EvpFinalDigestOperationStep
+class OpenSslDigestFinalOperationInstance extends Crypto::HashOperationInstance instanceof FinalDigestOperation
 {
   override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
     super.getPrimaryAlgorithmValueConsumer() = result

cpp/ql/lib/experimental/quantum/OpenSSL/Operations/KeyGenOperation.qll

@@ -13,10 +13,12 @@ class ECKeyGen extends OperationStep instanceof Call {
   ECKeyGen() { this.(Call).getTarget().getName() = "EC_KEY_generate_key" }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.(Call).getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.(Call).getArgument(0) and type = ContextIO()
   }
 
-  override DataFlow::Node getOutput(IOType type) { result.asExpr() = this and type = KeyIO() }
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this and type = KeyIO()
+  }
 
   override OperationStepType getStepType() { result = ContextCreationStep() }
 }
@@ -33,16 +35,19 @@ class EvpKeyGenInitialize extends OperationStep {
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = InitializerStep() }
 }
 
+/**
+ * A base class for final key generation operation steps.
+ */
 abstract class KeyGenFinalOperationStep extends OperationStep {
   override OperationStepType getStepType() { result = FinalStep() }
 }
@@ -54,26 +59,26 @@ class EvpPKeyQKeyGen extends KeyGenFinalOperationStep instanceof Call {
   EvpPKeyQKeyGen() { this.getTarget().getName() = "EVP_PKEY_Q_keygen" }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this and type = KeyIO()
+    result.asDefiningArgument() = this and type = KeyIO()
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
     // When arg 3 is a derived type, it is a curve name, otherwise it is a key size for RSA if provided
     // and arg 2 is the algorithm type
     this.getArgument(3).getType().getUnderlyingType() instanceof DerivedType and
-    result.asExpr() = this.getArgument(3) and
+    result.asIndirectExpr() = this.getArgument(3) and
     type = PrimaryAlgorithmIO()
     or
     not this.getArgument(3).getType().getUnderlyingType() instanceof DerivedType and
-    result.asExpr() = this.getArgument(2) and
+    result.asIndirectExpr() = this.getArgument(2) and
     type = PrimaryAlgorithmIO()
     or
     not this.getArgument(3).getType().getUnderlyingType() instanceof DerivedType and
-    result.asExpr() = this.getArgument(3) and
+    result.asIndirectExpr() = this.getArgument(3) and
     type = KeySizeIO()
   }
 }
@@ -84,7 +89,9 @@ class EvpPKeyQKeyGen extends KeyGenFinalOperationStep instanceof Call {
 class EvpRsaGen extends KeyGenFinalOperationStep instanceof Call {
   EvpRsaGen() { this.getTarget().getName() = "EVP_RSA_gen" }
 
-  override DataFlow::Node getOutput(IOType type) { result.asExpr() = this and type = KeyIO() }
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this and type = KeyIO()
+  }
 
   override DataFlow::Node getInput(IOType type) {
     result.asExpr() = this.getArgument(0) and type = KeySizeIO()
@@ -97,7 +104,9 @@ class EvpRsaGen extends KeyGenFinalOperationStep instanceof Call {
 class RsaGenerateKey extends KeyGenFinalOperationStep instanceof Call {
   RsaGenerateKey() { this.getTarget().getName() = "RSA_generate_key" }
 
-  override DataFlow::Node getOutput(IOType type) { result.asExpr() = this and type = KeyIO() }
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this and type = KeyIO()
+  }
 
   override DataFlow::Node getInput(IOType type) {
     result.asExpr() = this.getArgument(0) and type = KeySizeIO()
@@ -117,7 +126,7 @@ class RsaGenerateKeyEx extends KeyGenFinalOperationStep instanceof Call {
   override DataFlow::Node getInput(IOType type) {
     // arg 0 comes in as a blank RSA key, which we consider a context,
     // on output it is considered a key
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
   }
 }
 
@@ -128,13 +137,13 @@ class EvpPkeyGen extends KeyGenFinalOperationStep instanceof Call {
   EvpPkeyGen() { this.getTarget().getName() in ["EVP_PKEY_generate", "EVP_PKEY_keygen"] }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
     result.asDefiningArgument() = this.getArgument(1) and type = KeyIO()
     or
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 }
 
@@ -146,18 +155,14 @@ class EvpNewMacKey extends KeyGenFinalOperationStep {
   EvpNewMacKey() { this.getTarget().getName() = "EVP_PKEY_new_mac_key" }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
-    or
     // the raw key that is configured into the output key
-    result.asExpr() = this.getArgument(2) and type = KeyIO()
+    result.asIndirectExpr() = this.getArgument(2) and type = KeyIO()
     or
     result.asExpr() = this.getArgument(3) and type = KeySizeIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this and type = KeyIO()
-    or
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this and type = KeyIO()
   }
 }
 
@@ -165,7 +170,7 @@ class EvpNewMacKey extends KeyGenFinalOperationStep {
 /**
  * An `KeyGenerationOperationInstance` for the for all key gen final operation steps.
  */
-class KeyGenOperationInstance extends Crypto::KeyGenerationOperationInstance instanceof KeyGenFinalOperationStep
+class OpenSslKeyGenOperationInstance extends Crypto::KeyGenerationOperationInstance instanceof KeyGenFinalOperationStep
 {
   override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
     super.getPrimaryAlgorithmValueConsumer() = result

cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll

@@ -1,6 +1,6 @@
 private import experimental.quantum.Language
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
-import semmle.code.cpp.dataflow.new.DataFlow
+import semmle.code.cpp.dataflow.new.TaintTracking
 // Importing these intializers here to ensure the are part of any model that is
 // using OpenSslOperationBase. This further ensures that initializers are tied to opeartions
 // even if only importing the operation by itself.
@@ -58,7 +58,11 @@ newtype TIOType =
   // For OSSL_PARAM and OSSL_LIB_CTX use of OsslParamIO and OsslLibContextIO
   ContextIO() or
   DigestIO() or
+  // For OAEP and MGF1 hashes, there is a special IO type for these hashes
+  // it is recommended to set the most explicit type known, not both
   HashAlgorithmIO() or
+  HashAlgorithmOaepIO() or
+  HashAlgorithmMgf1IO() or
   IVorNonceIO() or
   KeyIO() or
   KeyOperationSubtypeIO() or
@@ -71,11 +75,13 @@ newtype TIOType =
   PaddingAlgorithmIO() or
   // Plaintext also includes a message for digest, signature, verification, and mac generation
   PlaintextIO() or
+  PlaintextSizeIO() or
   PrimaryAlgorithmIO() or
   RandomSourceIO() or
   SaltLengthIO() or
   SeedIO() or
-  SignatureIO()
+  SignatureIO() or
+  SignatureSizeIO()
 
 private string ioTypeToString(TIOType t) {
   t = CiphertextIO() and result = "CiphertextIO"
@@ -104,6 +110,8 @@ private string ioTypeToString(TIOType t) {
   or
   t = PlaintextIO() and result = "PlaintextIO"
   or
+  t = PlaintextSizeIO() and result = "PlaintextSizeIO"
+  or
   t = PrimaryAlgorithmIO() and result = "PrimaryAlgorithmIO"
   or
   t = RandomSourceIO() and result = "RandomSourceIO"
@@ -113,6 +121,8 @@ private string ioTypeToString(TIOType t) {
   t = SeedIO() and result = "SeedIO"
   or
   t = SignatureIO() and result = "SignatureIO"
+  or
+  t = SignatureSizeIO() and result = "SignatureSizeIO"
 }
 
 class IOType extends TIOType {
@@ -123,13 +133,13 @@ class IOType extends TIOType {
   }
 }
 
-//TODO: add more initializers as needed
 /**
  * The type of step in an `OperationStep`.
  * - `ContextCreationStep`: the creation of a context from an algorithm or key.
  *                       for example `EVP_MD_CTX_create(EVP_sha256())` or `EVP_PKEY_CTX_new(pkey, NULL)`
- * - `InitializerStep`: the initialization of an operation through some sort of shared/accumulated context
- *                       for example `EVP_DigestInit_ex(ctx, EVP_sha256(), NULL)`
+ * - `InitializerStep`: the initialization of an operation or state through some sort of shared/accumulated context
+ *                       for example `EVP_DigestInit_ex(ctx, EVP_sha256(), NULL)`, may also be used for pass through
+ *                       configuration, for example `EVP_PKEY_get1_RSA(key)` where a pkey is input into an RSA key return.
  * - `UpdateStep`: any operation that has and update/final paradigm, the update represents an intermediate step in an operation,
  *                       such as `EVP_DigestUpdate(ctx, data, len)`
  * - `FinalStep`: an ultimate operation step. This may be an explicit 'final' in an update/final paradigm, but not necessarily.
@@ -189,7 +199,7 @@ abstract class OperationStep extends Call {
    */
   predicate flowsToOperationStep(OperationStep sink) {
     sink = this or
-    OperationStepFlow::flow(this.getAnOutput(), sink.getAnInput())
+    OperationStepCtxFlow::flow(this.getAnOutput(), [sink.getAnInput(), sink.getAnOutput()])
   }
 
   /**
@@ -198,7 +208,7 @@ abstract class OperationStep extends Call {
    */
   predicate flowsFromOperationStep(OperationStep source) {
     source = this or
-    OperationStepFlow::flow(source.getAnOutput(), this.getAnInput())
+    OperationStepCtxFlow::flow(source.getAnOutput(), [this.getAnInput(), this.getAnOutput()])
   }
 
   /**
@@ -220,10 +230,13 @@ abstract class OperationStep extends Call {
     result.setsValue(type) and
     (
       // Do not consider a 'reset' to occur on updates
+      // but only for resets that are part of the same update/finalize
+      // progression (e.g., an update for an unrelated finalize is ignored)
       result.getStepType() = UpdateStep()
       or
       not exists(OperationStep reset |
         result != reset and
+        result != this and
         reset.setsValue(type) and
         reset.flowsToOperationStep(this) and
         result.flowsToOperationStep(reset)
@@ -245,8 +258,11 @@ abstract class OperationStep extends Call {
 
   /**
    * Gets an AVC for the primary algorithm for this operation.
-   * A primary algorithm is an AVC that flows to a ctx input directly or
-   * an AVC that flows to a primary algorithm input directly.
+   * A primary algorithm is an AVC that either:
+   * 0) `this` is an AVC (consider direct algorithm consumers like RSA_sign (algorithm is implicit) or EVP_PKEY_new_mac_key (NID is first arg) )
+   * 1) flows to a ctx input directly or
+   * 2) flows to a primary algorithm input directly or
+   * 3) flows to a key input directly (algorithm held in a key will be considered primary)
    * See `AvcContextCreationStep` for details about resetting scenarios.
    * Gets the first OperationStep an AVC flows to. If a context input,
    * the AVC is considered primary.
@@ -254,19 +270,24 @@ abstract class OperationStep extends Call {
    * operation step (dominating operation step, see `getDominatingInitializersToStep`).
    */
   Crypto::AlgorithmValueConsumer getPrimaryAlgorithmValueConsumer() {
-    exists(DataFlow::Node src, DataFlow::Node sink, IOType t, OperationStep avcSucc |
-      (t = PrimaryAlgorithmIO() or t = ContextIO()) and
-      avcSucc.flowsToOperationStep(this) and
-      src.asExpr() = result and
-      sink = avcSucc.getInput(t) and
+    this instanceof Crypto::AlgorithmValueConsumer and result = this
+    or
+    exists(
+      DataFlow::Node src, DataFlow::Node sink, IOType srcIntype, OperationStep avcConsumingPred
+    |
+      (srcIntype = ContextIO() or srcIntype = PrimaryAlgorithmIO() or srcIntype = KeyIO()) and
+      avcConsumingPred.flowsToOperationStep(this) and
+      src.asIndirectExpr() = result and
+      sink = avcConsumingPred.getInput(srcIntype) and
       AvcToOperationStepFlow::flow(src, sink) and
       (
-        // Case 1: the avcSucc step is a dominating initialization step
-        t = PrimaryAlgorithmIO() and
-        avcSucc = this.getDominatingInitializersToStep(PrimaryAlgorithmIO())
+        // Case 1: the avcConsumingPred step is a dominating primary algorithm initialization step
+        // or dominating key initialization step
+        (srcIntype = PrimaryAlgorithmIO() or srcIntype = KeyIO()) and
+        avcConsumingPred = this.getDominatingInitializersToStep(srcIntype)
         or
-        // Case 2: the succ is a context input (any avcSucc is valid)
-        t = ContextIO()
+        // Case 2: the pred is a context input
+        srcIntype = ContextIO()
       )
     )
   }
@@ -277,9 +298,11 @@ abstract class OperationStep extends Call {
    * TODO: generalize to use this for `getPrimaryAlgorithmValueConsumer`
    */
   Crypto::AlgorithmValueConsumer getAlgorithmValueConsumerForInput(IOType type) {
+    result = this and this.setsValue(type)
+    or
     exists(DataFlow::Node src, DataFlow::Node sink |
       AvcToOperationStepFlow::flow(src, sink) and
-      src.asExpr() = result and
+      src.asIndirectExpr() = result and
       sink = this.getInput(type)
     )
   }
@@ -357,7 +380,7 @@ private class CtxCopyOutArgCall extends CtxPassThroughCall {
 
   CtxCopyOutArgCall() {
     this.getTarget().getName().toLowerCase().matches("%copy%") and
-    n1.asExpr() = this.getAnArgument() and
+    n1.asIndirectExpr() = this.getAnArgument() and
     n1.getType() instanceof CtxType and
     n2.asDefiningArgument() = this.getAnArgument() and
     n2.getType() instanceof CtxType and
@@ -378,16 +401,18 @@ private class CtxCopyReturnCall extends CtxPassThroughCall, CtxPointerExpr {
 
   CtxCopyReturnCall() {
     this.getTarget().getName().toLowerCase().matches("%dup%") and
-    n1.asExpr() = this.getAnArgument() and
+    n1.asIndirectExpr() = this.getAnArgument() and
     n1.getType() instanceof CtxType
   }
 
   override DataFlow::Node getNode1() { result = n1 }
 
-  override DataFlow::Node getNode2() { result.asExpr() = this }
+  override DataFlow::Node getNode2() { result.asIndirectExpr() = this }
 }
 
-// TODO: is this still needed?
+// TODO: is this still needed? It appears to be (tests fail without it) but
+// I don't know why as EVP_PKEY_paramgen is an operation step and we pass through
+// operation steps already.
 /**
  * A call to `EVP_PKEY_paramgen` acts as a kind of pass through.
  * It's output pkey is eventually used in a new operation generating
@@ -401,34 +426,10 @@ private class CtxParamGenCall extends CtxPassThroughCall {
 
   CtxParamGenCall() {
     this.getTarget().getName() = "EVP_PKEY_paramgen" and
-    n1.asExpr() = this.getArgument(0) and
-    (
-      n2.asExpr() = this.getArgument(1)
-      or
-      n2.asDefiningArgument() = this.getArgument(1)
-    )
-  }
-
-  override DataFlow::Node getNode1() { result = n1 }
-
-  override DataFlow::Node getNode2() { result = n2 }
-}
-
-//TODO: I am not sure CallArgToCtxRet is needed anymore
-/**
- * If the current node is an argument to a function
- * that returns a pointer type, immediately flow through.
- * NOTE: this passthrough is required if we allow
- * intermediate steps to go into variables that are not a CTX type.
- * See for example `CtxParamGenCall`.
- */
-private class CallArgToCtxRet extends CtxPassThroughCall, CtxPointerExpr {
-  DataFlow::Node n1;
-  DataFlow::Node n2;
-
-  CallArgToCtxRet() {
-    this.getAnArgument() = n1.asExpr() and
-    n2.asExpr() = this
+    //Arg 0 is *ctx
+    n1.asIndirectExpr() = this.getArgument(0) and
+    //Arg 1 is **pkey
+    n2.asDefiningArgument() = this.getArgument(1)
   }
 
   override DataFlow::Node getNode1() { result = n1 }
@@ -439,7 +440,7 @@ private class CallArgToCtxRet extends CtxPassThroughCall, CtxPointerExpr {
 /**
  * A flow configuration from any non-final `OperationStep` to any other `OperationStep`.
  */
-module OperationStepFlowConfig implements DataFlow::ConfigSig {
+module OperationStepCtxFlowConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) {
     exists(OperationStep s |
       s.getAnOutput() = source or
@@ -455,22 +456,39 @@ module OperationStepFlowConfig implements DataFlow::ConfigSig {
   }
 
   predicate isBarrier(DataFlow::Node node) {
-    exists(CtxClearCall c | c.getAnArgument() = node.asExpr())
+    exists(CtxClearCall c | c.getAnArgument() = [node.asExpr(), node.asIndirectExpr()])
   }
 
   predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    node1.(AdditionalFlowInputStep).getOutput() = node2
+    or
     exists(CtxPassThroughCall c | c.getNode1() = node1 and c.getNode2() = node2)
     or
-    // Flow out through all outputs from an operation step if more than one output
-    // is defined.
-    exists(OperationStep s | s.getAnInput() = node1 and s.getAnOutput() = node2)
+    // Flow only through context and key inputs and outputs
+    // keys and context generally hold unifying context that link multiple steps
+    // Flow only out of finalize operations through key outputs, otherwise stop at final operations
+    exists(OperationStep s, IOType inType, IOType outType |
+      (s.getStepType() = FinalStep() implies outType = KeyIO()) and
+      (
+        inType = ContextIO()
+        or
+        inType = KeyIO()
+      ) and
+      (
+        outType = ContextIO()
+        or
+        outType = KeyIO()
+      ) and
+      s.getInput(inType) = node1 and
+      s.getOutput(outType) = node2
+    )
     // TODO: consideration for additional alises defined as follows:
     // if an output from an operation step itself flows from the output of another operation step
     // then the source of that flow's outputs (all of them) are potential aliases
   }
 }
 
-module OperationStepFlow = DataFlow::Global<OperationStepFlowConfig>;
+module OperationStepCtxFlow = TaintTracking::Global<OperationStepCtxFlowConfig>;
 
 /**
  * A flow from AVC to the first `OperationStep` the AVC reaches as an input.
@@ -483,7 +501,7 @@ module AvcToOperationStepFlowConfig implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { exists(OperationStep s | s.getAnInput() = sink) }
 
   predicate isBarrier(DataFlow::Node node) {
-    exists(CtxClearCall c | c.getAnArgument() = node.asExpr())
+    exists(CtxClearCall c | c.getAnArgument() = [node.asExpr(), node.asIndirectExpr()])
   }
 
   /**
@@ -496,7 +514,7 @@ module AvcToOperationStepFlowConfig implements DataFlow::ConfigSig {
   }
 }
 
-module AvcToOperationStepFlow = DataFlow::Global<AvcToOperationStepFlowConfig>;
+module AvcToOperationStepFlow = TaintTracking::Global<AvcToOperationStepFlowConfig>;
 
 module EncValToInitEncArgConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source.asExpr().getValue().toInt() in [0, 1] }
@@ -506,7 +524,7 @@ module EncValToInitEncArgConfig implements DataFlow::ConfigSig {
   }
 }
 
-module EncValToInitEncArgFlow = DataFlow::Global<EncValToInitEncArgConfig>;
+module EncValToInitEncArgFlow = TaintTracking::Global<EncValToInitEncArgConfig>;
 
 private Crypto::KeyOperationSubtype intToCipherOperationSubtype(int i) {
   i = 0 and

cpp/ql/lib/experimental/quantum/OpenSSL/Operations/SignatureOperation.qll

@@ -6,12 +6,25 @@ private import experimental.quantum.Language
 private import experimental.quantum.OpenSSL.AvcFlow
 private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
 private import experimental.quantum.OpenSSL.Operations.OpenSSLOperations
+private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstances
 
-// TODO: verification functions
 /**
  * A base class for final signature operations.
+ * The operation must be known to always be a signature operation,
+ * and not a MAC operation. Used for both verification and signing.
+ * NOTE: even an operation that may be a mac or signature but is known to take in
+ * only signature configurations should extend `SignatureOrMacFinalOperation`.
  */
-abstract class EvpSignatureFinalOperation extends OperationStep {
+abstract class SignatureFinalOperation extends OperationStep {
+  override OperationStepType getStepType() { result = FinalStep() }
+}
+
+/**
+ * A base class for final signature or MAC operations.
+ * The operation must be known to always be a signature or MAC operation.
+ * Used for both verification or signing.
+ */
+abstract class SignatureOrMacFinalOperation extends OperationStep {
   override OperationStepType getStepType() { result = FinalStep() }
 }
 
@@ -24,36 +37,32 @@ class EvpSignatureDigestInitializer extends OperationStep {
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
     this.getTarget().getName() = "EVP_DigestSignInit_ex" and
-    result.asExpr() = this.getArgument(3) and
+    result.asIndirectExpr() = this.getArgument(3) and
     type = OsslLibContextIO()
     or
-    result.asExpr() = this.getArgument(2) and type = HashAlgorithmIO()
+    result.asIndirectExpr() = this.getArgument(2) and type = HashAlgorithmIO()
     or
     this.getTarget().getName() = "EVP_DigestSignInit" and
-    result.asExpr() = this.getArgument(4) and
+    result.asIndirectExpr() = this.getArgument(4) and
     type = KeyIO()
     or
     this.getTarget().getName() = "EVP_DigestSignInit_ex" and
-    result.asExpr() = this.getArgument(5) and
+    result.asIndirectExpr() = this.getArgument(5) and
     type = KeyIO()
     or
     this.getTarget().getName() = "EVP_DigestSignInit_ex" and
-    result.asExpr() = this.getArgument(6) and
+    result.asIndirectExpr() = this.getArgument(6) and
     type = OsslParamIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
     or
     // EVP_PKEY_CTX
-    result.asExpr() = this.getArgument(1) and type = ContextIO()
-    or
-    this.getTarget().getName() = "EVP_DigestSignInit_ex" and
-    result.asExpr() = this.getArgument(6) and
-    type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(1) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = InitializerStep() }
@@ -66,13 +75,13 @@ class EvpSignInit extends OperationStep {
   EvpSignInit() { this.getTarget().getName() in ["EVP_SignInit", "EVP_SignInit_ex"] }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(1) and type = HashAlgorithmIO()
+    result.asIndirectExpr() = this.getArgument(1) and type = HashAlgorithmIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = InitializerStep() }
@@ -94,22 +103,22 @@ class EvpPkeySignInit extends OperationStep {
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
     this.getTarget().getName() in ["EVP_PKEY_sign_init_ex2", "EVP_PKEY_sign_message_init"] and
-    result.asExpr() = this.getArgument(1) and
+    result.asIndirectExpr() = this.getArgument(1) and
     type = PrimaryAlgorithmIO()
     or
     this.getTarget().getName() = "EVP_PKEY_sign_init_ex" and
-    result.asExpr() = this.getArgument(1) and
+    result.asIndirectExpr() = this.getArgument(1) and
     type = OsslParamIO()
     or
     // Argument 2 (0 based) only exists for EVP_PKEY_sign_init_ex2 and EVP_PKEY_sign_message_init
-    result.asExpr() = this.getArgument(2) and type = OsslParamIO()
+    result.asIndirectExpr() = this.getArgument(2) and type = OsslParamIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = InitializerStep() }
@@ -126,13 +135,13 @@ class EvpSignatureUpdateCall extends OperationStep {
   }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(1) and type = PlaintextIO()
+    result.asIndirectExpr() = this.getArgument(1) and type = PlaintextIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
   }
 
   override OperationStepType getStepType() { result = UpdateStep() }
@@ -141,73 +150,496 @@ class EvpSignatureUpdateCall extends OperationStep {
 /**
  * A call to EVP_SignFinal or EVP_SignFinal_ex.
  */
-class EvpSignFinal extends EvpSignatureFinalOperation {
+class EvpSignFinal extends SignatureFinalOperation {
   EvpSignFinal() { this.getTarget().getName() in ["EVP_SignFinal_ex", "EVP_SignFinal"] }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(3) and type = KeyIO()
+    result.asIndirectExpr() = this.getArgument(3) and type = KeyIO()
     or
     // params above 3 (0-based) only exist for EVP_SignFinal_ex
-    result.asExpr() = this.getArgument(4) and
+    result.asIndirectExpr() = this.getArgument(4) and
     type = OsslLibContextIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(1) and type = SignatureIO()
+    result.asDefiningArgument() = this.getArgument(1) and type = SignatureIO()
+    or
+    result.asDefiningArgument() = this.getArgument(2) and type = SignatureSizeIO()
   }
 }
 
 /**
- * A call to EVP_DigestSign or EVP_PKEY_sign.
+ * A call to EVP_PKEY_sign.
  */
-class EvpDigestSign extends EvpSignatureFinalOperation {
-  EvpDigestSign() { this.getTarget().getName() in ["EVP_DigestSign", "EVP_PKEY_sign"] }
+class EvpPkeySign extends SignatureFinalOperation {
+  EvpPkeySign() {
+    this.getTarget().getName() = "EVP_PKEY_sign" and
+    // Setting signature to NULL is not a final sign step but an
+    // intermediary step used to get the required buffer size.
+    // not tracking these calls.
+    (
+      exists(this.(Call).getArgument(1).getValue())
+      implies
+      this.(Call).getArgument(1).getValue().toInt() != 0
+    )
+  }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(3) and type = PlaintextIO()
+    result.asIndirectExpr() = this.getArgument(3) and type = PlaintextIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(1) and type = SignatureIO()
+    result.asDefiningArgument() = this.getArgument(1) and type = SignatureIO()
   }
 }
 
 /**
- * A call to EVP_DigestSignFinal or EVP_PKEY_sign_message_final.
+ * A call to EVP_DigestSign.
+ * This is a mac or sign operation.
  */
-class EvpDigestAndPkeySignFinal extends EvpSignatureFinalOperation {
-  EvpDigestAndPkeySignFinal() {
-    this.getTarget().getName() in [
-        "EVP_DigestSignFinal",
-        "EVP_PKEY_sign_message_final"
-      ]
-  }
+class EvpDigestSign extends SignatureOrMacFinalOperation {
+  EvpDigestSign() { this.getTarget().getName() = "EVP_DigestSign" }
 
   override DataFlow::Node getInput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asIndirectExpr() = this.getArgument(3) and type = PlaintextIO()
   }
 
   override DataFlow::Node getOutput(IOType type) {
-    result.asExpr() = this.getArgument(0) and type = ContextIO()
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
     or
-    result.asExpr() = this.getArgument(1) and type = SignatureIO()
+    result.asDefiningArgument() = this.getArgument(1) and type = SignatureIO()
+  }
+}
+
+/**
+ * A call to EVP_PKEY_sign_message_final.
+ */
+class EvpPkeySignFinal extends SignatureFinalOperation {
+  EvpPkeySignFinal() {
+    this.getTarget().getName() = "EVP_PKEY_sign_message_final" and
+    // Setting signature to NULL is not a final sign step but an
+    // intermediary step used to get the required buffer size.
+    // not tracking these calls.
+    (
+      exists(this.(Call).getArgument(1).getValue())
+      implies
+      this.(Call).getArgument(1).getValue().toInt() != 0
+    )
+  }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asDefiningArgument() = this.getArgument(1) and type = SignatureIO()
+    or
+    result.asExpr() = this.getArgument(2) and type = SignatureSizeIO()
+  }
+}
+
+/**
+ * A call to EVP_DigestSignFinal.
+ * This is a mac or sign operation.
+ */
+class EvpDigestSignFinal extends SignatureOrMacFinalOperation {
+  EvpDigestSignFinal() {
+    this.getTarget().getName() = "EVP_DigestSignFinal" and
+    // Setting signature to NULL is not a final sign step but an
+    // intermediary step used to get the required buffer size.
+    // not tracking these calls.
+    (
+      exists(this.(Call).getArgument(1).getValue())
+      implies
+      this.(Call).getArgument(1).getValue().toInt() != 0
+    )
+  }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asDefiningArgument() = this.getArgument(1) and type = SignatureIO()
   }
 
   override OperationStepType getStepType() { result = FinalStep() }
 }
 
 /**
- * An EVP signature operation instance.
+ * A call to EVP_DigestVerifyInit or EVP_DigestVerifyInit_ex.
  */
-class EvpSignatureOperationInstance extends Crypto::SignatureOperationInstance instanceof EvpSignatureFinalOperation
+class EvpDigestVerifyInit extends OperationStep {
+  EvpDigestVerifyInit() {
+    this.getTarget().getName() in ["EVP_DigestVerifyInit", "EVP_DigestVerifyInit_ex"]
+  }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asIndirectExpr() = this.getArgument(2) and type = HashAlgorithmIO()
+    or
+    this.getTarget().getName() = "EVP_DigestVerifyInit_ex" and
+    result.asIndirectExpr() = this.getArgument(3) and
+    type = OsslLibContextIO()
+    or
+    this.getTarget().getName() = "EVP_DigestVerifyInit_ex" and
+    result.asIndirectExpr() = this.getArgument(5) and
+    type = KeyIO()
+    or
+    this.getTarget().getName() = "EVP_DigestVerifyInit" and
+    result.asIndirectExpr() = this.getArgument(4) and
+    type = KeyIO()
+    or
+    this.getTarget().getName() = "EVP_DigestVerifyInit_ex" and
+    result.asIndirectExpr() = this.getArgument(6) and
+    type = OsslParamIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asDefiningArgument() = this.getArgument(1) and type = ContextIO()
+  }
+
+  override OperationStepType getStepType() { result = InitializerStep() }
+}
+
+/**
+ * A call to EVP_DigestVerifyUpdate.
+ */
+class EvpDigestVerifyUpdate extends OperationStep {
+  EvpDigestVerifyUpdate() { this.getTarget().getName() = "EVP_DigestVerifyUpdate" }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asIndirectExpr() = this.getArgument(1) and type = PlaintextIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+  }
+
+  override OperationStepType getStepType() { result = UpdateStep() }
+}
+
+/**
+ * A call to EVP_DigestVerifyFinal
+ */
+class EvpDigestVerifyFinal extends SignatureFinalOperation {
+  EvpDigestVerifyFinal() { this.getTarget().getName() = "EVP_DigestVerifyFinal" }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asIndirectExpr() = this.getArgument(1) and type = SignatureIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+  }
+}
+
+/**
+ * A call to EVP_DigestVerify
+ */
+class EvpDigestVerify extends SignatureFinalOperation {
+  EvpDigestVerify() { this.getTarget().getName() = "EVP_DigestVerify" }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asIndirectExpr() = this.getArgument(1) and type = SignatureIO()
+    or
+    result.asIndirectExpr() = this.getArgument(3) and type = PlaintextIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+  }
+}
+
+/**
+ * A call to `EVP_PKEY_verify_init`, `EVP_PKEY_verify_init_ex`,
+ * `EVP_PKEY_verify_init_ex2`, or `EVP_PKEY_verify_message_init`
+ * https://docs.openssl.org/master/man3/EVP_PKEY_verify/#synopsis
+ */
+class EvpVerifyInit extends OperationStep {
+  EvpVerifyInit() {
+    this.getTarget().getName() in [
+        "EVP_PKEY_verify_init", "EVP_PKEY_verify_init_ex", "EVP_PKEY_verify_init_ex2",
+        "EVP_PKEY_verify_message_init"
+      ]
+  }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+    or
+    this.getTarget().getName() = "EVP_PKEY_verify_init_ex" and
+    result.asIndirectExpr() = this.getArgument(1) and
+    type = OsslParamIO()
+    or
+    this.getTarget().getName() in ["EVP_PKEY_verify_init_ex2", "EVP_PKEY_verify_message_init"] and
+    result.asIndirectExpr() = this.getArgument(1) and
+    type = PrimaryAlgorithmIO()
+    or
+    this.getTarget().getName() in ["EVP_PKEY_verify_init_ex2", "EVP_PKEY_verify_message_init"] and
+    result.asIndirectExpr() = this.getArgument(2) and
+    type = OsslParamIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+  }
+
+  override OperationStepType getStepType() { result = InitializerStep() }
+}
+
+/**
+ * A call to `EVP_PKEY_CTX_set_signature`
+ * https://docs.openssl.org/master/man3/EVP_PKEY_verify/
+ */
+class EvpCtxSetSignatureInitializer extends OperationStep {
+  EvpCtxSetSignatureInitializer() { this.getTarget().getName() = "EVP_PKEY_CTX_set_signature" }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asIndirectExpr() = this.getArgument(1) and type = SignatureIO()
+    or
+    result.asExpr() = this.getArgument(2) and type = SignatureSizeIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+  }
+
+  override OperationStepType getStepType() { result = InitializerStep() }
+}
+
+/**
+ * A call to `EVP_PKEY_verify_message_update`.
+ */
+class EvpVerifyMessageUpdate extends OperationStep {
+  EvpVerifyMessageUpdate() { this.getTarget().getName() = "EVP_PKEY_verify_message_update" }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asIndirectExpr() = this.getArgument(1) and type = PlaintextIO()
+    or
+    result.asExpr() = this.getArgument(2) and type = PlaintextSizeIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+  }
+
+  override OperationStepType getStepType() { result = UpdateStep() }
+}
+
+/**
+ * A call to `EVP_PKEY_verify_message_final`.
+ */
+class EvpVerifyMessageFinal extends SignatureFinalOperation {
+  EvpVerifyMessageFinal() { this.getTarget().getName() = "EVP_PKEY_verify_message_final" }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+  }
+}
+
+/**
+ * A call to `EVP_PKEY_verify`
+ */
+class EvpVerify extends SignatureFinalOperation {
+  EvpVerify() { this.getTarget().getName() = "EVP_PKEY_verify" }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asIndirectExpr() = this.getArgument(1) and type = SignatureIO()
+    or
+    result.asExpr() = this.getArgument(2) and type = SignatureSizeIO()
+    or
+    result.asIndirectExpr() = this.getArgument(3) and type = PlaintextIO()
+    or
+    result.asExpr() = this.getArgument(4) and type = PlaintextSizeIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+  }
+}
+
+/**
+ * A call to `RSA_sign` or `RSA_verify`.
+ * https://docs.openssl.org/3.0/man3/RSA_sign/
+ */
+class RsaSignorVerify extends SignatureFinalOperation {
+  RsaSignorVerify() { this.getTarget().getName() in ["RSA_sign", "RSA_verify"] }
+
+  override DataFlow::Node getInput(IOType type) {
+    // Arg 0 is an NID (so asExpr not asIndirectExpr)
+    result.asExpr() = this.getArgument(0) and type = HashAlgorithmIO()
+    or
+    result.asIndirectExpr() = this.getArgument(1) and type = PlaintextIO()
+    or
+    result.asExpr() = this.getArgument(2) and type = PlaintextSizeIO()
+    or
+    this.getTarget().getName() = "RSA_verify" and
+    result.asIndirectExpr() = this.getArgument(3) and
+    type = SignatureIO()
+    or
+    this.getTarget().getName() = "RSA_verify" and
+    result.asIndirectExpr() = this.getArgument(4) and
+    type = SignatureSizeIO()
+    or
+    result.asIndirectExpr() = this.getArgument(5) and type = KeyIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+    or
+    this.getTarget().getName() = "RSA_sign" and
+    result.asDefiningArgument() = this.getArgument(3) and
+    type = SignatureIO()
+    or
+    this.getTarget().getName() = "RSA_sign" and
+    type = SignatureSizeIO() and
+    result.asDefiningArgument() = this.getArgument(4)
+  }
+}
+
+/**
+ * A call to `DSA_do_sign` or `DSA_do_verify`
+ */
+class DsaDoSignOrVerify extends SignatureFinalOperation {
+  DsaDoSignOrVerify() { this.getTarget().getName() in ["DSA_do_sign", "DSA_do_verify"] }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = PlaintextIO()
+    or
+    result.asExpr() = this.getArgument(1) and type = PlaintextSizeIO()
+    or
+    this.getTarget().getName() = "DSA_do_sign" and
+    result.asIndirectExpr() = this.getArgument(2) and
+    type = KeyIO()
+    or
+    this.getTarget().getName() = "DSA_do_verify" and
+    result.asIndirectExpr() = this.getArgument(2) and
+    type = SignatureIO()
+    or
+    this.getTarget().getName() = "DSA_do_verify" and
+    result.asIndirectExpr() = this.getArgument(3) and
+    type = KeyIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    this.getTarget().getName() = "DSA_do_sign" and
+    result.asIndirectExpr() = this and
+    type = SignatureIO()
+  }
+}
+
+/**
+ * A Call to `EVP_VerifyInit` or `EVP_VerifyInit_ex`
+ * - int EVP_VerifyInit_ex(EVP_MD_CTX *ctx, const EVP_MD *type, ENGINE *impl);
+ * - int EVP_VerifyInit(EVP_MD_CTX *ctx, const EVP_MD *type);
+ */
+class EVP_VerifyInitCall extends OperationStep {
+  EVP_VerifyInitCall() { this.getTarget().getName() in ["EVP_VerifyInit", "EVP_VerifyInit_ex"] }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asIndirectExpr() = this.getArgument(1) and type = HashAlgorithmIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+  }
+
+  override OperationStepType getStepType() { result = InitializerStep() }
+}
+
+/**
+ * A call to `EVP_VerifyUpdate`
+ * - int EVP_VerifyUpdate(EVP_MD_CTX *ctx, const void *d, unsigned int cnt);
+ */
+class EVP_VerifyUpdateCall extends OperationStep {
+  EVP_VerifyUpdateCall() { this.getTarget().getName() = "EVP_VerifyUpdate" }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asIndirectExpr() = this.getArgument(1) and type = PlaintextIO()
+    or
+    result.asIndirectExpr() = this.getArgument(2) and type = PlaintextSizeIO()
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+  }
+
+  override OperationStepType getStepType() { result = UpdateStep() }
+}
+
+/**
+ * A call to `EVP_VerifyFinal` or `EVP_VerifyFinal_ex`
+ * - int EVP_VerifyFinal_ex(EVP_MD_CTX *ctx, const unsigned char *sigbuf,
+ *                       unsigned int siglen, EVP_PKEY *pkey,
+ *                       OSSL_LIB_CTX *libctx, const char *propq);
+ *- int EVP_VerifyFinal(EVP_MD_CTX *ctx, unsigned char *sigbuf, unsigned int siglen,
+ *                    EVP_PKEY *pkey);                       *
+ */
+class EVP_VerifyFinalCall extends SignatureFinalOperation {
+  EVP_VerifyFinalCall() { this.getTarget().getName() in ["EVP_VerifyFinal", "EVP_VerifyFinal_ex"] }
+
+  override DataFlow::Node getInput(IOType type) {
+    result.asIndirectExpr() = this.getArgument(0) and type = ContextIO()
+    or
+    result.asIndirectExpr() = this.getArgument(1) and type = SignatureIO()
+    or
+    result.asExpr() = this.getArgument(2) and type = SignatureSizeIO()
+    or
+    result.asIndirectExpr() = this.getArgument(3) and type = KeyIO()
+    or
+    result.asIndirectExpr() = this.getArgument(4) and type = OsslLibContextIO()
+    // TODO: arg 5 propq?
+  }
+
+  override DataFlow::Node getOutput(IOType type) {
+    result.asDefiningArgument() = this.getArgument(0) and type = ContextIO()
+  }
+
+  override OperationStepType getStepType() { result = FinalStep() }
+}
+
+/**
+ * An instance of a signature operation.
+ * This is an OpenSSL specific class that extends the base SignatureOperationInstance.
+ */
+class OpenSslSignatureOperationInstance extends Crypto::SignatureOperationInstance instanceof SignatureFinalOperation
 {
   override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
     super.getPrimaryAlgorithmValueConsumer() = result
@@ -217,7 +649,7 @@ class EvpSignatureOperationInstance extends Crypto::SignatureOperationInstance i
    * Signing, verification or unknown.
    */
   override Crypto::KeyOperationSubtype getKeyOperationSubtype() {
-    // TODO: if this KeyOperationSubtype does not match initialization call's KeyOperationSubtype then we found a bug
+    // NOTE: if this KeyOperationSubtype does not match initialization call's KeyOperationSubtype then we found a bug
     if super.getTarget().getName().toLowerCase().matches("%sign%")
     then result instanceof Crypto::TSignMode
     else
@@ -227,14 +659,70 @@ class EvpSignatureOperationInstance extends Crypto::SignatureOperationInstance i
   }
 
   override Crypto::ConsumerInputDataFlowNode getNonceConsumer() {
-    // TODO: some signing operations may have explicit nonce generators
-    none()
+    // some signing operations may have explicit nonce generators
+    super.getDominatingInitializersToStep(IVorNonceIO()).getInput(IVorNonceIO()) = result
+  }
+
+  override Crypto::ConsumerInputDataFlowNode getKeyConsumer() {
+    super.getDominatingInitializersToStep(KeyIO()).getInput(KeyIO()) = result
+  }
+
+  override Crypto::ConsumerInputDataFlowNode getSignatureConsumer() {
+    super.getDominatingInitializersToStep(SignatureIO()).getInput(SignatureIO()) = result
+  }
+
+  override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() {
+    super.getOutputStepFlowingToStep(SignatureIO()).getOutput(SignatureIO()) = result
+  }
+
+  override Crypto::ConsumerInputDataFlowNode getInputConsumer() {
+    super.getDominatingInitializersToStep(PlaintextIO()).getInput(PlaintextIO()) = result
+  }
+
+  override Crypto::AlgorithmValueConsumer getHashAlgorithmValueConsumer() {
+    super
+        .getDominatingInitializersToStep(HashAlgorithmIO())
+        .getAlgorithmValueConsumerForInput(HashAlgorithmIO()) = result
+    or
+    // Handle cases where the hash is set through the primary algorithm
+    // RSA-SHA256 for example
+    // NOTE: assuming the hash would not be overridden, or if it is it is undefined
+    // i.e., if the above dominating initializer exists and the primary algorithm
+    // specifies a hash, consider both valid hash AVCs.
+    // TODO: can this behavior be build into the get dominating initializers?
+    super.getPrimaryAlgorithmValueConsumer() = result and
+    exists(OpenSslAlgorithmInstance i |
+      i.getAvc() = result and i instanceof Crypto::HashAlgorithmInstance
+    )
+  }
+
+  override predicate hasHashAlgorithmConsumer() {
+    exists(super.getDominatingInitializersToStep(HashAlgorithmIO()))
+  }
+}
+
+/**
+ * A class for signature or MAC operation instances.
+ * This is an OpenSSL specific class that extends the base SignatureOrMacOperationInstance.
+ */
+class OpenSslSignatureOrMacOperationInstance extends Crypto::SignatureOrMacOperationInstance instanceof SignatureOrMacFinalOperation
+{
+  override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
+    super.getPrimaryAlgorithmValueConsumer() = result
   }
 
   /**
-   * Keys provided in the initialization call or in a context are found by this method.
-   * Keys in explicit arguments are found by overridden methods in extending classes.
+   * Signing, verification or unknown.
    */
+  override Crypto::KeyOperationSubtype getKeyOperationSubtype() {
+    result instanceof Crypto::TSignMode or result instanceof Crypto::TMacMode
+  }
+
+  override Crypto::ConsumerInputDataFlowNode getNonceConsumer() {
+    // some signing operations may have explicit nonce generators
+    super.getDominatingInitializersToStep(IVorNonceIO()).getInput(IVorNonceIO()) = result
+  }
+
   override Crypto::ConsumerInputDataFlowNode getKeyConsumer() {
     super.getDominatingInitializersToStep(KeyIO()).getInput(KeyIO()) = result
   }
@@ -247,14 +735,24 @@ class EvpSignatureOperationInstance extends Crypto::SignatureOperationInstance i
     super.getDominatingInitializersToStep(PlaintextIO()).getInput(PlaintextIO()) = result
   }
 
-  /**
-   * TODO: only signing operations for now, change when verificaiton is added
-   */
-  override Crypto::ConsumerInputDataFlowNode getSignatureConsumer() { none() }
-
   override Crypto::AlgorithmValueConsumer getHashAlgorithmValueConsumer() {
     super
         .getDominatingInitializersToStep(HashAlgorithmIO())
         .getAlgorithmValueConsumerForInput(HashAlgorithmIO()) = result
+    or
+    // Handle cases where the hash is set through the primary algorithm
+    // RSA-SHA256 for example
+    // NOTE: assuming the hash would not be overridden, or if it is it is undefined
+    // i.e., if the above dominating initializer exists and the primary algorithm
+    // specifies a hash, consider both valid hash AVCs.
+    // TODO: can this behavior be build into the get dominating initializers?
+    super.getPrimaryAlgorithmValueConsumer() = result and
+    exists(OpenSslAlgorithmInstance i |
+      i.getAvc() = result and i instanceof Crypto::HashAlgorithmInstance
+    )
+  }
+
+  override predicate hasHashAlgorithmConsumer() {
+    exists(super.getDominatingInitializersToStep(HashAlgorithmIO()))
   }
 }

cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/RangeAnalysis.qll

@@ -412,7 +412,7 @@ private predicate boundFlowStepPhi(
   or
   exists(IRGuardCondition guard, boolean testIsTrue |
     guard = boundFlowCond(valueNumberOfOperand(op2), op1, delta, upper, testIsTrue) and
-    guard.controlsEdge(op2.getPredecessorBlock(), op2.getUse().getBlock(), testIsTrue) and
+    guard.controlsBranchEdge(op2.getPredecessorBlock(), op2.getUse().getBlock(), testIsTrue) and
     reason = TCondReason(guard)
   )
 }

cpp/ql/lib/qlpack.yml

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

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

@@ -87,6 +87,7 @@ class ElementBase extends @element {
  */
 class Element extends ElementBase {
   /** Gets the primary file where this element occurs. */
+  pragma[nomagic]
   File getFile() { result = this.getLocation().getFile() }
 
   /**

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

@@ -498,7 +498,9 @@ class FinalParameterUse extends UseImpl, TFinalParameterUse {
 
   int getArgumentIndex() { result = p.getIndex() }
 
-  override Node getNode() { finalParameterNodeHasParameterAndIndex(result, p, indirectionIndex) }
+  override FinalParameterNode getNode() {
+    finalParameterNodeHasParameterAndIndex(result, p, indirectionIndex)
+  }
 
   override int getIndirection() { result = indirectionIndex + 1 }
 
@@ -1000,7 +1002,7 @@ private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationI
     result instanceof FalseEdge
   }
 
-  class GuardValue = Boolean;
+  class GuardValue = IRGuards::GuardValue;
 
   class Guard instanceof IRGuards::IRGuardCondition {
     string toString() { result = super.toString() }
@@ -1008,7 +1010,7 @@ private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationI
     predicate hasValueBranchEdge(IRCfg::BasicBlock bb1, IRCfg::BasicBlock bb2, GuardValue branch) {
       exists(EdgeKind kind |
         super.getBlock() = bb1 and
-        kind = getConditionalEdge(branch) and
+        kind = getConditionalEdge(branch.asBooleanValue()) and
         bb1.getSuccessor(kind) = bb2
       )
     }
@@ -1021,7 +1023,7 @@ private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationI
   }
 
   predicate guardDirectlyControlsBlock(Guard guard, IRCfg::BasicBlock bb, GuardValue branch) {
-    guard.(IRGuards::IRGuardCondition).controls(bb, branch)
+    guard.(IRGuards::IRGuardCondition).valueControls(bb, branch)
   }
 
   predicate keepAllPhiInputBackEdges() { any() }
@@ -1048,25 +1050,35 @@ module BarrierGuardWithIntParam<guardChecksNodeSig/4 guardChecksNode> {
     )
   }
 
-  private predicate guardChecks(
-    DataFlowIntegrationInput::Guard g, SsaImpl::Definition def,
-    DataFlowIntegrationInput::GuardValue branch, int indirectionIndex
+  private predicate guardChecksInstr(
+    IRGuards::Guards_v1::Guard g, IRGuards::GuardsInput::Expr instr, boolean branch,
+    int indirectionIndex
   ) {
-    exists(UseImpl use |
-      guardChecksNode(g, use.getNode(), branch, indirectionIndex) and
-      ssaDefReachesCertainUse(def, use)
+    exists(Node node |
+      nodeHasInstruction(node, instr, indirectionIndex) and
+      guardChecksNode(g, node, branch, indirectionIndex)
     )
   }
 
+  private predicate guardChecksWithWrappers(
+    DataFlowIntegrationInput::Guard g, SsaImpl::Definition def, IRGuards::GuardValue val,
+    int indirectionIndex
+  ) {
+    IRGuards::Guards_v1::ValidationWrapperWithState<int, guardChecksInstr/4>::guardChecksDef(g, def,
+      val, indirectionIndex)
+  }
+
   Node getABarrierNode(int indirectionIndex) {
     // Only get the SynthNodes from the shared implementation, as the ExprNodes cannot
     // be matched on SourceVariable.
     result.(SsaSynthNode).getSynthNode() =
-      DataFlowIntegrationImpl::BarrierGuardDefWithState<int, guardChecks/4>::getABarrierNode(indirectionIndex)
+      DataFlowIntegrationImpl::BarrierGuardDefWithState<int, guardChecksWithWrappers/4>::getABarrierNode(indirectionIndex)
     or
     // Calculate the guarded UseImpls corresponding to ExprNodes directly.
-    exists(DataFlowIntegrationInput::Guard g, boolean branch, Definition def, IRBlock bb |
-      guardChecks(g, def, branch, indirectionIndex) and
+    exists(
+      DataFlowIntegrationInput::Guard g, IRGuards::GuardValue branch, Definition def, IRBlock bb
+    |
+      guardChecksWithWrappers(g, def, branch, indirectionIndex) and
       exists(UseImpl use |
         ssaDefReachesCertainUse(def, use) and
         use.getBlock() = bb and
@@ -1124,7 +1136,15 @@ predicate ssaFlow(Node nodeFrom, Node nodeTo) {
  */
 class PhiNode extends Definition instanceof SsaImpl::PhiNode {
   /** Gets a definition that is an input to this phi node. */
-  final Definition getAnInput() { phiHasInputFromBlock(this, result, _) }
+  final Definition getAnInput() { this.hasInputFromBlock(result, _) }
+
+  /**
+   * Holds if `input` is an input to this phi node along the edge originating
+   * in `bb`.
+   */
+  final predicate hasInputFromBlock(Definition input, IRBlock bb) {
+    phiHasInputFromBlock(this, input, bb)
+  }
 }
 
 /** An static single assignment (SSA) definition. */
@@ -1149,10 +1169,53 @@ class Definition extends SsaImpl::Definition {
     exists(SourceVariable sv, IRBlock bb, int i, UseImpl use |
       ssaDefReachesRead(sv, this, bb, i) and
       use.hasIndexInBlock(bb, i, sv) and
-      result = use.getNode().asOperand()
+      use = TDirectUseImpl(result, 0)
     )
   }
 
+  /**
+   * Holds if this definition defines the parameter `p` upon entry into the
+   * enclosing function.
+   */
+  pragma[nomagic]
+  predicate isParameterDefinition(Parameter p) {
+    this.getIndirectionIndex() = 0 and
+    getDefImpl(this).getValue().asInstruction().(InitializeParameterInstruction).getParameter() = p
+  }
+
+  /**
+   * Holds if this definition defines the `indirectionIndex`'th indirection of
+   * parameter `p` upon entry into the enclosing function.
+   */
+  pragma[nomagic]
+  predicate isIndirectParameterDefinition(Parameter p, int indirectionIndex) {
+    this.getIndirectionIndex() = indirectionIndex and
+    indirectionIndex > 0 and
+    getDefImpl(this).getValue().asInstruction().(InitializeParameterInstruction).getParameter() = p
+  }
+
+  /**
+   * Holds if this definition defines the implicit `this` parameter upon entry into
+   * the enclosing member function.
+   */
+  pragma[nomagic]
+  predicate isThisDefinition() {
+    this.getIndirectionIndex() = 0 and
+    getDefImpl(this).getValue().asInstruction().(InitializeParameterInstruction).hasIndex(-1)
+  }
+
+  /**
+   * Holds if this definition defines the implicit `*this` parameter (i.e., the
+   * indirection of the `this` parameter) upon entry into the enclosing member
+   * function.
+   */
+  pragma[nomagic]
+  predicate isIndirectThisDefinition(int indirectionIndex) {
+    this.getIndirectionIndex() = indirectionIndex and
+    indirectionIndex > 0 and
+    getDefImpl(this).getValue().asInstruction().(InitializeParameterInstruction).hasIndex(-1)
+  }
+
   /**
    * Gets an `Operand` that represents an indirect use of this definition.
    *
@@ -1167,10 +1230,11 @@ class Definition extends SsaImpl::Definition {
    * value that was defined by the definition.
    */
   Operand getAnIndirectUse(int indirectionIndex) {
+    indirectionIndex > 0 and
     exists(SourceVariable sv, IRBlock bb, int i, UseImpl use |
       ssaDefReachesRead(sv, this, bb, i) and
       use.hasIndexInBlock(bb, i, sv) and
-      result = use.getNode().asIndirectOperand(indirectionIndex)
+      use = TDirectUseImpl(result, indirectionIndex)
     )
   }
 

cpp/ql/lib/semmle/code/cpp/ir/implementation/EdgeKind.qll

@@ -52,11 +52,18 @@ class GotoEdge extends EdgeKindImpl, TGotoEdge {
   final override string toString() { result = "Goto" }
 }
 
+/**
+ * A "true" or "false" edge representing a successor of a conditional branch.
+ */
+abstract private class BooleanEdgeKindImpl extends EdgeKindImpl { }
+
+final class BooleanEdge = BooleanEdgeKindImpl;
+
 /**
  * A "true" edge, representing the successor of a conditional branch when the
  * condition is non-zero.
  */
-class TrueEdge extends EdgeKindImpl, TTrueEdge {
+class TrueEdge extends BooleanEdgeKindImpl, TTrueEdge {
   final override string toString() { result = "True" }
 }
 
@@ -64,7 +71,7 @@ class TrueEdge extends EdgeKindImpl, TTrueEdge {
  * A "false" edge, representing the successor of a conditional branch when the
  * condition is zero.
  */
-class FalseEdge extends EdgeKindImpl, TFalseEdge {
+class FalseEdge extends BooleanEdgeKindImpl, TFalseEdge {
   final override string toString() { result = "False" }
 }
 
@@ -95,19 +102,48 @@ class SehExceptionEdge extends ExceptionEdgeImpl, TSehExceptionEdge {
   final override string toString() { result = "SEH Exception" }
 }
 
+/**
+ * An edge from a `Switch` instruction to one of the cases, or to the default
+ * branch.
+ */
+abstract private class SwitchEdgeKindImpl extends EdgeKindImpl {
+  /**
+   * Gets the smallest value of the switch expression for which control will flow along this edge.
+   */
+  string getMinValue() { none() }
+
+  /**
+   * Gets the largest value of the switch expression for which control will flow along this edge.
+   */
+  string getMaxValue() { none() }
+
+  /**
+   * Gets the unique value of the switch expression for which control will
+   * flow along this edge, if any.
+   */
+  final string getValue() { result = unique( | | [this.getMinValue(), this.getMaxValue()]) }
+
+  /** Holds if this edge is the default edge. */
+  predicate isDefault() { none() }
+}
+
+final class SwitchEdge = SwitchEdgeKindImpl;
+
 /**
  * A "default" edge, representing the successor of a `Switch` instruction when
  * none of the case values matches the condition value.
  */
-class DefaultEdge extends EdgeKindImpl, TDefaultEdge {
+class DefaultEdge extends SwitchEdgeKindImpl, TDefaultEdge {
   final override string toString() { result = "Default" }
+
+  final override predicate isDefault() { any() }
 }
 
 /**
  * A "case" edge, representing the successor of a `Switch` instruction when the
  * the condition value matches a corresponding `case` label.
  */
-class CaseEdge extends EdgeKindImpl, TCaseEdge {
+class CaseEdge extends SwitchEdgeKindImpl, TCaseEdge {
   string minValue;
   string maxValue;
 
@@ -119,24 +155,9 @@ class CaseEdge extends EdgeKindImpl, TCaseEdge {
     else result = "Case[" + minValue + ".." + maxValue + "]"
   }
 
-  /**
-   * Gets the smallest value of the switch expression for which control will flow along this edge.
-   */
-  final string getMinValue() { result = minValue }
+  final override string getMinValue() { result = minValue }
 
-  /**
-   * Gets the largest value of the switch expression for which control will flow along this edge.
-   */
-  final string getMaxValue() { result = maxValue }
-
-  /**
-   * Gets the unique value of the switch expression for which control will
-   * flow along this edge, if any.
-   */
-  final string getValue() {
-    minValue = maxValue and
-    result = minValue
-  }
+  final override string getMaxValue() { result = maxValue }
 }
 
 /**

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

@@ -1084,6 +1084,12 @@ class BinaryInstruction extends Instruction {
     or
     op1 = this.getRightOperand() and op2 = this.getLeftOperand()
   }
+
+  /**
+   * Gets the instruction whose result provides the value of the left or right
+   * operand of this binary instruction.
+   */
+  Instruction getAnInput() { result = this.getLeft() or result = this.getRight() }
 }
 
 /**

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

@@ -1084,6 +1084,12 @@ class BinaryInstruction extends Instruction {
     or
     op1 = this.getRightOperand() and op2 = this.getLeftOperand()
   }
+
+  /**
+   * Gets the instruction whose result provides the value of the left or right
+   * operand of this binary instruction.
+   */
+  Instruction getAnInput() { result = this.getLeft() or result = this.getRight() }
 }
 
 /**

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

@@ -1084,6 +1084,12 @@ class BinaryInstruction extends Instruction {
     or
     op1 = this.getRightOperand() and op2 = this.getLeftOperand()
   }
+
+  /**
+   * Gets the instruction whose result provides the value of the left or right
+   * operand of this binary instruction.
+   */
+  Instruction getAnInput() { result = this.getLeft() or result = this.getRight() }
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/rangeanalysis/RangeAnalysisUtils.qll

@@ -1,10 +1,10 @@
 import cpp
 
 /**
- * Describes whether a relation is 'strict' (that is, a `<` or `>`
+ * The strictness of a relation. Either 'strict' (that is, a `<` or `>`
  * relation) or 'non-strict' (a `<=` or `>=` relation).
  */
-newtype RelationStrictness =
+newtype TRelationStrictness =
   /**
    * Represents that a relation is 'strict' (that is, a `<` or `>` relation).
    */
@@ -14,6 +14,19 @@ newtype RelationStrictness =
    */
   Nonstrict()
 
+/**
+ * The strictness of a relation. Either 'strict' (that is, a `<` or `>`
+ * relation) or 'non-strict' (a `<=` or `>=` relation).
+ */
+class RelationStrictness extends TRelationStrictness {
+  /** Gets the string representation of this relation strictness. */
+  string toString() {
+    this = Strict() and result = "strict"
+    or
+    this = Nonstrict() and result = "non-strict"
+  }
+}
+
 /**
  * Describes whether a relation is 'greater' (that is, a `>` or `>=`
  * relation) or 'lesser' (a `<` or `<=` relation).
@@ -105,10 +118,10 @@ predicate relOpWithSwap(
  *
  * This allows for the relation to be either as written, or with its
  * arguments reversed; for example, if `rel` is `x < 5` then
- * `relOpWithSwapAndNegate(rel, x, 5, Lesser(), Strict(), true)`,
- * `relOpWithSwapAndNegate(rel, 5, x, Greater(), Strict(), true)`,
- * `relOpWithSwapAndNegate(rel, x, 5, Greater(), Nonstrict(), false)` and
- * `relOpWithSwapAndNegate(rel, 5, x, Lesser(), Nonstrict(), false)` hold.
+ * - `relOpWithSwapAndNegate(rel, x, 5, Lesser(), Strict(), true)`,
+ * - `relOpWithSwapAndNegate(rel, 5, x, Greater(), Strict(), true)`,
+ * - `relOpWithSwapAndNegate(rel, x, 5, Greater(), Nonstrict(), false)` and
+ * - `relOpWithSwapAndNegate(rel, 5, x, Lesser(), Nonstrict(), false)` hold.
  */
 predicate relOpWithSwapAndNegate(
   RelationalOperation rel, Expr a, Expr b, RelationDirection dir, RelationStrictness strict,

cpp/ql/lib/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll

@@ -93,31 +93,42 @@ private float wideningUpperBounds(ArithmeticType t) {
   result = 1.0 / 0.0 // +Inf
 }
 
+/** Gets the widened lower bound for a given type and lower bound. */
+bindingset[type, lb]
+float widenLowerBound(Type type, float lb) {
+  result = max(float widenLB | widenLB = wideningLowerBounds(type) and widenLB <= lb | widenLB)
+}
+
+/** Gets the widened upper bound for a given type and upper bound. */
+bindingset[type, ub]
+float widenUpperBound(Type type, float ub) {
+  result = min(float widenUB | widenUB = wideningUpperBounds(type) and widenUB >= ub | widenUB)
+}
+
 /**
  * Gets the value of the expression `e`, if it is a constant.
  * This predicate also handles the case of constant variables initialized in different
  * compilation units, which doesn't necessarily have a getValue() result from the extractor.
  */
 private string getValue(Expr e) {
-  if exists(e.getValue())
-  then result = e.getValue()
-  else
-    /*
-     * It should be safe to propagate the initialization value to a variable if:
-     * The type of v is const, and
-     * The type of v is not volatile, and
-     * Either:
-     *   v is a local/global variable, or
-     *   v is a static member variable
-     */
+  result = e.getValue()
+  or
+  not exists(e.getValue()) and
+  /*
+   * It should be safe to propagate the initialization value to a variable if:
+   * The type of v is const, and
+   * The type of v is not volatile, and
+   * Either:
+   *   v is a local/global variable, or
+   *   v is a static member variable
+   */
 
-    exists(VariableAccess access, StaticStorageDurationVariable v |
-      not v.getUnderlyingType().isVolatile() and
-      v.getUnderlyingType().isConst() and
-      e = access and
-      v = access.getTarget() and
-      result = getValue(v.getAnAssignedValue())
-    )
+  exists(StaticStorageDurationVariable v |
+    not v.getUnderlyingType().isVolatile() and
+    v.getUnderlyingType().isConst() and
+    v = e.(VariableAccess).getTarget() and
+    result = getValue(v.getAnAssignedValue())
+  )
 }
 
 /**
@@ -505,6 +516,328 @@ private predicate isRecursiveExpr(Expr e) {
   )
 }
 
+/**
+ * Provides predicates that estimate the number of bounds that the range
+ * analysis might produce.
+ */
+private module BoundsEstimate {
+  /**
+   * Gets the limit beyond which we enable widening. That is, if the estimated
+   * number of bounds exceeds this limit, we enable widening such that the limit
+   * will not be reached.
+   */
+  float getBoundsLimit() {
+    // This limit is arbitrary, but low enough that it prevents timeouts on
+    // specific observed customer databases (and the in the tests).
+    result = 2.0.pow(40)
+  }
+
+  /** Gets the maximum number of bounds possible for `t` when widening is used. */
+  private int getNrOfWideningBounds(ArithmeticType t) {
+    result = strictcount(wideningLowerBounds(t)).maximum(strictcount(wideningUpperBounds(t)))
+  }
+
+  /**
+   * Holds if `boundFromGuard(guard, v, _, branch)` holds, but without
+   * relying on range analysis (which would cause non-monotonic recursion
+   * elsewhere).
+   */
+  private predicate hasBoundFromGuard(Expr guard, VariableAccess v, boolean branch) {
+    exists(Expr lhs | linearAccess(lhs, v, _, _) |
+      relOpWithSwapAndNegate(guard, lhs, _, _, _, branch)
+      or
+      eqOpWithSwapAndNegate(guard, lhs, _, true, branch)
+      or
+      eqZeroWithNegate(guard, lhs, true, branch)
+    )
+  }
+
+  /** Holds if `def` is a guard phi node for `v` with a bound from a guard. */
+  predicate isGuardPhiWithBound(RangeSsaDefinition def, StackVariable v, VariableAccess access) {
+    exists(Expr guard, boolean branch |
+      def.isGuardPhi(v, access, guard, branch) and
+      hasBoundFromGuard(guard, access, branch)
+    )
+  }
+
+  /**
+   * Gets the number of bounds for `def` when `def` is a guard phi node for the
+   * variable `v`.
+   */
+  language[monotonicAggregates]
+  private float nrOfBoundsPhiGuard(RangeSsaDefinition def, StackVariable v) {
+    // If we have
+    //
+    //   if (x < c) { e1 }
+    //   e2
+    //
+    // then `e2` is both a guard phi node (guarded by `x < c`) and a normal
+    // phi node (control is merged after the `if` statement).
+    //
+    // Assume `x` has `n` bounds. Then `n` bounds are propagated to the guard
+    // phi node `{ e1 }` and, since `{ e1 }` is input to `e2` as a normal phi
+    // node, `n` bounds are propagated to `e2`. If we also propagate the `n`
+    // bounds to `e2` as a guard phi node, then we square the number of
+    // bounds.
+    //
+    // However in practice `x < c` is going to cut down the number of bounds:
+    // The tracked bounds can't flow to both branches as that would require
+    // them to simultaneously be greater and smaller than `c`. To approximate
+    // this better, the contribution from a guard phi node that is also a
+    // normal phi node is 1.
+    exists(def.getAPhiInput(v)) and
+    isGuardPhiWithBound(def, v, _) and
+    result = 1
+    or
+    not exists(def.getAPhiInput(v)) and
+    // If there's different `access`es, then they refer to the same variable
+    // with the same lower bounds. Hence adding these guards make no sense (the
+    // implementation will take the union but they'll be removed by
+    // deduplication). Hence we use `max` as an approximation.
+    result =
+      max(VariableAccess access | isGuardPhiWithBound(def, v, access) | nrOfBoundsExpr(access))
+    or
+    def.isPhiNode(v) and
+    not isGuardPhiWithBound(def, v, _) and
+    result = 0
+  }
+
+  /**
+   * Gets the number of bounds for `def` when `def` is a normal phi node for the
+   * variable `v`.
+   */
+  language[monotonicAggregates]
+  private float nrOfBoundsPhiNormal(RangeSsaDefinition def, StackVariable v) {
+    result =
+      strictsum(RangeSsaDefinition inputDef |
+        inputDef = def.getAPhiInput(v)
+      |
+        nrOfBoundsDef(inputDef, v)
+      )
+    or
+    def.isPhiNode(v) and
+    not exists(def.getAPhiInput(v)) and
+    result = 0
+  }
+
+  /**
+   * Gets the number of bounds for `def` when `def` is an NE phi node for the
+   * variable `v`.
+   */
+  private float nrOfBoundsNEPhi(RangeSsaDefinition def, StackVariable v) {
+    exists(VariableAccess access | isNEPhi(v, def, access, _) and result = nrOfBoundsExpr(access))
+    or
+    def.isPhiNode(v) and
+    not isNEPhi(v, def, _, _) and
+    result = 0
+  }
+
+  /**
+   * Gets the number of bounds for `def` when `def` is an unsupported guard phi
+   * node for the variable `v`.
+   */
+  private float nrOfBoundsUnsupportedGuardPhi(RangeSsaDefinition def, StackVariable v) {
+    exists(VariableAccess access |
+      isUnsupportedGuardPhi(v, def, access) and
+      result = nrOfBoundsExpr(access)
+    )
+    or
+    def.isPhiNode(v) and
+    not isUnsupportedGuardPhi(v, def, _) and
+    result = 0
+  }
+
+  private float nrOfBoundsPhi(RangeSsaDefinition def, StackVariable v) {
+    // The cases for phi nodes are not mutually exclusive. For instance a phi
+    // node can be both a guard phi node and a normal phi node. To handle this
+    // we sum the contributions from the different cases.
+    result =
+      nrOfBoundsPhiGuard(def, v) + nrOfBoundsPhiNormal(def, v) + nrOfBoundsNEPhi(def, v) +
+        nrOfBoundsUnsupportedGuardPhi(def, v)
+  }
+
+  /** Gets the estimated number of bounds for `def` and `v`. */
+  float nrOfBoundsDef(RangeSsaDefinition def, StackVariable v) {
+    // Recursive definitions are already widened, so we simply estimate them as
+    // having the number of widening bounds available. This is crucial as it
+    // ensures that we don't follow recursive cycles when calculating the
+    // estimate. Had that not been the case the estimate itself would be at risk
+    // of causing performance issues and being non-functional.
+    if isRecursiveDef(def, v)
+    then result = getNrOfWideningBounds(getVariableRangeType(v))
+    else (
+      // Definitions with a defining value
+      exists(Expr defExpr | assignmentDef(def, v, defExpr) and result = nrOfBoundsExpr(defExpr))
+      or
+      // Assignment operations with a defining value
+      exists(AssignOperation assignOp |
+        def = assignOp and
+        assignOp.getLValue() = v.getAnAccess() and
+        result = nrOfBoundsExpr(assignOp)
+      )
+      or
+      // Phi nodes
+      result = nrOfBoundsPhi(def, v)
+      or
+      unanalyzableDefBounds(def, v, _, _) and result = 1
+    )
+  }
+
+  /**
+   * Gets a naive estimate of the number of bounds for `e`.
+   *
+   * The estimate is like an abstract interpretation of the range analysis,
+   * where the abstract value is the number of bounds. For instance,
+   * `nrOfBoundsExpr(12) = 1` and `nrOfBoundsExpr(x + y) = nrOfBoundsExpr(x) *
+   * nrOfBoundsExpr(y)`.
+   *
+   * The estimated number of bounds will usually be greater than the actual
+   * number of bounds, as the estimate can not detect cases where bounds are cut
+   * down when tracked precisely. For instance, in
+   * ```c
+   * int x = 1;
+   * if (cond) { x = 1; }
+   * int y = x + x;
+   * ```
+   * the actual number of bounds for `y` is 1. However, the estimate will be 4
+   * as the conditional assignment to `x` gives two bounds for `x` on the last
+   * line and the addition gives 2 * 2 bounds. There are two sources of anncuracies:
+   *
+   * 1. Without tracking the lower bounds we can't see that `x` is assigned a
+   * value that is equal to its lower bound.
+   * 2. Had the conditional assignment been `x = 2` then the estimate of two
+   * bounds for `x` would have been correct. However, the estimate of 4 for `y`
+   * would still be incorrect. Summing the actual bounds `{1,2}` with itself
+   * gives `{2,3,4}` which is only three bounds. Again, we can't realise this
+   * without tracking the bounds.
+   *
+   * Since these inaccuracies compound the estimated number of bounds can often
+   * be _much_ greater than the actual number of bounds. Do note though that the
+   * estimate is not _guaranteed_ to be an upper bound. In some cases the
+   * approximations might underestimate the number of bounds.
+   *
+   * This predicate is functional. This is crucial as:
+   *
+   * - It ensures that the computing the estimate itself is fast.
+   * - Our use of monotonic aggregates assumes functionality.
+   *
+   * Any non-functional case should be considered a bug.
+   */
+  float nrOfBoundsExpr(Expr e) {
+    // Similarly to what we do for definitions, we do not attempt to measure the
+    // number of bounds for recursive expressions.
+    if isRecursiveExpr(e)
+    then result = getNrOfWideningBounds(e.getUnspecifiedType())
+    else
+      if analyzableExpr(e)
+      then
+        // The cases here are an abstraction of and mirrors the cases inside
+        // `getLowerBoundsImpl`/`getUpperBoundsImpl`.
+        result = 1 and exists(getValue(e).toFloat())
+        or
+        exists(Expr operand | result = nrOfBoundsExpr(operand) |
+          effectivelyMultipliesByPositive(e, operand, _)
+          or
+          effectivelyMultipliesByNegative(e, operand, _)
+        )
+        or
+        exists(ConditionalExpr condExpr |
+          e = condExpr and
+          result = nrOfBoundsExpr(condExpr.getThen()) * nrOfBoundsExpr(condExpr.getElse())
+        )
+        or
+        exists(BinaryOperation binop |
+          e = binop and
+          result = nrOfBoundsExpr(binop.getLeftOperand()) * nrOfBoundsExpr(binop.getRightOperand())
+        |
+          e instanceof MaxExpr or
+          e instanceof MinExpr or
+          e instanceof AddExpr or
+          e instanceof SubExpr or
+          e instanceof UnsignedMulExpr or
+          e instanceof UnsignedBitwiseAndExpr
+        )
+        or
+        exists(AssignExpr assign | e = assign and result = nrOfBoundsExpr(assign.getRValue()))
+        or
+        exists(AssignArithmeticOperation assignOp |
+          e = assignOp and
+          result = nrOfBoundsExpr(assignOp.getLValue()) * nrOfBoundsExpr(assignOp.getRValue())
+        |
+          e instanceof AssignAddExpr or
+          e instanceof AssignSubExpr or
+          e instanceof UnsignedAssignMulExpr
+        )
+        or
+        // Handles `AssignMulByPositiveConstantExpr` and `AssignMulByNegativeConstantExpr`
+        exists(AssignMulByConstantExpr mulExpr |
+          e = mulExpr and
+          result = nrOfBoundsExpr(mulExpr.getLValue())
+        )
+        or
+        // Handles the prefix and postfix increment and decrement operators.
+        exists(CrementOperation crementOp |
+          e = crementOp and result = nrOfBoundsExpr(crementOp.getOperand())
+        )
+        or
+        exists(RemExpr remExpr | e = remExpr | result = nrOfBoundsExpr(remExpr.getRightOperand()))
+        or
+        exists(Conversion convExpr |
+          e = convExpr and
+          if convExpr.getUnspecifiedType() instanceof BoolType
+          then result = 1
+          else result = nrOfBoundsExpr(convExpr.getExpr())
+        )
+        or
+        exists(RangeSsaDefinition def, StackVariable v |
+          e = def.getAUse(v) and
+          result = nrOfBoundsDef(def, v) and
+          // Avoid returning two numbers when `e` is a use with a constant value.
+          not exists(getValue(e).toFloat())
+        )
+        or
+        exists(RShiftExpr rsExpr |
+          e = rsExpr and
+          exists(getValue(rsExpr.getRightOperand().getFullyConverted()).toInt()) and
+          result = nrOfBoundsExpr(rsExpr.getLeftOperand())
+        )
+      else (
+        exists(exprMinVal(e)) and result = 1
+      )
+  }
+}
+
+/**
+ * Holds if `v` is a variable for which widening should be used, as otherwise a
+ * very large number of bounds might be generated during the range analysis for
+ * `v`.
+ */
+private predicate varHasTooManyBounds(StackVariable v) {
+  exists(RangeSsaDefinition def |
+    def.getAVariable() = v and
+    BoundsEstimate::nrOfBoundsDef(def, v) > BoundsEstimate::getBoundsLimit()
+  )
+}
+
+/**
+ * Holds if `e` is an expression for which widening should be used, as otherwise
+ * a very large number of bounds might be generated during the range analysis
+ * for `e`.
+ */
+private predicate exprHasTooManyBounds(Expr e) {
+  BoundsEstimate::nrOfBoundsExpr(e) > BoundsEstimate::getBoundsLimit()
+  or
+  // A subexpressions of an expression with too many bounds may itself not have
+  // to many bounds. For instance, `x + y` can have too many bounds without `x`
+  // having as well. But in these cases, still want to consider `e` as having
+  // too many bounds since:
+  // - The overall result is widened anyway, so widening `e` as well is unlikely
+  //   to cause further precision loss.
+  // - The number of bounds could be very large but still below the arbitrary
+  //   limit. Hence widening `e` can improve performance.
+  exists(Expr pe | exprHasTooManyBounds(pe) and e.getParent() = pe)
+}
+
 /**
  * Holds if `binop` is a binary operation that's likely to be assigned a
  * quadratic (or more) number of candidate bounds during the analysis. This can
@@ -655,13 +988,8 @@ private float getTruncatedLowerBounds(Expr expr) {
     if exprMinVal(expr) <= newLB and newLB <= exprMaxVal(expr)
     then
       // Apply widening where we might get a combinatorial explosion.
-      if isRecursiveBinary(expr)
-      then
-        result =
-          max(float widenLB |
-            widenLB = wideningLowerBounds(expr.getUnspecifiedType()) and
-            not widenLB > newLB
-          )
+      if isRecursiveBinary(expr) or exprHasTooManyBounds(expr)
+      then result = widenLowerBound(expr.getUnspecifiedType(), newLB)
       else result = newLB
     else result = exprMinVal(expr)
   ) and
@@ -714,13 +1042,8 @@ private float getTruncatedUpperBounds(Expr expr) {
         if exprMinVal(expr) <= newUB and newUB <= exprMaxVal(expr)
         then
           // Apply widening where we might get a combinatorial explosion.
-          if isRecursiveBinary(expr)
-          then
-            result =
-              min(float widenUB |
-                widenUB = wideningUpperBounds(expr.getUnspecifiedType()) and
-                not widenUB < newUB
-              )
+          if isRecursiveBinary(expr) or exprHasTooManyBounds(expr)
+          then result = widenUpperBound(expr.getUnspecifiedType(), newUB)
           else result = newUB
         else result = exprMaxVal(expr)
       )
@@ -890,7 +1213,7 @@ private float getLowerBoundsImpl(Expr expr) {
       // equal to `min(-y + 1,y - 1)`.
       exists(float childLB |
         childLB = getFullyConvertedLowerBounds(remExpr.getAnOperand()) and
-        not childLB >= 0
+        childLB < 0
       |
         result = getFullyConvertedLowerBounds(remExpr.getRightOperand()) - 1
         or
@@ -1102,8 +1425,7 @@ private float getUpperBoundsImpl(Expr expr) {
       // adding `-rhsLB` to the set of upper bounds.
       exists(float rhsLB |
         rhsLB = getFullyConvertedLowerBounds(remExpr.getRightOperand()) and
-        not rhsLB >= 0
-      |
+        rhsLB < 0 and
         result = -rhsLB + 1
       )
     )
@@ -1248,8 +1570,7 @@ private float getPhiLowerBounds(StackVariable v, RangeSsaDefinition phi) {
   exists(VariableAccess access, Expr guard, boolean branch, float defLB, float guardLB |
     phi.isGuardPhi(v, access, guard, branch) and
     lowerBoundFromGuard(guard, access, guardLB, branch) and
-    defLB = getFullyConvertedLowerBounds(access)
-  |
+    defLB = getFullyConvertedLowerBounds(access) and
     // Compute the maximum of `guardLB` and `defLB`.
     if guardLB > defLB then result = guardLB else result = defLB
   )
@@ -1273,8 +1594,7 @@ private float getPhiUpperBounds(StackVariable v, RangeSsaDefinition phi) {
   exists(VariableAccess access, Expr guard, boolean branch, float defUB, float guardUB |
     phi.isGuardPhi(v, access, guard, branch) and
     upperBoundFromGuard(guard, access, guardUB, branch) and
-    defUB = getFullyConvertedUpperBounds(access)
-  |
+    defUB = getFullyConvertedUpperBounds(access) and
     // Compute the minimum of `guardUB` and `defUB`.
     if guardUB < defUB then result = guardUB else result = defUB
   )
@@ -1438,8 +1758,7 @@ private predicate upperBoundFromGuard(Expr guard, VariableAccess v, float ub, bo
 }
 
 /**
- * This predicate simplifies the results returned by
- * `linearBoundFromGuard`.
+ * This predicate simplifies the results returned by `linearBoundFromGuard`.
  */
 private predicate boundFromGuard(
   Expr guard, VariableAccess v, float boundValue, boolean isLowerBound,
@@ -1447,22 +1766,10 @@ private predicate boundFromGuard(
 ) {
   exists(float p, float q, float r, boolean isLB |
     linearBoundFromGuard(guard, v, p, q, r, isLB, strictness, branch) and
-    boundValue = (r - q) / p
-  |
+    boundValue = (r - q) / p and
     // If the multiplier is negative then the direction of the comparison
     // needs to be flipped.
-    p > 0 and isLowerBound = isLB
-    or
-    p < 0 and isLowerBound = isLB.booleanNot()
-  )
-  or
-  // When `!e` is true, we know that `0 <= e <= 0`
-  exists(float p, float q, Expr e |
-    linearAccess(e, v, p, q) and
-    eqZeroWithNegate(guard, e, true, branch) and
-    boundValue = (0.0 - q) / p and
-    isLowerBound = [false, true] and
-    strictness = Nonstrict()
+    if p < 0 then isLowerBound = isLB.booleanNot() else isLowerBound = isLB
   )
 }
 
@@ -1472,54 +1779,57 @@ private predicate boundFromGuard(
  * lower or upper bound for `v`.
  */
 private predicate linearBoundFromGuard(
-  ComparisonOperation guard, VariableAccess v, float p, float q, float boundValue,
+  Expr guard, VariableAccess v, float p, float q, float r,
   boolean isLowerBound, // Is this a lower or an upper bound?
   RelationStrictness strictness, boolean branch // Which control-flow branch is this bound valid on?
 ) {
-  // For the comparison x < RHS, we create two bounds:
-  //
-  //   1. x < upperbound(RHS)
-  //   2. x >= typeLowerBound(RHS.getUnspecifiedType())
-  //
-  exists(Expr lhs, Expr rhs, RelationDirection dir, RelationStrictness st |
-    linearAccess(lhs, v, p, q) and
-    relOpWithSwapAndNegate(guard, lhs, rhs, dir, st, branch)
-  |
-    isLowerBound = directionIsGreater(dir) and
-    strictness = st and
-    getBounds(rhs, boundValue, isLowerBound)
+  exists(Expr lhs | linearAccess(lhs, v, p, q) |
+    // For the comparison x < RHS, we create the following bounds:
+    //   1. x < upperbound(RHS)
+    //   2. x >= typeLowerBound(RHS.getUnspecifiedType())
+    exists(Expr rhs, RelationDirection dir, RelationStrictness st |
+      relOpWithSwapAndNegate(guard, lhs, rhs, dir, st, branch)
+    |
+      isLowerBound = directionIsGreater(dir) and
+      strictness = st and
+      r = getBounds(rhs, isLowerBound)
+      or
+      isLowerBound = directionIsLesser(dir) and
+      strictness = Nonstrict() and
+      r = getExprTypeBounds(rhs, isLowerBound)
+    )
     or
-    isLowerBound = directionIsLesser(dir) and
-    strictness = Nonstrict() and
-    exprTypeBounds(rhs, boundValue, isLowerBound)
-  )
-  or
-  // For x == RHS, we create the following bounds:
-  //
-  //   1. x <= upperbound(RHS)
-  //   2. x >= lowerbound(RHS)
-  //
-  exists(Expr lhs, Expr rhs |
-    linearAccess(lhs, v, p, q) and
-    eqOpWithSwapAndNegate(guard, lhs, rhs, true, branch) and
-    getBounds(rhs, boundValue, isLowerBound) and
+    // For x == RHS, we create the following bounds:
+    //   1. x <= upperbound(RHS)
+    //   2. x >= lowerbound(RHS)
+    exists(Expr rhs |
+      eqOpWithSwapAndNegate(guard, lhs, rhs, true, branch) and
+      r = getBounds(rhs, isLowerBound) and
+      strictness = Nonstrict()
+    )
+    or
+    // When `x` is equal to 0 we create the following bounds:
+    //   1. x <= 0
+    //   2. x >= 0
+    eqZeroWithNegate(guard, lhs, true, branch) and
+    r = 0.0 and
+    isLowerBound = [false, true] and
     strictness = Nonstrict()
   )
-  // x != RHS and !x are handled elsewhere
+}
+
+/** Get the fully converted lower or upper bounds of `expr` based on `isLowerBound`. */
+private float getBounds(Expr expr, boolean isLowerBound) {
+  isLowerBound = true and result = getFullyConvertedLowerBounds(expr)
+  or
+  isLowerBound = false and result = getFullyConvertedUpperBounds(expr)
 }
 
 /** Utility for `linearBoundFromGuard`. */
-private predicate getBounds(Expr expr, float boundValue, boolean isLowerBound) {
-  isLowerBound = true and boundValue = getFullyConvertedLowerBounds(expr)
+private float getExprTypeBounds(Expr expr, boolean isLowerBound) {
+  isLowerBound = true and result = exprMinVal(expr.getFullyConverted())
   or
-  isLowerBound = false and boundValue = getFullyConvertedUpperBounds(expr)
-}
-
-/** Utility for `linearBoundFromGuard`. */
-private predicate exprTypeBounds(Expr expr, float boundValue, boolean isLowerBound) {
-  isLowerBound = true and boundValue = exprMinVal(expr.getFullyConverted())
-  or
-  isLowerBound = false and boundValue = exprMaxVal(expr.getFullyConverted())
+  isLowerBound = false and result = exprMaxVal(expr.getFullyConverted())
 }
 
 /**
@@ -1810,18 +2120,12 @@ module SimpleRangeAnalysisInternal {
     |
       // Widening: check whether the new lower bound is from a source which
       // depends recursively on the current definition.
-      if isRecursiveDef(def, v)
+      if isRecursiveDef(def, v) or varHasTooManyBounds(v)
       then
         // The new lower bound is from a recursive source, so we round
         // down to one of a limited set of values to prevent the
         // recursion from exploding.
-        result =
-          max(float widenLB |
-            widenLB = wideningLowerBounds(getVariableRangeType(v)) and
-            not widenLB > truncatedLB
-          |
-            widenLB
-          )
+        result = widenLowerBound(getVariableRangeType(v), truncatedLB)
       else result = truncatedLB
     )
     or
@@ -1840,18 +2144,12 @@ module SimpleRangeAnalysisInternal {
     |
       // Widening: check whether the new upper bound is from a source which
       // depends recursively on the current definition.
-      if isRecursiveDef(def, v)
+      if isRecursiveDef(def, v) or varHasTooManyBounds(v)
       then
         // The new upper bound is from a recursive source, so we round
         // up to one of a fixed set of values to prevent the recursion
         // from exploding.
-        result =
-          min(float widenUB |
-            widenUB = wideningUpperBounds(getVariableRangeType(v)) and
-            not widenUB < truncatedUB
-          |
-            widenUB
-          )
+        result = widenUpperBound(getVariableRangeType(v), truncatedUB)
       else result = truncatedUB
     )
     or
@@ -1859,4 +2157,60 @@ module SimpleRangeAnalysisInternal {
     // bound is `typeUpperBound`.
     defMightOverflowNegatively(def, v) and result = varMaxVal(v)
   }
+
+  /** Gets the estimate of the number of bounds for `e`. */
+  float estimateNrOfBounds(Expr e) { result = BoundsEstimate::nrOfBoundsExpr(e) }
+}
+
+/** Provides predicates for debugging the simple range analysis library. */
+private module Debug {
+  Locatable getRelevantLocatable() {
+    exists(string filepath, int startline |
+      result.getLocation().hasLocationInfo(filepath, startline, _, _, _) and
+      filepath.matches("%/test.c") and
+      startline = [621 .. 639]
+    )
+  }
+
+  float debugGetLowerBoundsImpl(Expr e) {
+    e = getRelevantLocatable() and
+    result = getLowerBoundsImpl(e)
+  }
+
+  float debugGetUpperBoundsImpl(Expr e) {
+    e = getRelevantLocatable() and
+    result = getUpperBoundsImpl(e)
+  }
+
+  /**
+   * Counts the number of lower bounds for a given expression. This predicate is
+   * useful for identifying performance issues in the range analysis.
+   */
+  predicate countGetLowerBoundsImpl(Expr e, int n) {
+    e = getRelevantLocatable() and
+    n = strictcount(float lb | lb = getLowerBoundsImpl(e) | lb)
+  }
+
+  float debugNrOfBounds(Expr e) {
+    e = getRelevantLocatable() and
+    result = BoundsEstimate::nrOfBoundsExpr(e)
+  }
+
+  /**
+   * Finds any expressions for which `nrOfBounds` is not functional. The result
+   * should be empty, so this predicate is useful to debug non-functional cases.
+   */
+  int nonFunctionalNrOfBounds(Expr e) {
+    strictcount(BoundsEstimate::nrOfBoundsExpr(e)) > 1 and
+    result = BoundsEstimate::nrOfBoundsExpr(e)
+  }
+
+  /**
+   * Holds if `e` is an expression that has a lower bound, but where
+   * `nrOfBounds` does not compute an estimate.
+   */
+  predicate missingNrOfBounds(Expr e, float n) {
+    n = lowerBound(e) and
+    not exists(BoundsEstimate::nrOfBoundsExpr(e))
+  }
 }

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExprSpecific.qll

@@ -259,7 +259,7 @@ module SemanticExprConfig {
   }
 
   predicate guardHasBranchEdge(Guard guard, BasicBlock bb1, BasicBlock bb2, boolean branch) {
-    guard.controlsEdge(bb1, bb2, branch)
+    guard.controlsBranchEdge(bb1, bb2, branch)
   }
 
   Guard comparisonGuard(Expr e) { getSemanticExpr(result) = e }

cpp/ql/src/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.5.2
+
+No user-facing changes.
+
 ## 1.5.1
 
 No user-facing changes.

cpp/ql/src/Critical/OverflowDestination.ql

@@ -85,10 +85,8 @@ module OverflowDestinationConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(FunctionCall fc | result = fc.getLocation() |
+    exists(FunctionCall fc | result = [fc.getLocation(), sink.getLocation()] |
       sourceSized(fc, sink.asIndirectConvertedExpr())
     )
   }

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

@@ -171,12 +171,10 @@ module NonConstFlowConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    result = sink.getLocation()
-    or
-    exists(FormattingFunctionCall call, Expr formatString | result = call.getLocation() |
+    exists(FormattingFunctionCall call, Expr formatString |
+      result = [call.getLocation(), sink.getLocation()]
+    |
       isSinkImpl(sink, formatString) and
       call.getArgument(call.getFormatParameterIndex()) = formatString
     )

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

@@ -136,7 +136,7 @@ private module NetworkToBufferSizeConfig implements DataFlow::ConfigSig {
 
   predicate isBarrier(DataFlow::Node node) {
     exists(GuardCondition gc, GVN gvn |
-      gc.getAChild*() = gvn.getAnExpr() and
+      gc.(Expr).getAChild*() = gvn.getAnExpr() and
       globalValueNumber(node.asExpr()) = gvn and
       gc.controls(node.asExpr().getBasicBlock(), _)
     )

cpp/ql/src/Security/CWE/CWE-078/ExecTainted.ql

@@ -155,7 +155,7 @@ module ExecTaintConfig implements DataFlow::StateConfigSig {
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     exists(DataFlow::Node concatResult, Expr command, ExecState state |
-      result = [concatResult.getLocation(), command.getLocation()] and
+      result = [concatResult.getLocation(), command.getLocation(), sink.getLocation()] and
       isSink(sink, state) and
       isSinkImpl(sink, command, _) and
       concatResult = state.getOutgoingNode()

cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql

@@ -58,7 +58,9 @@ module SqlTaintedConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(Expr taintedArg | result = taintedArg.getLocation() | taintedArg = asSinkExpr(sink))
+    exists(Expr taintedArg | result = [taintedArg.getLocation(), sink.getLocation()] |
+      taintedArg = asSinkExpr(sink)
+    )
   }
 }
 

cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.ql

@@ -128,7 +128,7 @@ module Config implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(BufferWrite bw | result = bw.getLocation() | isSink(sink, bw, _))
+    exists(BufferWrite bw | result = [bw.getLocation(), sink.getLocation()] | isSink(sink, bw, _))
   }
 }
 

cpp/ql/src/Security/CWE/CWE-129/ImproperArrayIndexValidation.ql

@@ -26,13 +26,13 @@ predicate isFlowSource(FS::FlowSource source, string sourceType) {
 predicate guardChecks(IRGuardCondition g, Expr e, boolean branch) {
   exists(Operand op | op.getDef().getConvertedResultExpression() = e |
     // `op < k` is true and `k > 0`
-    g.comparesLt(op, any(int k | k > 0), true, any(BooleanValue bv | bv.getValue() = branch))
+    g.comparesLt(op, any(int k | k > 0), true, any(GuardValue bv | bv.asBooleanValue() = branch))
     or
     // `op < _ + k` is true and `k > 0`.
     g.comparesLt(op, _, any(int k | k > 0), true, branch)
     or
     // op == k
-    g.comparesEq(op, _, true, any(BooleanValue bv | bv.getValue() = branch))
+    g.comparesEq(op, _, true, any(GuardValue bv | bv.asBooleanValue() = branch))
     or
     // op == _ + k
     g.comparesEq(op, _, _, true, branch)

cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql

@@ -124,7 +124,8 @@ module UncontrolledArithConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSourceLocation(DataFlow::Node source) {
-    result = getExpr(source).getLocation()
+    isSource(source) and
+    result = [getExpr(source).getLocation(), source.getLocation()]
   }
 }
 

cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql

@@ -95,7 +95,7 @@ module TaintedAllocationSizeConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(Expr alloc | result = alloc.getLocation() | allocSink(alloc, sink))
+    exists(Expr alloc | result = [alloc.getLocation(), sink.getLocation()] | allocSink(alloc, sink))
   }
 }
 

cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql

@@ -76,7 +76,9 @@ module Config implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(Expr condition | result = condition.getLocation() | isSink(sink, condition))
+    exists(Expr condition | result = [condition.getLocation(), sink.getLocation()] |
+      isSink(sink, condition)
+    )
   }
 }
 

cpp/ql/src/Security/CWE/CWE-295/SSLResultConflation.ql

@@ -29,7 +29,7 @@ module VerifyResultConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SslGetVerifyResultCall }
 
   predicate isSink(DataFlow::Node sink) {
-    exists(GuardCondition guard | guard.getAChild*() = sink.asExpr())
+    exists(GuardCondition guard | guard.(Expr).getAChild*() = sink.asExpr())
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }

cpp/ql/src/Security/CWE/CWE-311/CleartextBufferWrite.ql

@@ -51,7 +51,9 @@ module ToBufferConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(SensitiveBufferWrite w | result = w.getLocation() | isSinkImpl(sink, w))
+    exists(SensitiveBufferWrite w | result = [w.getLocation(), sink.getLocation()] |
+      isSinkImpl(sink, w)
+    )
   }
 }
 

cpp/ql/src/Security/CWE/CWE-311/CleartextFileWrite.ql

@@ -35,11 +35,13 @@ module FromSensitiveConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSourceLocation(DataFlow::Node sourceNode) {
-    exists(SensitiveExpr source | result = source.getLocation() | isSourceImpl(sourceNode, source))
+    exists(SensitiveExpr source | result = [source.getLocation(), sourceNode.getLocation()] |
+      isSourceImpl(sourceNode, source)
+    )
   }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(FileWrite w | result = w.getLocation() | isSinkImpl(sink, w, _))
+    exists(FileWrite w | result = [w.getLocation(), sink.getLocation()] | isSinkImpl(sink, w, _))
   }
 }
 

cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql

@@ -249,7 +249,9 @@ module FromSensitiveConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(NetworkSendRecv networkSendRecv | result = networkSendRecv.getLocation() |
+    exists(NetworkSendRecv networkSendRecv |
+      result = [networkSendRecv.getLocation(), sink.getLocation()]
+    |
       isSinkSendRecv(sink, networkSendRecv)
     )
   }

cpp/ql/src/Security/CWE/CWE-313/CleartextSqliteDatabase.ql

@@ -127,13 +127,13 @@ module FromSensitiveConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSourceLocation(DataFlow::Node source) {
-    exists(SensitiveExpr sensitive | result = sensitive.getLocation() |
+    exists(SensitiveExpr sensitive | result = [sensitive.getLocation(), source.getLocation()] |
       isSourceImpl(source, sensitive)
     )
   }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(SqliteFunctionCall sqliteCall | result = sqliteCall.getLocation() |
+    exists(SqliteFunctionCall sqliteCall | result = [sqliteCall.getLocation(), sink.getLocation()] |
       isSinkImpl(sink, sqliteCall, _)
     )
   }

cpp/ql/src/Security/CWE/CWE-319/UseOfHttp.ql

@@ -91,10 +91,9 @@ module HttpStringToUrlOpenConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSourceLocation(DataFlow::Node source) {
-    result = source.asIndirectExpr().getLocation()
+    isSource(source) and
+    result = [source.asIndirectExpr().getLocation(), source.getLocation()]
   }
-
-  Location getASelectedSinkLocation(DataFlow::Node sink) { none() }
 }
 
 module HttpStringToUrlOpen = TaintTracking::Global<HttpStringToUrlOpenConfig>;

cpp/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.qhelp

@@ -3,11 +3,15 @@
   "qhelp.dtd">
 <qhelp>
 <overview>
-<p>Using broken or weak cryptographic algorithms can leave data vulnerable to being decrypted.</p>
+<p>Using broken or weak cryptographic algorithms may compromise security guarantees such as confidentiality, integrity, and authenticity.</p>
 
-<p>Many cryptographic algorithms provided by cryptography libraries are known to be weak, or 
-flawed. Using such an algorithm means that an attacker may be able to easily decrypt the encrypted
-data.</p>
+<p>Many cryptographic algorithms are known to be weak or flawed. The security guarantees of a system often rely on the underlying cryptography, so using a weak algorithm can have severe consequences. For example:
+</p>
+<ul>
+  <li>If a weak encryption algorithm is used, an attacker may be able to decrypt sensitive data.</li>
+  <li>If a weak hashing algorithm is used to protect data integrity, an attacker may be able to craft a malicious input that has the same hash as a benign one.</li>
+  <li>If a weak algorithm is used for digital signatures, an attacker may be able to forge signatures and impersonate legitimate users.</li>
+</ul>
 
 </overview>
 <recommendation>

cpp/ql/src/Security/CWE/CWE-367/TOCTOUFilesystemRace.ql

@@ -115,7 +115,7 @@ predicate checksPath(Expr check, Expr checkPath) {
 
 pragma[nomagic]
 predicate checkPathControlsUse(Expr check, Expr checkPath, Expr use) {
-  exists(GuardCondition guard | referenceTo(check, guard.getAChild*()) |
+  exists(GuardCondition guard | referenceTo(check, guard.(Expr).getAChild*()) |
     guard.controls(use.getBasicBlock(), _)
   ) and
   checksPath(pragma[only_bind_into](check), checkPath)
@@ -123,7 +123,7 @@ predicate checkPathControlsUse(Expr check, Expr checkPath, Expr use) {
 
 pragma[nomagic]
 predicate fileNameOperationControlsUse(Expr check, Expr checkPath, Expr use) {
-  exists(GuardCondition guard | referenceTo(check, guard.getAChild*()) |
+  exists(GuardCondition guard | referenceTo(check, guard.(Expr).getAChild*()) |
     guard.controls(use.getBasicBlock(), _)
   ) and
   pragma[only_bind_into](check) = filenameOperation(checkPath)

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

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

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 1.5.1
+lastReleaseVersion: 1.5.2

cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.ql

@@ -50,8 +50,6 @@ module WordexpTaintConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
 }
 
 module WordexpTaint = TaintTracking::Global<WordexpTaintConfig>;

cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql

@@ -187,12 +187,14 @@ module ArrayAddressToDerefConfig implements DataFlow::StateConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSourceLocation(DataFlow::Node source) {
-    exists(Variable v | result = v.getLocation() | isSourceImpl(source, v))
+    exists(Variable v | result = v.getLocation() or result = source.getLocation() |
+      isSourceImpl(source, v)
+    )
   }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     exists(PointerArithmeticInstruction pai, Instruction deref |
-      result = [pai, deref].getLocation() and
+      result = [[pai, deref].getLocation(), sink.getLocation()] and
       isInvalidPointerDerefSink2(sink, deref, _) and
       isSink(sink, ArrayAddressToDerefConfig::TOverflowArithmetic(pai))
     )