Merge pull request #272 from microsoft/jb1/2.22.3

Merge upstream `codeql-cli-2.22.3`

.gitattributes

@@ -88,3 +88,8 @@
 # swift prebuilt resources
 /swift/third_party/resources/*.zip filter=lfs diff=lfs merge=lfs -text
 /swift/third_party/resources/*.tar.zst filter=lfs diff=lfs merge=lfs -text
+
+# This upgrade script must use windows line-endings to be compatible with old
+# databases.
+/powershell/ql/lib/upgrades/ce269c61feda10a8ca0d16519085f7e55741a694/old.dbscheme eol=crlf
+/powershell/downgrades/802d5b9f407fb0dac894df1c0b4584f2215e1512/semmlecode.powershell.dbscheme eol=crlf

.github/workflows/microsoft-codeql-pack-publish.yml

@@ -0,0 +1,152 @@
+name: Microsoft CodeQL Pack Publish
+
+on:
+  workflow_dispatch:
+
+jobs:
+  check-branch:
+    runs-on: ubuntu-latest
+    steps:
+    - name: Fail if not on main branch
+      run: |
+        if [ "$GITHUB_REF" != "refs/heads/main" ]; then
+          echo "This workflow can only run on the 'main' branch."
+          exit 1
+        fi
+  codeqlversion:
+    needs: check-branch
+    runs-on: ubuntu-latest
+    outputs:
+      codeql_version: ${{ steps.set_codeql_version.outputs.codeql_version }}
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0 
+      - name: Set CodeQL Version
+        id: set_codeql_version
+        run: |
+            git fetch
+            git fetch --tags
+            CURRENT_COMMIT=$(git rev-list -1 HEAD)
+            CURRENT_TAG=$(git describe --tags --abbrev=0 --match 'codeql-cli/v*' $CURRENT_COMMIT)
+            CODEQL_VERSION="${CURRENT_TAG#codeql-cli/}"
+            echo "CODEQL_VERSION=$CODEQL_VERSION" >> $GITHUB_OUTPUT
+  publishlibs:
+    environment: secure-publish
+    needs: codeqlversion
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        language: ['powershell']
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v4
+    - name: Install CodeQL
+      shell: bash
+      run: |
+        gh extension install github/gh-codeql
+        gh codeql download "${{ needs.codeqlversion.outputs.codeql_version }}"
+        gh codeql set-version "${{ needs.codeqlversion.outputs.codeql_version }}"
+      env:
+        GITHUB_TOKEN: ${{ github.token }}
+    - name: Publish OS Microsoft CodeQL Lib Pack
+      shell: bash
+      run: |
+        # Download latest qlpack
+        gh codeql pack download "microsoft/$LANGUAGE-all"
+        PACK_DIR="$HOME/.codeql/packages/microsoft/$LANGUAGE-all"
+        VERSION_COUNT=$(ls -d "$PACK_DIR"/*/ | wc -l)
+        [[ "$VERSION_COUNT" -ne 1 ]] && { echo "Expected exactly one version in $PACK_DIR, but found $VERSION_COUNT. Exiting."; exit 1; }
+
+        # Increment version
+        CURRENT_VERSION=$(ls -v "$PACK_DIR" | tail -n 1)
+        MAJOR=$(echo "$CURRENT_VERSION" | cut -d. -f1)
+        MINOR=$(echo "$CURRENT_VERSION" | cut -d. -f2)
+        PATCH=$(echo "$CURRENT_VERSION" | cut -d. -f3)
+        NEXT_VERSION="$MAJOR.$MINOR.$((PATCH + 1))"
+
+        # Extract dependencies from the existing qlpack.yml before deleting
+        DEPENDENCIES=$(yq 'select(has("dependencies")) | .dependencies | {"dependencies": .}' "$LANGUAGE/ql/lib/qlpack.yml" 2>/dev/null)
+        DATAEXTENSIONS=$(yq 'select(has("dataExtensions")) | .dataExtensions | {"dataExtensions": .}' "$LANGUAGE/ql/lib/qlpack.yml" 2>/dev/null)
+        rm -f "$LANGUAGE/ql/lib/qlpack.yml" "$LANGUAGE/ql/lib/qlpack.lock"
+
+        # Create new qlpack.yml with modified content
+        cat <<EOF > "$LANGUAGE/ql/lib/qlpack.yml"
+        name: microsoft/$LANGUAGE-all
+        version: $NEXT_VERSION
+        extractor: $LANGUAGE
+        groups:
+          - $LANGUAGE
+          - microsoft-all
+        dbscheme: semmlecode.$LANGUAGE.dbscheme
+        extractor: $LANGUAGE
+        library: true
+        upgrades: upgrades
+        $DEPENDENCIES
+        $DATAEXTENSIONS
+        warnOnImplicitThis: true
+        EOF
+
+        # Publish pack
+        cat "$LANGUAGE/ql/lib/qlpack.yml"
+        gh codeql pack publish "$LANGUAGE/ql/lib"
+      env:
+        LANGUAGE: ${{ matrix.language }}
+        GITHUB_TOKEN: ${{ secrets.PACKAGE_PUBLISH }}
+  publish:
+    environment: secure-publish
+    needs: codeqlversion
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        language: ['csharp', 'cpp', 'java', 'javascript', 'python', 'ruby', 'go', 'rust', 'swift', 'powershell', 'iac']
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v4
+    - name: Install CodeQL
+      shell: bash
+      run: |
+        gh extension install github/gh-codeql
+        gh codeql download "${{ needs.codeqlversion.outputs.codeql_version }}"
+        gh codeql set-version "${{ needs.codeqlversion.outputs.codeql_version }}"
+      env:
+        GITHUB_TOKEN: ${{ github.token }}
+    - name: Publish OS Microsoft CodeQL Pack
+      shell: bash
+      run: |
+        # Download latest qlpack
+        gh codeql pack download "microsoft/$LANGUAGE-queries"
+        PACK_DIR="$HOME/.codeql/packages/microsoft/$LANGUAGE-queries"
+        VERSION_COUNT=$(ls -d "$PACK_DIR"/*/ | wc -l)
+        [[ "$VERSION_COUNT" -ne 1 ]] && { echo "Expected exactly one version in $PACK_DIR, but found $VERSION_COUNT. Exiting."; exit 1; }
+
+        # Increment version
+        CURRENT_VERSION=$(ls -v "$PACK_DIR" | tail -n 1)
+        MAJOR=$(echo "$CURRENT_VERSION" | cut -d. -f1)
+        MINOR=$(echo "$CURRENT_VERSION" | cut -d. -f2)
+        PATCH=$(echo "$CURRENT_VERSION" | cut -d. -f3)
+        NEXT_VERSION="$MAJOR.$MINOR.$((PATCH + 1))"
+
+        # Extract dependencies from the existing qlpack.yml before deleting
+        DEPENDENCIES=$(yq 'select(has("dependencies")) | .dependencies | {"dependencies": .}' "$LANGUAGE/ql/src/qlpack.yml" 2>/dev/null)
+        rm -f "$LANGUAGE/ql/src/qlpack.yml" "$LANGUAGE/ql/src/qlpack.lock"
+
+        # Create new qlpack.yml with modified content
+        cat <<EOF > "$LANGUAGE/ql/src/qlpack.yml"
+        name: microsoft/$LANGUAGE-queries
+        version: $NEXT_VERSION
+        extractor: $LANGUAGE
+        groups:
+          - $LANGUAGE
+          - queries
+        $DEPENDENCIES
+        EOF
+
+        # Publish pack
+        cat "$LANGUAGE/ql/src/qlpack.yml"
+        gh codeql pack publish "$LANGUAGE/ql/src"
+      env:
+        LANGUAGE: ${{ matrix.language }}
+        GITHUB_TOKEN: ${{ secrets.PACKAGE_PUBLISH }}
+

.github/workflows/powershell-pr-check.yml

@@ -0,0 +1,32 @@
+name: PowerShell PR Check
+
+on:
+  pull_request:
+    branches:
+      - main
+  workflow_dispatch:
+
+jobs:
+  powershell-pr-check:
+    name: powershell-pr-check
+    runs-on: windows-latest
+    if: github.repository == 'microsoft/codeql'
+    permissions:
+      contents: read
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+          token: ${{ github.token }}
+      - name: Setup CodeQL
+        uses: ./.github/actions/fetch-codeql
+        with:
+          channel: release
+      - name: Install PowerShell
+        run: |
+          $path = Split-Path (Get-Command codeql).Source
+          ./powershell/build-win64.ps1 $path
+      - name: Run QL tests
+        run: |
+          codeql test run --threads=0 powershell/ql/test

.github/workflows/sync-main-tags.yml

@@ -0,0 +1,28 @@
+name: Sync Main Tags
+
+on:
+  pull_request:
+    types:
+      - closed
+    branches:
+      - main
+
+jobs:
+  sync-main-tags:
+    name: Sync Main Tags
+    runs-on: ubuntu-latest
+    if: github.repository == 'microsoft/codeql' && github.event.pull_request.merged == true && github.event.pull_request.head.ref == 'auto/sync-main-pr'
+    permissions:
+      contents: write
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+      - name: Push Tags
+        run: |
+          git remote add upstream https://github.com/github/codeql.git
+          git fetch upstream --tags --force
+          git push --force origin --tags
+        env:
+          GH_TOKEN: ${{ secrets.WORKFLOW_TOKEN }}

.github/workflows/sync-main.yml

@@ -0,0 +1,91 @@
+name: Sync Main
+
+on:
+  push:
+    branches:
+      - main
+    paths:
+      - .github/workflows/sync-main.yml
+  schedule:
+    - cron: '55 * * * *'
+  
+jobs:
+  sync-main:
+    name: Sync-main
+    runs-on: ubuntu-latest
+    if: github.repository == 'microsoft/codeql'
+    permissions:
+      contents: write
+      pull-requests: write
+      
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+          token: ${{ secrets.WORKFLOW_TOKEN }}
+      - name: Git config
+        shell: bash
+        run: |
+          git config user.name "dilanbhalla"
+          git config user.email "dilanbhalla@microsoft.com"
+      - name: Git checkout auto/sync-main-pr
+        shell: bash
+        run: |
+          git fetch origin
+          if git ls-remote --exit-code --heads origin auto/sync-main-pr > /dev/null; then
+            echo "Branch exists remotely. Checking it out."
+            git checkout -B auto/sync-main-pr origin/auto/sync-main-pr
+          else
+            echo "Branch does not exist remotely. Creating from main."
+            git checkout -B auto/sync-main-pr origin/main
+            git push -u origin auto/sync-main-pr
+          fi
+      - name: Sync origin/main
+        shell: bash
+        run: |
+          echo "::group::Sync with main branch"
+          git pull origin auto/sync-main-pr; exitCode=$?; if [ $exitCode -ne 0 ]; then exitCode=0; fi
+          git pull origin main --no-rebase
+          git push --force origin auto/sync-main-pr
+          echo "::endgroup::"
+      - name: Sync upstream/codeql-cli/latest
+        shell: bash
+        run: |
+          echo "::group::Set up remote"
+          git remote add upstream https://github.com/github/codeql.git
+          git fetch upstream --tags --force
+          echo "::endgroup::"
+          echo "::group::Merge codeql-cli/latest"
+          set -x
+          git merge codeql-cli/latest
+          set +x
+          echo "::endgroup::"
+      - name: Push sync branch
+        run: |
+          git push origin auto/sync-main-pr
+        env:
+          GITHUB_TOKEN: ${{ secrets.WORKFLOW_TOKEN }}
+          GH_TOKEN: ${{ secrets.WORKFLOW_TOKEN }}
+      - name: Create PR if it doesn't exist
+        shell: bash
+        run: |
+          pr_number=$(gh pr list --repo microsoft/codeql --head auto/sync-main-pr --base main --json number --jq '.[0].number')
+          if [ -n "$pr_number" ]; then
+            echo "PR from auto/sync-main-pr to main already exists (PR #$pr_number). Exiting gracefully."
+          else
+            if git fetch origin main auto/sync-main-pr && [ -n "$(git rev-list origin/main..origin/auto/sync-main-pr)" ]; then
+              echo "PR does not exist. Creating one..."
+              gh pr create --repo microsoft/codeql --fill -B main -H auto/sync-main-pr \
+                --label 'autogenerated' \
+                --title 'Sync Main (autogenerated)' \
+                --body "This PR syncs the latest changes from \`codeql-cli/latest\` into \`main\`." \
+                --reviewer 'MathiasVP' \
+                --reviewer 'ropwareJB'
+            else
+              echo "No changes to sync from auto/sync-main-pr to main. Exiting gracefully."
+            fi
+          fi
+        env:
+          GH_TOKEN: ${{ secrets.WORKFLOW_TOKEN }}
+

.gitmodules

@@ -0,0 +1,3 @@
+[submodule "iac"]
+	path = iac
+	url = https://github.com/advanced-security/codeql-extractor-iac

MODULE.bazel

@@ -230,7 +230,7 @@ use_repo(
     "kotlin-compiler-2.1.0-Beta1",
     "kotlin-compiler-2.1.20-Beta1",
     "kotlin-compiler-2.2.0-Beta1",
-    "kotlin-compiler-2.2.20-Beta1",
+    "kotlin-compiler-2.2.20-Beta2",
     "kotlin-compiler-embeddable-1.6.0",
     "kotlin-compiler-embeddable-1.6.20",
     "kotlin-compiler-embeddable-1.7.0",
@@ -243,7 +243,7 @@ use_repo(
     "kotlin-compiler-embeddable-2.1.0-Beta1",
     "kotlin-compiler-embeddable-2.1.20-Beta1",
     "kotlin-compiler-embeddable-2.2.0-Beta1",
-    "kotlin-compiler-embeddable-2.2.20-Beta1",
+    "kotlin-compiler-embeddable-2.2.20-Beta2",
     "kotlin-stdlib-1.6.0",
     "kotlin-stdlib-1.6.20",
     "kotlin-stdlib-1.7.0",
@@ -256,7 +256,7 @@ use_repo(
     "kotlin-stdlib-2.1.0-Beta1",
     "kotlin-stdlib-2.1.20-Beta1",
     "kotlin-stdlib-2.2.0-Beta1",
-    "kotlin-stdlib-2.2.20-Beta1",
+    "kotlin-stdlib-2.2.20-Beta2",
 )
 
 go_sdk = use_extension("@rules_go//go:extensions.bzl", "go_sdk")

README.md

@@ -29,3 +29,5 @@ You can install the [CodeQL for Visual Studio Code](https://marketplace.visualst
 ### Tasks
 
 The `.vscode/tasks.json` file defines custom tasks specific to working in this repository. To invoke one of these tasks, select the `Terminal | Run Task...` menu option, and then select the desired task from the dropdown. You can also invoke the `Tasks: Run Task` command from the command palette.
+
+

SECURITY.md

@@ -0,0 +1,41 @@
+<!-- BEGIN MICROSOFT SECURITY.MD V0.0.8 BLOCK -->
+
+## Security
+
+Microsoft takes the security of our software products and services seriously, which includes all source code repositories managed through our GitHub organizations, which include [Microsoft](https://github.com/microsoft), [Azure](https://github.com/Azure), [DotNet](https://github.com/dotnet), [AspNet](https://github.com/aspnet), [Xamarin](https://github.com/xamarin), and [our GitHub organizations](https://opensource.microsoft.com/).
+
+If you believe you have found a security vulnerability in any Microsoft-owned repository that meets [Microsoft's definition of a security vulnerability](https://aka.ms/opensource/security/definition), please report it to us as described below.
+
+## Reporting Security Issues
+
+**Please do not report security vulnerabilities through public GitHub issues.**
+
+Instead, please report them to the Microsoft Security Response Center (MSRC) at [https://msrc.microsoft.com/create-report](https://aka.ms/opensource/security/create-report).
+
+If you prefer to submit without logging in, send email to [secure@microsoft.com](mailto:secure@microsoft.com).  If possible, encrypt your message with our PGP key; please download it from the [Microsoft Security Response Center PGP Key page](https://aka.ms/opensource/security/pgpkey).
+
+You should receive a response within 24 hours. If for some reason you do not, please follow up via email to ensure we received your original message. Additional information can be found at [microsoft.com/msrc](https://aka.ms/opensource/security/msrc). 
+
+Please include the requested information listed below (as much as you can provide) to help us better understand the nature and scope of the possible issue:
+
+  * Type of issue (e.g. buffer overflow, SQL injection, cross-site scripting, etc.)
+  * Full paths of source file(s) related to the manifestation of the issue
+  * The location of the affected source code (tag/branch/commit or direct URL)
+  * Any special configuration required to reproduce the issue
+  * Step-by-step instructions to reproduce the issue
+  * Proof-of-concept or exploit code (if possible)
+  * Impact of the issue, including how an attacker might exploit the issue
+
+This information will help us triage your report more quickly.
+
+If you are reporting for a bug bounty, more complete reports can contribute to a higher bounty award. Please visit our [Microsoft Bug Bounty Program](https://aka.ms/opensource/security/bounty) page for more details about our active programs.
+
+## Preferred Languages
+
+We prefer all communications to be in English.
+
+## Policy
+
+Microsoft follows the principle of [Coordinated Vulnerability Disclosure](https://aka.ms/opensource/security/cvd).
+
+<!-- END MICROSOFT SECURITY.MD BLOCK -->

actions/ql/lib/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.4.14
+
+No user-facing changes.
+
 ## 0.4.13
 
 ### Bug Fixes

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

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

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.4.13
+lastReleaseVersion: 0.4.14

actions/ql/lib/qlpack.yml

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

actions/ql/src/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.6.6
+
+No user-facing changes.
+
 ## 0.6.5
 
 No user-facing changes.

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

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

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.6.5
+lastReleaseVersion: 0.6.6

actions/ql/src/qlpack.yml

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

cpp/ql/lib/CHANGELOG.md

@@ -1,3 +1,13 @@
+## 5.4.0
+
+### New Features
+
+* Exposed various SSA-related classes (`Definition`, `PhiNode`, `ExplicitDefinition`, `DirectExplicitDefinition`, and `IndirectExplicitDefinition`) which were previously only usable inside the internal dataflow directory.
+
+### Minor Analysis Improvements
+
+* The `cpp/overrun-write` query now recognizes more bound checks and thus produces fewer false positives.
+
 ## 5.3.0
 
 ### Deprecated APIs

cpp/ql/lib/change-notes/2023-10-12-additional-call-targets.md

@@ -0,0 +1,4 @@
+---
+category: feature
+---
+* Added a new class `AdditionalCallTarget` for specifying additional call targets.

cpp/ql/lib/change-notes/2025-07-23-overrun-write.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* The `cpp/overrun-write` query now recognizes more bound checks and thus produces fewer false positives.

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

@@ -0,0 +1,9 @@
+## 5.4.0
+
+### New Features
+
+* Exposed various SSA-related classes (`Definition`, `PhiNode`, `ExplicitDefinition`, `DirectExplicitDefinition`, and `IndirectExplicitDefinition`) which were previously only usable inside the internal dataflow directory.
+
+### Minor Analysis Improvements
+
+* The `cpp/overrun-write` query now recognizes more bound checks and thus produces fewer false positives.

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 5.3.0
+lastReleaseVersion: 5.4.0

cpp/ql/lib/experimental/cryptography/utils/OpenSSL/CryptoFunction.qll

@@ -115,6 +115,10 @@ private string normalizeFunctionName(Function f, string algType) {
   (result.matches("RSA") implies not f.getName().toUpperCase().matches("%UNIVERSAL%")) and
   //rsaz functions deemed to be too low level, and can be ignored
   not f.getLocation().getFile().getBaseName().matches("rsaz_exp.c") and
+  // SHA false positives
+  (result.matches("SHA") implies not f.getName().toUpperCase().matches("%SHAKE%")) and
+  // CAST false positives
+  (result.matches("CAST") implies not f.getName().toUpperCase().matches(["%UPCAST%", "%DOWNCAST%"])) and
   // General False positives
   // Functions that 'get' do not set an algorithm, and therefore are considered ignorable
   not f.getName().toLowerCase().matches("%get%")

cpp/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-all
-version: 5.3.1-dev
+version: 5.4.0
 groups: cpp
 dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp
@@ -15,6 +15,7 @@ dependencies:
   codeql/tutorial: ${workspace}
   codeql/util: ${workspace}
   codeql/xml: ${workspace}
+  codeql/global-controlflow: ${workspace}
 dataExtensions:
   - ext/*.model.yml
   - ext/generated/**/*.model.yml

cpp/ql/lib/semmle/code/cpp/controlflow/SSA.qll

@@ -15,6 +15,13 @@ class StandardSsa extends SsaHelper {
 }
 
 /**
+ * NOTE: If possible, prefer the SSA classes exposed by the new dataflow
+ * library:
+ * ```
+ * import semmle.code.cpp.dataflow.new.DataFlow
+ * // use `DataFlow::Ssa::Definition`
+ * ```
+ *
  * A definition of one or more SSA variables, including phi node definitions.
  * An _SSA variable_, as defined in the literature, is effectively the pair of
  * an `SsaDefinition d` and a `StackVariable v`, written `(d, v)` in this

cpp/ql/lib/semmle/code/cpp/interproccontrolflow/ControlFlow.qll

@@ -0,0 +1,11 @@
+import cpp
+
+/**
+ * Provides classes for performing global (inter-procedural) control flow analyses.
+ */
+module ControlFlow {
+  private import internal.ControlFlowSpecific
+  private import codeql.globalcontrolflow.ControlFlow
+  import ControlFlowMake<Location, CppControlFlow>
+  import Public
+}

cpp/ql/lib/semmle/code/cpp/interproccontrolflow/internal/ControlFlowPrivate.qll

@@ -0,0 +1,41 @@
+private import semmle.code.cpp.ir.IR
+private import cpp as Cpp
+private import ControlFlowPublic
+private import semmle.code.cpp.ir.dataflow.internal.DataFlowPrivate as DataFlowPrivate
+private import semmle.code.cpp.ir.dataflow.internal.DataFlowImplCommon as DataFlowImplCommon
+
+predicate edge(Node n1, Node n2) { n1.asInstruction().getASuccessor() = n2.asInstruction() }
+
+predicate callTarget(CallNode call, Callable target) {
+  exists(DataFlowPrivate::DataFlowCall dfCall | dfCall.asCallInstruction() = call.asInstruction() |
+    DataFlowImplCommon::viableCallableCached(dfCall).asSourceCallable() = target
+    or
+    DataFlowImplCommon::viableCallableLambda(dfCall, _).asSourceCallable() = target
+  )
+}
+
+predicate flowEntry(Callable c, Node entry) {
+  entry.asInstruction().(EnterFunctionInstruction).getEnclosingFunction() = c
+}
+
+predicate flowExit(Callable c, Node exitNode) {
+  exitNode.asInstruction().(ExitFunctionInstruction).getEnclosingFunction() = c
+}
+
+Callable getEnclosingCallable(Node n) { n.getEnclosingFunction() = result }
+
+predicate hiddenNode(Node n) { n.asInstruction() instanceof PhiInstruction }
+
+private newtype TSplit = TNone() { none() }
+
+class Split extends TSplit {
+  abstract string toString();
+
+  abstract Cpp::Location getLocation();
+
+  abstract predicate entry(Node n1, Node n2);
+
+  abstract predicate exit(Node n1, Node n2);
+
+  abstract predicate blocked(Node n1, Node n2);
+}

cpp/ql/lib/semmle/code/cpp/interproccontrolflow/internal/ControlFlowPublic.qll

@@ -0,0 +1,79 @@
+private import semmle.code.cpp.ir.IR
+private import cpp
+
+private newtype TNode = TInstructionNode(Instruction i)
+
+abstract private class NodeImpl extends TNode {
+  /** Gets the `Instruction` associated with this node, if any. */
+  Instruction asInstruction() { result = this.(InstructionNode).getInstruction() }
+
+  /** Gets the `Expr` associated with this node, if any. */
+  Expr asExpr() { result = this.(ExprNode).getExpr() }
+
+  /** Gets the `Parameter` associated with this node, if any. */
+  Parameter asParameter() { result = this.(ParameterNode).getParameter() }
+
+  /** Gets the location of this node. */
+  Location getLocation() { none() }
+
+  /**
+   * Holds if this element is at the specified location.
+   * The location spans column `startcolumn` of line `startline` to
+   * column `endcolumn` of line `endline` in file `filepath`.
+   * For more information, see
+   * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
+   */
+  final predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+  }
+
+  /** Gets a textual representation of this node. */
+  abstract string toString();
+
+  /** Gets the enclosing callable of this node. */
+  abstract Callable getEnclosingFunction();
+}
+
+final class Node = NodeImpl;
+
+private class InstructionNode extends NodeImpl {
+  Instruction instr;
+
+  InstructionNode() { this = TInstructionNode(instr) }
+
+  /** Gets the `Instruction` associated with this node. */
+  Instruction getInstruction() { result = instr }
+
+  final override Location getLocation() { result = instr.getLocation() }
+
+  final override string toString() { result = instr.getAst().toString() }
+
+  final override Callable getEnclosingFunction() { result = instr.getEnclosingFunction() }
+}
+
+private class ExprNode extends InstructionNode {
+  Expr e;
+
+  ExprNode() { e = this.getInstruction().getConvertedResultExpression() }
+
+  /** Gets the `Expr` associated with this node. */
+  Expr getExpr() { result = e }
+}
+
+private class ParameterNode extends InstructionNode {
+  override InitializeParameterInstruction instr;
+  Parameter p;
+
+  ParameterNode() { p = instr.getParameter() }
+
+  /** Gets the `Parameter` associated with this node. */
+  Parameter getParameter() { result = p }
+}
+
+class CallNode extends InstructionNode {
+  override CallInstruction instr;
+}
+
+class Callable = Function;

cpp/ql/lib/semmle/code/cpp/interproccontrolflow/internal/ControlFlowSpecific.qll

@@ -0,0 +1,19 @@
+/**
+ * Provides IR-specific definitions for use in the data flow library.
+ */
+
+private import cpp
+private import codeql.globalcontrolflow.ControlFlow
+
+module Private {
+  import ControlFlowPrivate
+}
+
+module Public {
+  import ControlFlowPublic
+}
+
+module CppControlFlow implements InputSig<Location> {
+  import Private
+  import Public
+}

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

@@ -1,6 +1,5 @@
 private import cpp
 private import semmle.code.cpp.ir.IR
-private import semmle.code.cpp.ir.dataflow.DataFlow
 private import DataFlowPrivate
 private import DataFlowUtil
 private import DataFlowImplCommon as DataFlowImplCommon
@@ -60,7 +59,7 @@ private module VirtualDispatch {
      * `resolve` predicate to stitch that information together and resolve the
      * call.
      */
-    abstract DataFlow::Node getDispatchValue();
+    abstract Node getDispatchValue();
 
     /** Gets a candidate target for this call. */
     abstract Function resolve();
@@ -72,17 +71,13 @@ private module VirtualDispatch {
      * parameter is true when the search is allowed to continue backwards into
      * a parameter; non-recursive callers should pass `_` for `allowFromArg`.
      */
-    predicate flowsFrom(DataFlow::Node src, boolean allowFromArg) {
+    predicate flowsFrom(Node src, boolean allowFromArg) {
       src = this.getDispatchValue() and allowFromArg = true
       or
-      exists(DataFlow::Node other, boolean allowOtherFromArg |
-        this.flowsFrom(other, allowOtherFromArg)
-      |
+      exists(Node other, boolean allowOtherFromArg | this.flowsFrom(other, allowOtherFromArg) |
         // Call argument
         exists(DataFlowCall call, Position i |
-          other
-              .(DataFlow::ParameterNode)
-              .isParameterOf(pragma[only_bind_into](call).getStaticCallTarget(), i) and
+          other.(ParameterNode).isParameterOf(pragma[only_bind_into](call).getStaticCallTarget(), i) and
           src.(ArgumentNode).argumentOf(call, pragma[only_bind_into](pragma[only_bind_out](i)))
         ) and
         allowOtherFromArg = true and
@@ -96,7 +91,7 @@ private module VirtualDispatch {
         allowFromArg = false
         or
         // Local flow
-        DataFlow::localFlowStep(src, other) and
+        localFlowStep(src, other) and
         allowFromArg = allowOtherFromArg
         or
         // Flow from global variable to load.
@@ -159,11 +154,11 @@ private module VirtualDispatch {
   private class DataSensitiveExprCall extends DataSensitiveCall {
     DataSensitiveExprCall() { not exists(this.getStaticCallTarget()) }
 
-    override DataFlow::Node getDispatchValue() { result.asOperand() = this.getCallTargetOperand() }
+    override Node getDispatchValue() { result.asOperand() = this.getCallTargetOperand() }
 
     override Function resolve() {
       exists(FunctionInstruction fi |
-        this.flowsFrom(DataFlow::instructionNode(fi), _) and
+        this.flowsFrom(instructionNode(fi), _) and
         result = fi.getFunctionSymbol()
       ) and
       (
@@ -186,7 +181,7 @@ private module VirtualDispatch {
       )
     }
 
-    override DataFlow::Node getDispatchValue() { result.asInstruction() = this.getArgument(-1) }
+    override Node getDispatchValue() { result.asInstruction() = this.getArgument(-1) }
 
     override MemberFunction resolve() {
       exists(Class overridingClass |
@@ -213,7 +208,7 @@ private module VirtualDispatch {
     pragma[noinline]
     private predicate hasFlowFromCastFrom(Class derivedClass) {
       exists(ConvertToBaseInstruction toBase |
-        this.flowsFrom(DataFlow::instructionNode(toBase), _) and
+        this.flowsFrom(instructionNode(toBase), _) and
         derivedClass = toBase.getDerivedClass()
       )
     }
@@ -270,7 +265,7 @@ private predicate mayBenefitFromCallContext(
   exists(InitializeParameterInstruction init |
     not exists(call.getStaticCallTarget()) and
     init.getEnclosingFunction() = f.getUnderlyingCallable() and
-    call.flowsFrom(DataFlow::instructionNode(init), _) and
+    call.flowsFrom(instructionNode(init), _) and
     init.getParameter().getIndex() = arg
   )
 }

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

@@ -4,7 +4,7 @@ private import semmle.code.cpp.ir.IR
 private import DataFlowDispatch
 private import semmle.code.cpp.ir.internal.IRCppLanguage
 private import semmle.code.cpp.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
-private import SsaInternals as Ssa
+private import SsaImpl as Ssa
 private import DataFlowImplCommon as DataFlowImplCommon
 private import codeql.util.Unit
 private import Node0ToString
@@ -1982,19 +1982,23 @@ module IteratorFlow {
 
     predicate allowFlowIntoUncertainDef(IteratorSsa::UncertainWriteDefinition def) { any() }
 
+    class GuardValue = Void;
+
     class Guard extends Void {
-      predicate hasBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
+      predicate hasValueBranchEdge(
+        SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, GuardValue val
+      ) {
         none()
       }
 
-      predicate controlsBranchEdge(
-        SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch
+      predicate valueControlsBranchEdge(
+        SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, GuardValue val
       ) {
         none()
       }
     }
 
-    predicate guardDirectlyControlsBlock(Guard guard, SsaInput::BasicBlock bb, boolean branch) {
+    predicate guardDirectlyControlsBlock(Guard guard, SsaInput::BasicBlock bb, GuardValue val) {
       none()
     }
 

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

@@ -13,7 +13,7 @@ private import semmle.code.cpp.models.interfaces.DataFlow
 private import semmle.code.cpp.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
 private import DataFlowPrivate
 private import ModelUtil
-private import SsaInternals as Ssa
+private import SsaImpl as SsaImpl
 private import DataFlowImplCommon as DataFlowImplCommon
 private import codeql.util.Unit
 private import Node0ToString
@@ -39,38 +39,39 @@ private newtype TIRDataFlowNode =
   TNode0(Node0Impl node) { DataFlowImplCommon::forceCachingInSameStage() } or
   TGlobalLikeVariableNode(GlobalLikeVariable var, int indirectionIndex) {
     indirectionIndex =
-      [getMinIndirectionsForType(var.getUnspecifiedType()) .. Ssa::getMaxIndirectionsForType(var.getUnspecifiedType())]
+      [getMinIndirectionsForType(var.getUnspecifiedType()) .. SsaImpl::getMaxIndirectionsForType(var.getUnspecifiedType())]
   } or
   TPostUpdateNodeImpl(Operand operand, int indirectionIndex) {
     operand = any(FieldAddress fa).getObjectAddressOperand() and
-    indirectionIndex = [0 .. Ssa::countIndirectionsForCppType(Ssa::getLanguageType(operand))]
+    indirectionIndex =
+      [0 .. SsaImpl::countIndirectionsForCppType(SsaImpl::getLanguageType(operand))]
     or
-    Ssa::isModifiableByCall(operand, indirectionIndex)
+    SsaImpl::isModifiableByCall(operand, indirectionIndex)
   } or
-  TSsaSynthNode(Ssa::SynthNode n) or
+  TSsaSynthNode(SsaImpl::SynthNode n) or
   TSsaIteratorNode(IteratorFlow::IteratorFlowNode n) or
   TRawIndirectOperand0(Node0Impl node, int indirectionIndex) {
-    Ssa::hasRawIndirectOperand(node.asOperand(), indirectionIndex)
+    SsaImpl::hasRawIndirectOperand(node.asOperand(), indirectionIndex)
   } or
   TRawIndirectInstruction0(Node0Impl node, int indirectionIndex) {
     not exists(node.asOperand()) and
-    Ssa::hasRawIndirectInstruction(node.asInstruction(), indirectionIndex)
+    SsaImpl::hasRawIndirectInstruction(node.asInstruction(), indirectionIndex)
   } or
   TFinalParameterNode(Parameter p, int indirectionIndex) {
-    exists(Ssa::FinalParameterUse use |
+    exists(SsaImpl::FinalParameterUse use |
       use.getParameter() = p and
       use.getIndirectionIndex() = indirectionIndex
     )
   } or
-  TFinalGlobalValue(Ssa::GlobalUse globalUse) or
-  TInitialGlobalValue(Ssa::GlobalDef globalUse) or
+  TFinalGlobalValue(SsaImpl::GlobalUse globalUse) or
+  TInitialGlobalValue(SsaImpl::GlobalDef globalUse) or
   TBodyLessParameterNodeImpl(Parameter p, int indirectionIndex) {
     // Rule out parameters of catch blocks.
     not exists(p.getCatchBlock()) and
     // We subtract one because `getMaxIndirectionsForType` returns the maximum
     // indirection for a glvalue of a given type, and this doesn't apply to
     // parameters.
-    indirectionIndex = [0 .. Ssa::getMaxIndirectionsForType(p.getUnspecifiedType()) - 1] and
+    indirectionIndex = [0 .. SsaImpl::getMaxIndirectionsForType(p.getUnspecifiedType()) - 1] and
     not any(InitializeParameterInstruction init).getParameter() = p
   } or
   TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn)
@@ -81,7 +82,7 @@ private newtype TIRDataFlowNode =
 class FieldAddress extends Operand {
   FieldAddressInstruction fai;
 
-  FieldAddress() { fai = this.getDef() and not Ssa::ignoreOperand(this) }
+  FieldAddress() { fai = this.getDef() and not SsaImpl::ignoreOperand(this) }
 
   /** Gets the field associated with this instruction. */
   Field getField() { result = fai.getField() }
@@ -126,7 +127,7 @@ predicate conversionFlow(
     )
     or
     additional = true and
-    Ssa::isAdditionalConversionFlow(opFrom, instrTo)
+    SsaImpl::isAdditionalConversionFlow(opFrom, instrTo)
   )
   or
   isPointerArith = true and
@@ -183,7 +184,7 @@ class Node extends TIRDataFlowNode {
     or
     this.asOperand().getUse() = block.getInstruction(i)
     or
-    exists(Ssa::SynthNode ssaNode |
+    exists(SsaImpl::SynthNode ssaNode |
       this.(SsaSynthNode).getSynthNode() = ssaNode and
       ssaNode.getBasicBlock() = block and
       ssaNode.getIndex() = i
@@ -364,10 +365,10 @@ class Node extends TIRDataFlowNode {
    * pointed to by `p`.
    */
   Expr asDefinition(boolean uncertain) {
-    exists(StoreInstruction store, Ssa::Definition def |
+    exists(StoreInstruction store, SsaImpl::Definition def |
       store = this.asInstruction() and
       result = asDefinitionImpl(store) and
-      Ssa::defToNode(this, def, _) and
+      SsaImpl::defToNode(this, def, _) and
       if def.isCertain() then uncertain = false else uncertain = true
     )
   }
@@ -627,7 +628,7 @@ class OperandNode extends Node, Node0 {
  * For example, `stripPointers(int*&)` is `int*` and `stripPointers(int*)` is `int`.
  */
 Type stripPointer(Type t) {
-  result = any(Ssa::Indirection ind | ind.getType() = t).getBaseType()
+  result = any(SsaImpl::Indirection ind | ind.getType() = t).getBaseType()
   or
   result = t.(PointerToMemberType).getBaseType()
   or
@@ -694,12 +695,12 @@ class PostFieldUpdateNode extends PostUpdateNodeImpl {
  * in a data flow graph.
  */
 class SsaSynthNode extends Node, TSsaSynthNode {
-  Ssa::SynthNode node;
+  SsaImpl::SynthNode node;
 
   SsaSynthNode() { this = TSsaSynthNode(node) }
 
   /** Gets the synthesized SSA node associated with this node. */
-  Ssa::SynthNode getSynthNode() { result = node }
+  SsaImpl::SynthNode getSynthNode() { result = node }
 
   override DataFlowCallable getEnclosingCallable() {
     result.asSourceCallable() = this.getFunction()
@@ -782,12 +783,12 @@ class SideEffectOperandNode extends Node instanceof IndirectOperand {
  * from a function body.
  */
 class FinalGlobalValue extends Node, TFinalGlobalValue {
-  Ssa::GlobalUse globalUse;
+  SsaImpl::GlobalUse globalUse;
 
   FinalGlobalValue() { this = TFinalGlobalValue(globalUse) }
 
   /** Gets the underlying SSA use. */
-  Ssa::GlobalUse getGlobalUse() { result = globalUse }
+  SsaImpl::GlobalUse getGlobalUse() { result = globalUse }
 
   override DataFlowCallable getEnclosingCallable() {
     result.asSourceCallable() = this.getFunction()
@@ -814,12 +815,12 @@ class FinalGlobalValue extends Node, TFinalGlobalValue {
  * a function body.
  */
 class InitialGlobalValue extends Node, TInitialGlobalValue {
-  Ssa::GlobalDef globalDef;
+  SsaImpl::GlobalDef globalDef;
 
   InitialGlobalValue() { this = TInitialGlobalValue(globalDef) }
 
   /** Gets the underlying SSA definition. */
-  Ssa::GlobalDef getGlobalDef() { result = globalDef }
+  SsaImpl::GlobalDef getGlobalDef() { result = globalDef }
 
   override DataFlowCallable getEnclosingCallable() {
     result.asSourceCallable() = this.getFunction()
@@ -1288,11 +1289,11 @@ class UninitializedNode extends Node {
   LocalVariable v;
 
   UninitializedNode() {
-    exists(Ssa::Definition def, Ssa::SourceVariable sv |
+    exists(SsaImpl::Definition def, SsaImpl::SourceVariable sv |
       def.getIndirectionIndex() = 0 and
       def.getValue().asInstruction() instanceof UninitializedInstruction and
-      Ssa::defToNode(this, def, sv) and
-      v = sv.getBaseVariable().(Ssa::BaseIRVariable).getIRVariable().getAst()
+      SsaImpl::defToNode(this, def, sv) and
+      v = sv.getBaseVariable().(SsaImpl::BaseIRVariable).getIRVariable().getAst()
     )
   }
 
@@ -1722,7 +1723,7 @@ private module Cached {
   cached
   predicate flowsToBackEdge(Node n) {
     exists(Node succ, IRBlock bb1, IRBlock bb2 |
-      Ssa::ssaFlow(n, succ) and
+      SsaImpl::ssaFlow(n, succ) and
       bb1 = n.getBasicBlock() and
       bb2 = succ.getBasicBlock() and
       bb1 != bb2 and
@@ -1820,7 +1821,7 @@ private module Cached {
   predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo, string model) {
     (
       // Def-use/Use-use flow
-      Ssa::ssaFlow(nodeFrom, nodeTo)
+      SsaImpl::ssaFlow(nodeFrom, nodeTo)
       or
       IteratorFlow::localFlowStep(nodeFrom, nodeTo)
       or
@@ -1833,7 +1834,7 @@ private module Cached {
       |
         simpleOperandLocalFlowStep(iFrom, opTo) and
         // Omit when the instruction node also represents the operand.
-        not iFrom = Ssa::getIRRepresentationOfOperand(opTo)
+        not iFrom = SsaImpl::getIRRepresentationOfOperand(opTo)
       )
       or
       // Indirect operand -> (indirect) instruction flow
@@ -1906,7 +1907,7 @@ private module Cached {
       // We also want a write coming out of an `OutNode` to flow `nodeTo`.
       // This is different from `reverseFlowInstruction` since `nodeFrom` can never
       // be an `OutNode` when it's defined by an instruction.
-      Ssa::outNodeHasAddressAndIndex(nodeFrom, address, indirectionIndex)
+      SsaImpl::outNodeHasAddressAndIndex(nodeFrom, address, indirectionIndex)
     )
   }
 
@@ -2099,7 +2100,7 @@ private newtype TContent =
   TFieldContent(Field f, int indirectionIndex) {
     // the indirection index for field content starts at 1 (because `TFieldContent` is thought of as
     // the address of the field, `FieldAddress` in the IR).
-    indirectionIndex = [1 .. Ssa::getMaxIndirectionsForType(f.getUnspecifiedType())] and
+    indirectionIndex = [1 .. SsaImpl::getMaxIndirectionsForType(f.getUnspecifiedType())] and
     // Reads and writes of union fields are tracked using `UnionContent`.
     not f.getDeclaringType() instanceof Union
   } or
@@ -2111,7 +2112,9 @@ private newtype TContent =
       // field can be read by any read of the union's fields. Again, the indirection index
       // is 1-based (because 0 is considered the address).
       indirectionIndex =
-        [1 .. max(Ssa::getMaxIndirectionsForType(getAFieldWithSize(u, bytes).getUnspecifiedType()))]
+        [1 .. max(SsaImpl::getMaxIndirectionsForType(getAFieldWithSize(u, bytes)
+                    .getUnspecifiedType())
+          )]
     )
   } or
   TElementContent(int indirectionIndex) {
@@ -2354,7 +2357,7 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
       controls(g, result, edge)
     )
     or
-    result = Ssa::BarrierGuard<guardChecksNode/3>::getABarrierNode()
+    result = SsaImpl::BarrierGuard<guardChecksNode/3>::getABarrierNode()
   }
 
   /**
@@ -2453,7 +2456,7 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
     )
     or
     result =
-      Ssa::BarrierGuardWithIntParam<guardChecksIndirectNode/4>::getABarrierNode(indirectionIndex)
+      SsaImpl::BarrierGuardWithIntParam<guardChecksIndirectNode/4>::getABarrierNode(indirectionIndex)
   }
 }
 
@@ -2490,7 +2493,7 @@ module InstructionBarrierGuard<instructionGuardChecksSig/3 instructionGuardCheck
       controls(g, result, edge)
     )
     or
-    result = Ssa::BarrierGuard<guardChecksNode/3>::getABarrierNode()
+    result = SsaImpl::BarrierGuard<guardChecksNode/3>::getABarrierNode()
   }
 
   bindingset[value, n]
@@ -2520,7 +2523,7 @@ module InstructionBarrierGuard<instructionGuardChecksSig/3 instructionGuardCheck
     )
     or
     result =
-      Ssa::BarrierGuardWithIntParam<guardChecksIndirectNode/4>::getABarrierNode(indirectionIndex)
+      SsaImpl::BarrierGuardWithIntParam<guardChecksIndirectNode/4>::getABarrierNode(indirectionIndex)
   }
 }
 
@@ -2576,3 +2579,16 @@ Function getARuntimeTarget(Call call) {
     result = DataFlowImplCommon::viableCallableLambda(dfCall, _).asSourceCallable()
   )
 }
+
+/** A module that provides static single assignment (SSA) information. */
+module Ssa {
+  class Definition = SsaImpl::Definition;
+
+  class ExplicitDefinition = SsaImpl::ExplicitDefinition;
+
+  class DirectExplicitDefinition = SsaImpl::DirectExplicitDefinition;
+
+  class IndirectExplicitDefinition = SsaImpl::IndirectExplicitDefinition;
+
+  class PhiNode = SsaImpl::PhiNode;
+}

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

@@ -4,15 +4,15 @@
  */
 
 private import semmle.code.cpp.ir.IR
-private import semmle.code.cpp.ir.dataflow.DataFlow
+private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs
 private import DataFlowUtil
 private import DataFlowPrivate
-private import SsaInternals as Ssa
+private import SsaImpl as Ssa
 
 /**
  * Gets the instruction that goes into `input` for `call`.
  */
-DataFlow::Node callInput(CallInstruction call, FunctionInput input) {
+Node callInput(CallInstruction call, FunctionInput input) {
   // An argument or qualifier
   exists(int index |
     result.asOperand() = call.getArgumentOperand(index) and
@@ -62,8 +62,8 @@ Node callOutput(CallInstruction call, FunctionOutput output) {
   result = callOutputWithIndirectionIndex(call, output, _)
 }
 
-DataFlow::Node callInput(CallInstruction call, FunctionInput input, int d) {
-  exists(DataFlow::Node n | n = callInput(call, input) and d > 0 |
+Node callInput(CallInstruction call, FunctionInput input, int d) {
+  exists(Node n | n = callInput(call, input) and d > 0 |
     // An argument or qualifier
     hasOperandAndIndex(result, n.asOperand(), d)
     or
@@ -85,7 +85,7 @@ private IndirectReturnOutNode getIndirectReturnOutNode(CallInstruction call, int
  */
 bindingset[d]
 Node callOutput(CallInstruction call, FunctionOutput output, int d) {
-  exists(DataFlow::Node n, int indirectionIndex |
+  exists(Node n, int indirectionIndex |
     n = callOutputWithIndirectionIndex(call, output, indirectionIndex) and d > 0
   |
     // The return value

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

@@ -1,6 +1,6 @@
 private import cpp
 private import semmle.code.cpp.ir.IR
-private import SsaInternals as Ssa
+private import SsaImpl as Ssa
 
 /**
  * A property provider that hides all instructions and operands that are not relevant for IR dataflow.

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

@@ -2,7 +2,7 @@ private import cpp
 private import semmle.code.cpp.ir.IR
 private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
 private import semmle.code.cpp.ir.dataflow.internal.DataFlowPrivate
-private import SsaInternals as Ssa
+private import SsaImpl as Ssa
 private import PrintIRUtilities
 
 /**

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

@@ -1,4 +1,4 @@
-private import codeql.ssa.Ssa as SsaImplCommon
+private import codeql.ssa.Ssa as Ssa
 private import semmle.code.cpp.ir.IR
 private import DataFlowUtil
 private import DataFlowImplCommon as DataFlowImplCommon
@@ -12,7 +12,7 @@ private import semmle.code.cpp.ir.internal.IRCppLanguage
 private import semmle.code.cpp.ir.dataflow.internal.ModelUtil
 private import semmle.code.cpp.ir.implementation.raw.internal.TranslatedInitialization
 private import DataFlowPrivate
-import SsaInternalsCommon
+import SsaImplCommon
 
 private module SourceVariables {
   cached
@@ -884,7 +884,7 @@ private predicate baseSourceVariableIsGlobal(
   )
 }
 
-private module SsaInput implements SsaImplCommon::InputSig<Location> {
+private module SsaInput implements Ssa::InputSig<Location> {
   import InputSigCommon
   import SourceVariables
 
@@ -958,9 +958,11 @@ class GlobalDef extends Definition {
   GlobalLikeVariable getVariable() { result = impl.getVariable() }
 }
 
-private module SsaImpl = SsaImplCommon::Make<Location, SsaInput>;
+private module SsaImpl = Ssa::Make<Location, SsaInput>;
 
 private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationInputSig {
+  private import codeql.util.Boolean
+
   class Expr extends Instruction {
     Expr() {
       exists(IRBlock bb, int i |
@@ -992,10 +994,14 @@ private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationI
     result instanceof FalseEdge
   }
 
+  class GuardValue = Boolean;
+
   class Guard instanceof IRGuards::IRGuardCondition {
     string toString() { result = super.toString() }
 
-    predicate hasBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
+    predicate hasValueBranchEdge(
+      SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, GuardValue branch
+    ) {
       exists(EdgeKind kind |
         super.getBlock() = bb1 and
         kind = getConditionalEdge(branch) and
@@ -1003,12 +1009,14 @@ private module DataFlowIntegrationInput implements SsaImpl::DataFlowIntegrationI
       )
     }
 
-    predicate controlsBranchEdge(SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch) {
-      this.hasBranchEdge(bb1, bb2, branch)
+    predicate valueControlsBranchEdge(
+      SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, GuardValue branch
+    ) {
+      this.hasValueBranchEdge(bb1, bb2, branch)
     }
   }
 
-  predicate guardDirectlyControlsBlock(Guard guard, SsaInput::BasicBlock bb, boolean branch) {
+  predicate guardDirectlyControlsBlock(Guard guard, SsaInput::BasicBlock bb, GuardValue branch) {
     guard.(IRGuards::IRGuardCondition).controls(bb, branch)
   }
 
@@ -1037,7 +1045,8 @@ module BarrierGuardWithIntParam<guardChecksNodeSig/4 guardChecksNode> {
   }
 
   private predicate guardChecks(
-    DataFlowIntegrationInput::Guard g, SsaImpl::Definition def, boolean branch, int indirectionIndex
+    DataFlowIntegrationInput::Guard g, SsaImpl::Definition def,
+    DataFlowIntegrationInput::GuardValue branch, int indirectionIndex
   ) {
     exists(UseImpl use |
       guardChecksNode(g, use.getNode(), branch, indirectionIndex) and
@@ -1116,9 +1125,11 @@ class PhiNode extends Definition instanceof SsaImpl::PhiNode {
 
 /** An static single assignment (SSA) definition. */
 class Definition extends SsaImpl::Definition {
-  // TODO: Include prior definitions of uncertain writes or rename predicate
-  // i.e. the disjunct `SsaImpl::uncertainWriteDefinitionInput(this, result)`
-  private Definition getAPhiInputOrPriorDefinition() { result = this.(PhiNode).getAnInput() }
+  private Definition getAPhiInputOrPriorDefinition() {
+    result = this.(PhiNode).getAnInput()
+    or
+    SsaImpl::uncertainWriteDefinitionInput(this, result)
+  }
 
   /**
    * Gets a definition that ultimately defines this SSA definition and is
@@ -1129,6 +1140,36 @@ class Definition extends SsaImpl::Definition {
     not result instanceof PhiNode
   }
 
+  /** Gets an `Operand` that represents a use of this definition. */
+  Operand getAUse() {
+    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()
+    )
+  }
+
+  /**
+   * Gets an `Operand` that represents an indirect use of this definition.
+   *
+   * The use is indirect because the operand represents a pointer that points
+   * to the value written by this definition. For example in:
+   * ```cpp
+   * 1. int x = 42;
+   * 2. int* p = &x;
+   * ```
+   * There is an `ExplicitDefinition` corresponding to `x = 42` on line 1 and
+   * the definition has an indirect use on line 2 because `&x` points to the
+   * value that was defined by the definition.
+   */
+  Operand getAnIndirectUse(int indirectionIndex) {
+    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)
+    )
+  }
+
   /**
    * INTERNAL: Do not use.
    */
@@ -1161,4 +1202,63 @@ class Definition extends SsaImpl::Definition {
   Type getUnspecifiedType() { result = this.getUnderlyingType().getUnspecifiedType() }
 }
 
+/**
+ * An SSA definition that corresponds to an explicit definition.
+ */
+class ExplicitDefinition extends Definition, SsaImpl::WriteDefinition {
+  DefImpl def;
+
+  ExplicitDefinition() {
+    exists(IRBlock bb, int i, SourceVariable sv |
+      this.definesAt(sv, bb, i) and
+      def.hasIndexInBlock(sv, bb, i)
+    )
+  }
+
+  /**
+   * Gets the `Instruction` computing the value that is written to the
+   * associated SSA variable by this SSA definition.
+   *
+   * If `this.getIndirectionIndex() = 0` (i.e., if `this` is an instance of
+   * `DirectExplicitDefinition`) then the SSA variable is present in the source
+   * code.
+   * However, if `this.getIndirectionIndex() > 0` (i.e., if `this` is an
+   * instance of `IndirectExplicitDefinition`) then the SSA variable associated
+   * with this definition represents the memory pointed to by a variable in the
+   * source code.
+   */
+  Instruction getAssignedInstruction() { result = def.getValue().asInstruction() }
+}
+
+/**
+ * An explicit SSA definition that writes an indirect value to a pointer.
+ *
+ * For example in:
+ * ```cpp
+ * int x = 42;  // (1)
+ * int* p = &x; // (2)
+ * ```
+ * There are three `ExplicitDefinition`:
+ * 1. A `DirectExplicitDefinition` at (1) which writes `42` to the SSA variable
+ * corresponding to `x`.
+ * 2. A `DirectExplicitDefinition` at (2) which writes `&x` to the SSA variable
+ * corresponding to `p`.
+ * 3. A `IndirectExplicitDefinition` at (2) which writes `*&x` (i.e., `x`) to
+ * the SSA variable corresponding to `*p`.
+ */
+class IndirectExplicitDefinition extends ExplicitDefinition {
+  IndirectExplicitDefinition() { this.getIndirectionIndex() > 0 }
+}
+
+/**
+ * An SSA definition that corresponds to an explicit definition.
+ *
+ * Unlike `ExplicitDefinition` this class does not include indirect
+ * explicit definition. See `IndirectExplicitDefinition` if you want to include
+ * those.
+ */
+class DirectExplicitDefinition extends ExplicitDefinition {
+  DirectExplicitDefinition() { this.getIndirectionIndex() = 0 }
+}
+
 import SsaCached

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

@@ -5,7 +5,7 @@ private import semmle.code.cpp.models.interfaces.DataFlow
 private import semmle.code.cpp.models.interfaces.SideEffect
 private import DataFlowUtil
 private import DataFlowPrivate
-private import SsaInternals as Ssa
+private import SsaImpl as Ssa
 private import semmle.code.cpp.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
 private import semmle.code.cpp.ir.dataflow.FlowSteps
 

cpp/ql/lib/semmle/code/cpp/models/implementations/Iterator.qll

@@ -26,6 +26,14 @@ private class IteratorTraits extends Class {
   }
 
   Type getIteratorType() { result = this.getTemplateArgument(0) }
+
+  Type getValueType() {
+    exists(TypedefType t |
+      this.getAMember() = t and
+      t.getName() = "value_type" and
+      result = t.getUnderlyingType()
+    )
+  }
 }
 
 /**
@@ -34,16 +42,13 @@ private class IteratorTraits extends Class {
  */
 private class IteratorByTraits extends Iterator {
   IteratorTraits trait;
-
-  IteratorByTraits() { trait.getIteratorType() = this }
-
-  override Type getValueType() {
-    exists(TypedefType t |
-      trait.getAMember() = t and
-      t.getName() = "value_type" and
-      result = t.getUnderlyingType()
-    )
+  IteratorByTraits() {
+    trait.getIteratorType() = this and
+    not trait.getValueType() = this
   }
+
+  override Type getValueType() { result = trait.getValueType() }
+
 }
 
 /**

cpp/ql/src/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 1.4.5
+
+### Minor Analysis Improvements
+
+* The "Initialization code not run" query (`cpp/initialization-not-run`) no longer reports an alert on static global variables that have no dereference.
+
 ## 1.4.4
 
 ### Minor Analysis Improvements

cpp/ql/src/Critical/InitialisationNotRun.ql

@@ -32,9 +32,18 @@ predicate called(Function f) {
   exists(FunctionAccess fa | fa.getTarget() = f)
 }
 
+predicate staticWithoutDereference(GlobalVariable v) {
+  v.isStatic() and
+  not exists(VariableAccess va |
+    va = v.getAnAccess() and
+    dereferenced(va)
+  )
+}
+
 from GlobalVariable v
 where
   global(v) and
+  not staticWithoutDereference(v) and
   not exists(VariableAccess lval |
     v.getAnAccess() = lval and
     lval.isUsedAsLValue() and

cpp/ql/src/Likely Bugs/Leap Year/Adding365DaysPerYear.qhelp

@@ -11,7 +11,7 @@ It is not safe to assume that a year is 365 days long.</p>
 
 <recommendation>
 <p>Determine whether the time span in question contains a leap day, then perform the calculation using the correct number
-of days.  Alternatively, use an established library routine that already contains correct leap year logic.</p>
+of days. Alternatively, use an established library routine that already contains correct leap year logic.</p>
 </recommendation>
 
 <references>

cpp/ql/src/Likely Bugs/Leap Year/Adding365DaysPerYear.ql

@@ -4,8 +4,8 @@
  *              value of 365, it may be a sign that leap years are not taken
  *              into account.
  * @kind problem
- * @problem.severity warning
- * @id cpp/leap-year/adding-365-days-per-year
+ * @problem.severity error
+ * @id cpp/microsoft/public/leap-year/adding-365-days-per-year
  * @precision medium
  * @tags leap-year
  *       correctness
@@ -13,11 +13,13 @@
 
 import cpp
 import LeapYear
+import semmle.code.cpp.dataflow.new.DataFlow
 
 from Expr source, Expr sink
 where
   PossibleYearArithmeticOperationCheckFlow::flow(DataFlow::exprNode(source),
     DataFlow::exprNode(sink))
 select sink,
-  "An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios.",
-  source, source.toString()
+  "$@: This arithmetic operation $@ uses a constant value of 365 ends up modifying the date/time located at $@, without considering leap year scenarios.",
+  sink.getEnclosingFunction(), sink.getEnclosingFunction().toString(), source, source.toString(),
+  sink, sink.toString()

cpp/ql/src/Likely Bugs/Leap Year/AntiPattern5InvalidLeapYearCheck.ql

@@ -0,0 +1,17 @@
+/**
+ * @name Leap Year Invalid Check (AntiPattern 5)
+ * @description An expression is used to check a year is presumably a leap year, but the conditions used are insufficient.
+ * @kind problem
+ * @problem.severity warning
+ * @id cpp/microsoft/public/leap-year/invalid-leap-year-check
+ * @precision medium
+ * @tags leap-year
+ *       correctness
+ */
+
+import cpp
+import LeapYear
+
+from Mod4CheckedExpr exprMod4
+where not exists(ExprCheckLeapYear lyCheck | lyCheck.getAChild*() = exprMod4)
+select exprMod4, "Possible Insufficient Leap Year check (AntiPattern 5)"

cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll

@@ -3,7 +3,7 @@
  */
 
 import cpp
-import semmle.code.cpp.ir.dataflow.TaintTracking
+import semmle.code.cpp.dataflow.new.TaintTracking
 import semmle.code.cpp.commons.DateTime
 
 /**
@@ -41,6 +41,271 @@ class CheckForLeapYearOperation extends Expr {
   }
 }
 
+bindingset[modVal]
+Expr moduloCheckEQ_0(EQExpr eq, int modVal) {
+  exists(RemExpr rem | rem = eq.getLeftOperand() |
+    result = rem.getLeftOperand() and
+    rem.getRightOperand().getValue().toInt() = modVal
+  ) and
+  eq.getRightOperand().getValue().toInt() = 0
+}
+
+bindingset[modVal]
+Expr moduloCheckNEQ_0(NEExpr neq, int modVal) {
+  exists(RemExpr rem | rem = neq.getLeftOperand() |
+    result = rem.getLeftOperand() and
+    rem.getRightOperand().getValue().toInt() = modVal
+  ) and
+  neq.getRightOperand().getValue().toInt() = 0
+}
+
+/**
+ * Returns if the two expressions resolve to the same value, albeit it is a fuzzy attempt.
+ * SSA is not fit for purpose here as calls break SSA equivalence.
+ */
+predicate exprEq_propertyPermissive(Expr e1, Expr e2) {
+  not e1 = e2 and
+  (
+    DataFlow::localFlow(DataFlow::exprNode(e1), DataFlow::exprNode(e2))
+    or
+    if e1 instanceof ThisExpr and e2 instanceof ThisExpr
+    then any()
+    else
+      /* If it's a direct Access, check that the target is the same. */
+      if e1 instanceof Access
+      then e1.(Access).getTarget() = e2.(Access).getTarget()
+      else
+        /* If it's a Call, compare qualifiers and only permit no-argument Calls. */
+        if e1 instanceof Call
+        then
+          e1.(Call).getTarget() = e2.(Call).getTarget() and
+          e1.(Call).getNumberOfArguments() = 0 and
+          e2.(Call).getNumberOfArguments() = 0 and
+          if e1.(Call).hasQualifier()
+          then exprEq_propertyPermissive(e1.(Call).getQualifier(), e2.(Call).getQualifier())
+          else any()
+        else
+          /* If it's a binaryOperation, compare op and recruse */
+          if e1 instanceof BinaryOperation
+          then
+            e1.(BinaryOperation).getOperator() = e2.(BinaryOperation).getOperator() and
+            exprEq_propertyPermissive(e1.(BinaryOperation).getLeftOperand(),
+              e2.(BinaryOperation).getLeftOperand()) and
+            exprEq_propertyPermissive(e1.(BinaryOperation).getRightOperand(),
+              e2.(BinaryOperation).getRightOperand())
+          else
+            // Otherwise fail (and permit the raising of a finding)
+            if e1 instanceof Literal
+            then e1.(Literal).getValue() = e2.(Literal).getValue()
+            else none()
+  )
+}
+
+/**
+ * An expression that is the subject of a mod-4 check.
+ * ie `expr % 4 == 0`
+ */
+class Mod4CheckedExpr extends Expr {
+  Mod4CheckedExpr() { exists(Expr e | e = moduloCheckEQ_0(this, 4)) }
+}
+
+/**
+ * Year Div of 100 not equal to 0:
+ * - `year % 100 != 0`
+ * - `!(year % 100 == 0)`
+ */
+abstract class ExprCheckCenturyComponentDiv100 extends Expr {
+  abstract Expr getYearExpr();
+}
+
+/**
+ * The normal form of the expression `year % 100 != 0`.
+ */
+final class ExprCheckCenturyComponentDiv100Normative extends ExprCheckCenturyComponentDiv100 {
+  ExprCheckCenturyComponentDiv100Normative() { exists(moduloCheckNEQ_0(this, 100)) }
+
+  override Expr getYearExpr() { result = moduloCheckNEQ_0(this, 100) }
+}
+
+/**
+ * The inverted form of the expression `year % 100 != 0`, ie `!(year % 100 == 0)`
+ */
+final class ExprCheckCenturyComponentDiv100Inverted extends ExprCheckCenturyComponentDiv100, NotExpr
+{
+  ExprCheckCenturyComponentDiv100Inverted() { exists(moduloCheckEQ_0(this.getOperand(), 100)) }
+
+  override Expr getYearExpr() { result = moduloCheckEQ_0(this.getOperand(), 100) }
+}
+
+/**
+ * A check that an expression is divisible by 400 or not
+ * - `(year % 400 == 0)`
+ * - `!(year % 400 != 0)`
+ */
+abstract class ExprCheckCenturyComponentDiv400 extends Expr {
+  abstract Expr getYearExpr();
+}
+
+/**
+ * The normative form of expression is divisible by 400:
+ * ie `year % 400 == 0`
+ */
+final class ExprCheckCenturyComponentDiv400Normative extends ExprCheckCenturyComponentDiv400 {
+  ExprCheckCenturyComponentDiv400Normative() { exists(moduloCheckEQ_0(this, 400)) }
+
+  override Expr getYearExpr() {
+    exists(Expr e |
+      e = moduloCheckEQ_0(this, 400) and
+      (
+        if e instanceof ConvertedYearByOffset
+        then result = e.(ConvertedYearByOffset).getYearOperand()
+        else result = e
+      )
+    )
+  }
+}
+
+/**
+ * An arithmetic operation that seemingly converts an operand between time formats.
+ */
+class ConvertedYearByOffset extends BinaryArithmeticOperation {
+  ConvertedYearByOffset() {
+    this.getAnOperand().getValue().toInt() instanceof TimeFormatConversionOffset
+  }
+
+  Expr getYearOperand() {
+    this.getLeftOperand().getValue().toInt() instanceof TimeFormatConversionOffset and
+    result = this.getRightOperand()
+    or
+    this.getRightOperand().getValue().toInt() instanceof TimeFormatConversionOffset and
+    result = this.getLeftOperand()
+  }
+}
+
+/**
+ * A flow configuration to track DataFlow from a `CovertedYearByOffset` to some `StructTmLeapYearFieldAccess`.
+ */
+module LocalConvertedYearByOffsetToLeapYearCheckFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node n) { not n.asExpr() instanceof ConvertedYearByOffset }
+
+  predicate isSink(DataFlow::Node n) { n.asExpr() instanceof StructTmLeapYearFieldAccess }
+}
+
+module LocalConvertedYearByOffsetToLeapYearCheckFlow =
+  DataFlow::Global<LocalConvertedYearByOffsetToLeapYearCheckFlowConfig>;
+
+/**
+ * The set of ints (or strings) which represent a value that is typically used to convert between time data types.
+ */
+final class TimeFormatConversionOffset extends int {
+  TimeFormatConversionOffset() {
+    this =
+      [
+        1900, // tm_year represents years since 1900
+        1970, // converting from/to Unix epoch
+        2000, // some systems may use 2000 for 2-digit year conversions
+      ]
+  }
+}
+
+/**
+ * The inverted form of expression is divisible by 400:
+ * ie `!(year % 400 != 0)`
+ */
+final class ExprCheckCenturyComponentDiv400Inverted extends ExprCheckCenturyComponentDiv400, NotExpr
+{
+  ExprCheckCenturyComponentDiv400Inverted() { exists(moduloCheckNEQ_0(this.getOperand(), 400)) }
+
+  override Expr getYearExpr() { result = moduloCheckNEQ_0(this.getOperand(), 400) }
+}
+
+/**
+ * The Century component of a Leap-Year guard
+ */
+class ExprCheckCenturyComponent extends LogicalOrExpr {
+  ExprCheckCenturyComponent() {
+    exists(ExprCheckCenturyComponentDiv400 exprDiv400, ExprCheckCenturyComponentDiv100 exprDiv100 |
+      this.getAnOperand() = exprDiv100 and
+      this.getAnOperand() = exprDiv400 and
+      exprEq_propertyPermissive(exprDiv100.getYearExpr(), exprDiv400.getYearExpr())
+    )
+  }
+
+  Expr getYearExpr() {
+    exists(ExprCheckCenturyComponentDiv400 exprDiv400 |
+      this.getAnOperand() = exprDiv400 and
+      result = exprDiv400.getYearExpr()
+    )
+  }
+}
+
+/**
+ * A **Valid** Leap year check expression.
+ */
+abstract class ExprCheckLeapYear extends Expr { }
+
+/**
+ * A valid Leap-Year guard expression of the following form:
+ *  `dt.Year % 4 == 0 && (dt.Year % 100 != 0 || dt.Year % 400 == 0)`
+ */
+final class ExprCheckLeapYearFormA extends ExprCheckLeapYear, LogicalAndExpr {
+  ExprCheckLeapYearFormA() {
+    exists(Expr e, ExprCheckCenturyComponent centuryCheck |
+      e = moduloCheckEQ_0(this.getLeftOperand(), 4) and
+      centuryCheck = this.getAnOperand().getAChild*() and
+      exprEq_propertyPermissive(e, centuryCheck.getYearExpr())
+    )
+  }
+}
+
+/**
+ * A valid Leap-Year guard expression of the following forms:
+ *  `year % 400 == 0 || (year % 100 != 0 && year % 4 == 0)`
+ *  `(year + 1900) % 400 == 0 || (year % 100 != 0 && year % 4 == 0)`
+ */
+final class ExprCheckLeapYearFormB extends ExprCheckLeapYear, LogicalOrExpr {
+  ExprCheckLeapYearFormB() {
+    exists(VariableAccess va1, VariableAccess va2, VariableAccess va3 |
+      va1 = moduloCheckEQ_0(this.getAnOperand(), 400) and
+      va2 = moduloCheckNEQ_0(this.getAnOperand().(LogicalAndExpr).getAnOperand(), 100) and
+      va3 = moduloCheckEQ_0(this.getAnOperand().(LogicalAndExpr).getAnOperand(), 4) and
+      // The 400-leap year check may be offset by [1900,1970,2000].
+      exists(Expr va1_subExpr | va1_subExpr = va1.getAChild*() |
+        exprEq_propertyPermissive(va1_subExpr, va2) and
+        exprEq_propertyPermissive(va2, va3)
+      )
+    )
+  }
+}
+
+Expr leapYearOpEnclosingElement(CheckForLeapYearOperation op) { result = op.getEnclosingElement() }
+
+/**
+ * A value that resolves as a constant integer that represents some normalization or conversion between date types.
+ */
+pragma[inline]
+private predicate isNormalizationPrimitiveValue(Expr e) {
+  e.getValue().toInt() = [1900, 2000, 1980, 80]
+}
+
+/**
+ * A normalization operation is an expression that is merely attempting to convert between two different datetime schemes,
+ * and does not apply any additional mutation to the represented value.
+ */
+pragma[inline]
+predicate isNormalizationOperation(Expr e) {
+  isNormalizationPrimitiveValue([e, e.(Operation).getAChild()])
+  or
+  // Special case for transforming marshaled 2-digit year date:
+  // theTime.wYear += 100*value;
+  e.(Operation).getAChild().(MulExpr).getValue().toInt() = 100
+}
+
+/**
+ * Get the field accesses used in a `ExprCheckLeapYear` expression.
+ */
+LeapYearFieldAccess leapYearCheckFieldAccess(ExprCheckLeapYear a) { result = a.getAChild*() }
+
 /**
  * A `YearFieldAccess` that would represent an access to a year field on a struct and is used for arguing about leap year calculations.
  */
@@ -73,48 +338,7 @@ abstract class LeapYearFieldAccess extends YearFieldAccess {
     this.isModified() and
     exists(Operation op |
       op.getAnOperand() = this and
-      (
-        op instanceof AssignArithmeticOperation and
-        not (
-          op.getAChild().getValue().toInt() = 1900
-          or
-          op.getAChild().getValue().toInt() = 2000
-          or
-          op.getAChild().getValue().toInt() = 1980
-          or
-          op.getAChild().getValue().toInt() = 80
-          or
-          // Special case for transforming marshaled 2-digit year date:
-          // theTime.wYear += 100*value;
-          exists(MulExpr mulBy100 | mulBy100 = op.getAChild() |
-            mulBy100.getAChild().getValue().toInt() = 100
-          )
-        )
-        or
-        exists(BinaryArithmeticOperation bao |
-          bao = op.getAnOperand() and
-          // we're specifically interested in calculations that update the existing
-          // value (like `x = x + 1`), so look for a child `YearFieldAccess`.
-          bao.getAChild*() instanceof YearFieldAccess and
-          not (
-            bao.getAChild().getValue().toInt() = 1900
-            or
-            bao.getAChild().getValue().toInt() = 2000
-            or
-            bao.getAChild().getValue().toInt() = 1980
-            or
-            bao.getAChild().getValue().toInt() = 80
-            or
-            // Special case for transforming marshaled 2-digit year date:
-            // theTime.wYear += 100*value;
-            exists(MulExpr mulBy100 | mulBy100 = op.getAChild() |
-              mulBy100.getAChild().getValue().toInt() = 100
-            )
-          )
-        )
-        or
-        op instanceof CrementOperation
-      )
+      not isNormalizationOperation(op)
     )
   }
 
@@ -155,9 +379,7 @@ abstract class LeapYearFieldAccess extends YearFieldAccess {
     // but these centurial years are leap years if they are exactly divisible by 400
     //
     // https://aa.usno.navy.mil/faq/docs/calendars.php
-    this.isUsedInMod4Operation() and
-    this.additionalModulusCheckForLeapYear(400) and
-    this.additionalModulusCheckForLeapYear(100)
+    this = leapYearCheckFieldAccess(_)
   }
 }
 
@@ -175,19 +397,9 @@ class StructTmLeapYearFieldAccess extends LeapYearFieldAccess {
   StructTmLeapYearFieldAccess() { this.getTarget().getName() = "tm_year" }
 
   override predicate isUsedInCorrectLeapYearCheck() {
-    this.isUsedInMod4Operation() and
-    this.additionalModulusCheckForLeapYear(400) and
-    this.additionalModulusCheckForLeapYear(100) and
-    // tm_year represents years since 1900
-    (
-      this.additionalAdditionOrSubstractionCheckForLeapYear(1900)
-      or
-      // some systems may use 2000 for 2-digit year conversions
-      this.additionalAdditionOrSubstractionCheckForLeapYear(2000)
-      or
-      // converting from/to Unix epoch
-      this.additionalAdditionOrSubstractionCheckForLeapYear(1970)
-    )
+    this = leapYearCheckFieldAccess(_) and
+    /* There is some data flow from some conversion arithmetic to this expression. */
+    LocalConvertedYearByOffsetToLeapYearCheckFlow::flow(_, DataFlow::exprNode(this))
   }
 }
 
@@ -206,10 +418,10 @@ class ChecksForLeapYearFunctionCall extends FunctionCall {
 }
 
 /**
- * Data flow configuration for finding a variable access that would flow into
+ * A `DataFlow` configuraiton for finding a variable access that would flow into
  * a function call that includes an operation to check for leap year.
  */
-private module LeapYearCheckConfig implements DataFlow::ConfigSig {
+private module LeapYearCheckFlowConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source.asExpr() instanceof VariableAccess }
 
   predicate isSink(DataFlow::Node sink) {
@@ -217,11 +429,10 @@ private module LeapYearCheckConfig implements DataFlow::ConfigSig {
   }
 }
 
-module LeapYearCheckFlow = DataFlow::Global<LeapYearCheckConfig>;
+module LeapYearCheckFlow = DataFlow::Global<LeapYearCheckFlowConfig>;
 
 /**
- * Data flow configuration for finding an operation with hardcoded 365 that will flow into
- * a `FILEINFO` field.
+ * A `DataFlow` configuration for finding an operation with hardcoded 365 that will flow into a `_FILETIME` field.
  */
 private module FiletimeYearArithmeticOperationCheckConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) {
@@ -246,46 +457,72 @@ module FiletimeYearArithmeticOperationCheckFlow =
   DataFlow::Global<FiletimeYearArithmeticOperationCheckConfig>;
 
 /**
- * Taint configuration for finding an operation with hardcoded 365 that will flow into any known date/time field.
+ * A `DataFlow` configuration for finding an operation with hardcoded 365 that will flow into any known date/time field.
  */
 private module PossibleYearArithmeticOperationCheckConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) {
-    exists(Operation op | op = source.asExpr() |
-      op.getAChild*().getValue().toInt() = 365 and
-      (
-        not op.getParent() instanceof Expr or
-        op.getParent() instanceof Assignment
-      )
-    )
-  }
-
-  predicate isBarrierIn(DataFlow::Node node) { isSource(node) }
-
-  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-    // flow from anything on the RHS of an assignment to a time/date structure to that
-    // assignment.
-    exists(StructLikeClass dds, FieldAccess fa, Assignment aexpr, Expr e |
-      e = node1.asExpr() and
-      fa = node2.asExpr()
-    |
-      (dds instanceof PackedTimeType or dds instanceof UnpackedTimeType) and
-      fa.getQualifier().getUnderlyingType() = dds and
-      aexpr.getLValue() = fa and
-      aexpr.getRValue().getAChild*() = e
-    )
+    // NOTE: addressing current issue with new IR dataflow, where
+    // constant folding occurs before dataflow nodes are associated
+    // with the constituent literals.
+    source.asExpr().getAChild*().getValue().toInt() = 365 and
+    not exists(DataFlow::Node parent | parent.asExpr().getAChild+() = source.asExpr())
   }
 
   predicate isSink(DataFlow::Node sink) {
     exists(StructLikeClass dds, FieldAccess fa, AssignExpr aexpr |
-      aexpr.getRValue() = sink.asExpr()
-    |
       (dds instanceof PackedTimeType or dds instanceof UnpackedTimeType) and
       fa.getQualifier().getUnderlyingType() = dds and
       fa.isModified() and
-      aexpr.getLValue() = fa
+      aexpr.getLValue() = fa and
+      sink.asExpr() = aexpr.getRValue()
     )
   }
 }
 
 module PossibleYearArithmeticOperationCheckFlow =
   TaintTracking::Global<PossibleYearArithmeticOperationCheckConfig>;
+
+/**
+ * A `YearFieldAccess` that is modifying the year by any arithmetic operation.
+ *
+ * NOTE:
+ * To change this class to work for general purpose date transformations that do not check the return value,
+ * make the following changes:
+ *  - change `extends LeapYearFieldAccess` to `extends FieldAccess`.
+ *  - change `this.isModifiedByArithmeticOperation()` to `this.isModified()`.
+ * Expect a lower precision for a general purpose version.
+ */
+class DateStructModifiedFieldAccess extends LeapYearFieldAccess {
+  DateStructModifiedFieldAccess() {
+    exists(Field f, StructLikeClass struct |
+      f.getAnAccess() = this and
+      struct.getAField() = f and
+      struct.getUnderlyingType() instanceof UnpackedTimeType and
+      this.isModifiedByArithmeticOperation()
+    )
+  }
+}
+
+/**
+ * This is a list of APIs that will get the system time, and therefore guarantee that the value is valid.
+ */
+class SafeTimeGatheringFunction extends Function {
+  SafeTimeGatheringFunction() {
+    this.getQualifiedName() = ["GetFileTime", "GetSystemTime", "NtQuerySystemTime"]
+  }
+}
+
+/**
+ * This list of APIs should check for the return value to detect problems during the conversion.
+ */
+class TimeConversionFunction extends Function {
+  TimeConversionFunction() {
+    this.getQualifiedName() =
+      [
+        "FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime",
+        "SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime",
+        "TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime",
+        "RtlTimeToSecondsSince1970", "_mkgmtime"
+      ]
+  }
+}

cpp/ql/src/Likely Bugs/Leap Year/LeapYearConditionalLogic.qhelp

@@ -0,0 +1,26 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+  <include src="LeapYear.inc.qhelp" />  
+  <p>This anti-pattern occurs when a developer uses conditional logic to execute a different path of code for a leap year than for a common year, without fully testing both code paths.</p>
+  <p>Though using a framework or library's leap year function is better than manually calculating the leap year (as described in anti-pattern 5), it can still be a source of errors if the result is used to execute a different code path. The bug is especially easy to be masked if the year is derived from the current time of the system clock. See Prevention Measures for techniques to avoid this bug.</p>
+</overview>
+<recommendation>
+  <ul>
+  <li>Avoid using conditional logic that creates a separate branch in your code for leap year.</li>
+  <li>Ensure your code is testable, and test how it will behave when presented with leap year dates of February 29th and December 31st as inputs.</li>
+  </ul>
+</recommendation>
+<example>
+<p>Note in the following examples, that year, month, and day might instead be .wYear, .wMonth, and .wDay fields of a SYSTEMTIME structure, or might be .tm_year, .tm_mon, and .tm_mday fields of a struct tm.</p>
+<sample src="examples/LeapYearConditionalLogicBad.c" />
+</example>
+
+<references>
+  <li>NASA / Goddard Space Flight Center - <a href="https://eclipse.gsfc.nasa.gov/SEhelp/calendars.html">Calendars</a></li>
+  <li>Wikipedia - <a href="https://en.wikipedia.org/wiki/Leap_year_bug"> Leap year bug</a> </li>
+  <li>Microsoft Azure blog - <a href="https://azure.microsoft.com/en-us/blog/is-your-code-ready-for-the-leap-year/"> Is your code ready for the leap year?</a> </li>
+</references>
+</qhelp>

cpp/ql/src/Likely Bugs/Leap Year/LeapYearConditionalLogic.ql

@@ -0,0 +1,28 @@
+/**
+ * @name Leap Year Conditional Logic (AntiPattern 7)
+ * @description Conditional logic is present for leap years and common years, potentially leading to untested code pathways.
+ * @kind problem
+ * @problem.severity warning
+ * @id cpp/microsoft/public/leap-year/conditional-logic-branches
+ * @precision medium
+ * @tags leap-year
+ *       correctness
+ */
+
+import cpp
+import LeapYear
+import semmle.code.cpp.dataflow.new.DataFlow
+
+class IfStmtLeapYearCheck extends IfStmt {
+  IfStmtLeapYearCheck() {
+    this.hasElse() and
+    exists(ExprCheckLeapYear lyCheck, DataFlow::Node source, DataFlow::Node sink |
+      source.asExpr() = lyCheck and
+      sink.asExpr() = this.getCondition() and
+      DataFlow::localFlow(source, sink)
+    )
+  }
+}
+
+from IfStmtLeapYearCheck lyCheckIf
+select lyCheckIf, "Leap Year conditional statement may have untested code paths"

cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.qhelp

@@ -15,10 +15,10 @@
 </recommendation>
 <example>
 <p>In this example, we are adding 1 year to the current date. This may work most of the time, but on any given February 29th, the resulting value will be invalid.</p>
-<sample src="UncheckedLeapYearAfterYearModificationBad.c" />
+<sample src="examples/UncheckedLeapYearAfterYearModificationBad.c" />
 
 <p>To fix this bug, check the result for leap year.</p>
-<sample src="UncheckedLeapYearAfterYearModificationGood.c" />
+<sample src="examples/UncheckedLeapYearAfterYearModificationGood.c" />
 </example>
 
 <references>

cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql

@@ -1,9 +1,9 @@
 /**
- * @name Year field changed using an arithmetic operation without checking for leap year
+ * @name Year field changed using an arithmetic operation without checking for leap year (AntiPattern 1)
  * @description A field that represents a year is being modified by an arithmetic operation, but no proper check for leap years can be detected afterwards.
  * @kind problem
  * @problem.severity warning
- * @id cpp/leap-year/unchecked-after-arithmetic-year-modification
+ * @id cpp/microsoft/public/leap-year/unchecked-after-arithmetic-year-modification
  * @precision medium
  * @tags leap-year
  *       correctness
@@ -12,13 +12,16 @@
 import cpp
 import LeapYear
 
-from Variable var, LeapYearFieldAccess yfa
-where
-  exists(VariableAccess va |
+/**
+ * Holds if there is no known leap-year verification for the given `YearWriteOp`.
+ * Binds the `var` argument to the qualifier of the `ywo` argument.
+ */
+bindingset[ywo]
+predicate isYearModifedWithoutExplicitLeapYearCheck(Variable var, YearWriteOp ywo) {
+  exists(VariableAccess va, YearFieldAccess yfa |
+    yfa = ywo.getYearAccess() and
     yfa.getQualifier() = va and
     var.getAnAccess() = va and
-    // The year is modified with an arithmetic operation. Avoid values that are likely false positives
-    yfa.isModifiedByArithmeticOperationNotForNormalization() and
     // Avoid false positives
     not (
       // If there is a local check for leap year after the modification
@@ -41,8 +44,10 @@ where
         LeapYearCheckFlow::flow(DataFlow::exprNode(yfacheck), DataFlow::exprNode(fc.getAnArgument()))
       )
       or
-      // If there is a successor or predecessor that sets the month = 1
-      exists(MonthFieldAccess mfa, AssignExpr ae |
+      // If there is a successor or predecessor that sets the month or day to a fixed value
+      exists(FieldAccess mfa, AssignExpr ae, int val |
+        mfa instanceof MonthFieldAccess or mfa instanceof DayFieldAccess
+      |
         mfa.getQualifier() = var.getAnAccess() and
         mfa.isModified() and
         (
@@ -50,10 +55,87 @@ where
           yfa.getBasicBlock() = mfa.getBasicBlock().getASuccessor+()
         ) and
         ae = mfa.getEnclosingElement() and
-        ae.getAnOperand().getValue().toInt() = 1
+        ae.getAnOperand().getValue().toInt() = val
       )
     )
   )
-select yfa,
-  "Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found.",
-  yfa.getTarget(), yfa.getTarget().toString(), var, var.toString()
+}
+
+/**
+ * The set of all write operations to the Year field of a date struct.
+ */
+abstract class YearWriteOp extends Operation {
+  /** Extracts the access to the Year field */
+  abstract YearFieldAccess getYearAccess();
+
+  /** Get the expression which represents the new value. */
+  abstract Expr getMutationExpr();
+}
+
+/**
+ * A unary operation (Crement) performed on a Year field.
+ */
+class YearWriteOpUnary extends YearWriteOp, UnaryOperation {
+  YearWriteOpUnary() { this.getOperand() instanceof YearFieldAccess }
+
+  override YearFieldAccess getYearAccess() { result = this.getOperand() }
+
+  override Expr getMutationExpr() { result = this }
+}
+
+/**
+ * An assignment operation or mutation on the Year field of a date object.
+ */
+class YearWriteOpAssignment extends YearWriteOp, Assignment {
+  YearWriteOpAssignment() { this.getLValue() instanceof YearFieldAccess }
+
+  override YearFieldAccess getYearAccess() { result = this.getLValue() }
+
+  override Expr getMutationExpr() {
+    // Note: may need to use DF analysis to pull out the original value,
+    // if there is excessive false positives.
+    if this.getOperator() = "="
+    then
+      exists(DataFlow::Node source, DataFlow::Node sink |
+        sink.asExpr() = this.getRValue() and
+        OperationToYearAssignmentFlow::flow(source, sink) and
+        result = source.asExpr()
+      )
+    else result = this
+  }
+}
+
+/**
+ * A DataFlow configuration for identifying flows from some non trivial access or literal
+ * to the Year field of a date object.
+ */
+module OperationToYearAssignmentConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node n) {
+    not n.asExpr() instanceof Access and
+    not n.asExpr() instanceof Literal
+  }
+
+  predicate isSink(DataFlow::Node n) {
+    exists(Assignment a |
+      a.getLValue() instanceof YearFieldAccess and
+      a.getRValue() = n.asExpr()
+    )
+  }
+}
+
+module OperationToYearAssignmentFlow = DataFlow::Global<OperationToYearAssignmentConfig>;
+
+from Variable var, YearWriteOp ywo, Expr mutationExpr
+where
+  mutationExpr = ywo.getMutationExpr() and
+  isYearModifedWithoutExplicitLeapYearCheck(var, ywo) and
+  not isNormalizationOperation(mutationExpr) and
+  not ywo instanceof AddressOfExpr and
+  not exists(Call c, TimeConversionFunction f | f.getACallToThisFunction() = c |
+    c.getAnArgument().getAChild*() = var.getAnAccess() and
+    ywo.getASuccessor*() = c
+  )
+select ywo,
+  "$@: Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found.",
+  ywo.getEnclosingFunction(), ywo.getEnclosingFunction().toString(),
+  ywo.getYearAccess().getTarget(), ywo.getYearAccess().getTarget().toString(), var, var.toString()

cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.qhelp

@@ -27,10 +27,10 @@
 </recommendation>
 <example>
 <p>In this example, we are adding 1 year to the current date. This may work most of the time, but on any given February 29th, the resulting value will be invalid.</p>
-<sample src="UncheckedLeapYearAfterYearModificationBad.c" />
+<sample src="examples/UncheckedLeapYearAfterYearModificationBad.c" />
 
 <p>To fix this bug, you must verify the return value for <code>SystemTimeToFileTime</code> and handle any potential error accordingly.</p>
-<sample src="UncheckedLeapYearAfterYearModificationGood.c" />
+<sample src="examples/UncheckedLeapYearAfterYearModificationGood.c" />
 </example>
 
 <references>

cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql

@@ -1,11 +1,11 @@
 /**
- * @name Unchecked return value for time conversion function
+ * @name Unchecked return value for time conversion function (AntiPattern 6)
  * @description When the return value of a fallible time conversion function is
  *              not checked for failure, its output parameters may contain
  *              invalid dates.
  * @kind problem
  * @problem.severity warning
- * @id cpp/leap-year/unchecked-return-value-for-time-conversion-function
+ * @id cpp/microsoft/public/leap-year/unchecked-return-value-for-time-conversion-function
  * @precision medium
  * @tags leap-year
  *       correctness
@@ -14,51 +14,6 @@
 import cpp
 import LeapYear
 
-/**
- * A `YearFieldAccess` that is modifying the year by any arithmetic operation.
- *
- * NOTE:
- * To change this class to work for general purpose date transformations that do not check the return value,
- * make the following changes:
- *  - change `extends LeapYearFieldAccess` to `extends FieldAccess`.
- *  - change `this.isModifiedByArithmeticOperation()` to `this.isModified()`.
- * Expect a lower precision for a general purpose version.
- */
-class DateStructModifiedFieldAccess extends LeapYearFieldAccess {
-  DateStructModifiedFieldAccess() {
-    exists(Field f, StructLikeClass struct |
-      f.getAnAccess() = this and
-      struct.getAField() = f and
-      struct.getUnderlyingType() instanceof UnpackedTimeType and
-      this.isModifiedByArithmeticOperation()
-    )
-  }
-}
-
-/**
- * This is a list of APIs that will get the system time, and therefore guarantee that the value is valid.
- */
-class SafeTimeGatheringFunction extends Function {
-  SafeTimeGatheringFunction() {
-    this.getQualifiedName() = ["GetFileTime", "GetSystemTime", "NtQuerySystemTime"]
-  }
-}
-
-/**
- * This list of APIs should check for the return value to detect problems during the conversion.
- */
-class TimeConversionFunction extends Function {
-  TimeConversionFunction() {
-    this.getQualifiedName() =
-      [
-        "FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime",
-        "SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime",
-        "TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime",
-        "RtlTimeToSecondsSince1970", "_mkgmtime"
-      ]
-  }
-}
-
 from FunctionCall fcall, TimeConversionFunction trf, Variable var
 where
   fcall = trf.getACallToThisFunction() and
@@ -104,5 +59,6 @@ where
     )
   )
 select fcall,
-  "Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe.",
-  trf, trf.getQualifiedName().toString(), var, var.getName()
+  "$@: Return value of $@ function should be verified to check for any error because variable $@ is not guaranteed to be safe.",
+  fcall.getEnclosingFunction(), fcall.getEnclosingFunction().toString(), trf,
+  trf.getQualifiedName().toString(), var, var.getName()

cpp/ql/src/Likely Bugs/Leap Year/UnsafeArrayForDaysOfYear.qhelp

@@ -16,15 +16,15 @@
 </recommendation>
 <example>
 <p>In this example, we are allocating 365 integers, one for each day of the year. This code will fail on a leap year, when there are 366 days.</p>
-<sample src="UnsafeArrayForDaysOfYearBad.c" />
+<sample src="examples/UnsafeArrayForDaysOfYearBad.c" />
 
 <p>When using arrays, allocate the correct number of elements to match the year.</p>
-<sample src="UnsafeArrayForDaysOfYearGood.c" />
+<sample src="examples/UnsafeArrayForDaysOfYearGood.c" />
 </example>
 
 <references>
   <li>NASA / Goddard Space Flight Center - <a href="https://eclipse.gsfc.nasa.gov/SEhelp/calendars.html">Calendars</a></li>
-  <li>Wikipedia - <a href="https://en.wikipedia.org/wiki/Leap_year_bug"> Leap year bug</a> </li>
+  <li>Wikipedia - <a href="https://en.wikipedia.org/wiki/Leap_year_bug"> Leap year bug</a></li>
   <li>Microsoft Azure blog - <a href="https://azure.microsoft.com/en-us/blog/is-your-code-ready-for-the-leap-year/"> Is your code ready for the leap year?</a> </li>
 </references>
 </qhelp>

cpp/ql/src/Likely Bugs/Leap Year/UnsafeArrayForDaysOfYear.ql

@@ -1,41 +1,62 @@
 /**
- * @name Unsafe array for days of the year
+ * @name Unsafe array for days of the year (AntiPattern 4)
  * @description An array of 365 items typically indicates one entry per day of the year, but without considering leap years, which would be 366 days.
  *              An access on a leap year could result in buffer overflow bugs.
  * @kind problem
  * @problem.severity warning
- * @id cpp/leap-year/unsafe-array-for-days-of-the-year
+ * @id cpp/microsoft/public/leap-year/unsafe-array-for-days-of-the-year
  * @precision low
- * @tags security
- *       leap-year
+ * @tags leap-year
+ *       correctness
  */
 
 import cpp
 
-class LeapYearUnsafeDaysOfTheYearArrayType extends ArrayType {
-  LeapYearUnsafeDaysOfTheYearArrayType() { this.getArraySize() = 365 }
-}
+/* Note: We used to have a `LeapYearUnsafeDaysOfTheYearArrayType` class which was the
+	set of ArrayType that had a fixed length of 365. However, to eliminate false positives,
+	we use `isElementAnArrayOfFixedSize` that *also* finds arrays of 366 items, where the programmer
+	has also catered for leap years. 
+	So, instead of `instanceof` checks, for simplicity, we simply pass in 365/366 as integers as needed.
+*/
 
-from Element element, string allocType
-where
+bindingset[size]
+predicate isElementAnArrayOfFixedSize(
+  Element element, Type t, Declaration f, string allocType, int size
+) {
   exists(NewArrayExpr nae |
     element = nae and
-    nae.getAllocatedType() instanceof LeapYearUnsafeDaysOfTheYearArrayType and
-    allocType = "an array allocation"
+    nae.getAllocatedType().(ArrayType).getArraySize() = size and
+    allocType = "an array allocation" and
+    f = nae.getEnclosingFunction() and
+    t = nae.getAllocatedType().(ArrayType).getBaseType()
   )
   or
   exists(Variable var |
     var = element and
-    var.getType() instanceof LeapYearUnsafeDaysOfTheYearArrayType and
-    allocType = "an array allocation"
+    var.getType().(ArrayType).getArraySize() = size and
+    allocType = "an array allocation" and
+    f = var and
+    t = var.getType().(ArrayType).getBaseType()
   )
   or
   exists(ConstructorCall cc |
     element = cc and
     cc.getTarget().hasName("vector") and
-    cc.getArgument(0).getValue().toInt() = 365 and
-    allocType = "a std::vector allocation"
+    cc.getArgument(0).getValue().toInt() = size and
+    allocType = "a std::vector allocation" and
+    f = cc.getEnclosingFunction() and
+    t = cc.getTarget().getDeclaringType()
+  )
+}
+
+from Element element, string allocType, Declaration f, Type t
+where
+  isElementAnArrayOfFixedSize(element, t, f, allocType, 365) and
+  not exists(Element element2, Declaration f2 |
+    isElementAnArrayOfFixedSize(element2, t, f2, _, 366) and
+    if f instanceof Function then f = f2 else f.getParentScope() = f2.getParentScope()
   )
 select element,
-  "There is " + allocType +
-    " with a hard-coded set of 365 elements, which may indicate the number of days in a year without considering leap year scenarios."
+  "$@: There is " + allocType +
+    " with a hard-coded set of 365 elements, which may indicate the number of days in a year without considering leap year scenarios.",
+  f, f.toString()

cpp/ql/src/Likely Bugs/Leap Year/examples/LeapYearConditionalLogicBad.c

@@ -0,0 +1,21 @@
+// Checking for leap year
+bool isLeapYear = year % 4 == 0 && (year % 100 != 0 || year % 400 == 0);
+if (isLeapYear)
+{
+    // untested path
+}
+else
+{
+    // tested path
+}
+
+
+// Checking specifically for the leap day
+if (month == 2 && day == 29)  // (or 1 with a tm_mon value)
+{
+    // untested path
+}
+else
+{
+    // tested path
+}

cpp/ql/src/Microsoft/Likely Bugs/Conversion/BadOverflowGuard.qhelp

@@ -0,0 +1,20 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+  <p>Checking for overflow of an addition by comparing against one of the arguments of the addition fails if the size of all the argument types are smaller than 4 bytes. This is because the result of the addition is promoted to a 4 byte int.</p>
+</overview>
+  
+<recommendation>
+<p>Check the overflow by comparing the addition against a value that is at least 4 bytes.</p>
+</recommendation>
+
+<example>
+  <p>In this example, the result of the comparison will result in an integer overflow.</p>
+  <sample src="BadOverflowGuardBadCode.c" />
+
+  <p>To fix the bug, check the overflow by comparing the addition against a value that is at least 4 bytes.</p>
+  <sample src="BadOverflowGuardGoodCode.c" />
+</example>
+</qhelp>

cpp/ql/src/Microsoft/Likely Bugs/Conversion/BadOverflowGuard.ql

@@ -0,0 +1,31 @@
+/**
+ * @name Bad overflow check
+ * @description Checking for overflow of an addition by comparing against one
+ *              of the arguments of the addition fails if the size of all the
+ *              argument types are smaller than 4 bytes. This is because the
+ *              result of the addition is promoted to a 4 byte int.
+ * @kind problem
+ * @problem.severity error
+ * @tags security
+ *       external/cwe/cwe-190
+ *       external/cwe/cwe-191
+ * @id cpp/microsoft/public/badoverflowguard
+ */
+
+import cpp
+
+/*
+ * Example:
+ *
+ * uint16 v, uint16 b
+ * if ((v + b < v) <-- bad check for overflow
+ */
+
+from AddExpr a, Variable v, RelationalOperation cmp
+where
+  a.getAnOperand() = v.getAnAccess() and
+  forall(Expr op | op = a.getAnOperand() | op.getType().getSize() < 4) and
+  cmp.getAnOperand() = a and
+  cmp.getAnOperand() = v.getAnAccess() and
+  not a.getExplicitlyConverted().getType().getSize() < 4
+select cmp, "Bad overflow check"

cpp/ql/src/Microsoft/Likely Bugs/Conversion/BadOverflowGuardBadCode.c

@@ -0,0 +1,9 @@
+unsigned short CheckForInt16OverflowBadCode(unsigned short v, unsigned short b)
+{
+    if (v + b < v) // BUG: "v + b" will be promoted to 32 bits
+    {
+        // ... do something
+    }
+
+    return v + b;
+}

cpp/ql/src/Microsoft/Likely Bugs/Conversion/BadOverflowGuardGoodCode.c

@@ -0,0 +1,9 @@
+unsigned short CheckForInt16OverflowCorrectCode(unsigned short v, unsigned short b)
+{
+    if (v + b > 0x00FFFF)
+    {
+        // ... do something
+    }
+
+    return v + b;
+}

cpp/ql/src/Microsoft/Likely Bugs/Drivers/IncorrectUsageOfRtlCompareMemory.qhelp

@@ -0,0 +1,29 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p><code>RtlCompareMemory</code> routine compares two blocks of memory and returns the number of bytes that match, not a boolean value indicating a full comparison like <code>RtlEqualMemory</code> does.</p> 
+    <p>This query detects the return value of <code>RtlCompareMemory</code> being handled as a boolean.</p>
+  </overview>
+
+<recommendation>
+  <p>Any findings from this rule may indicate that the return value of a call to <code>RtlCompareMemory</code> is being incorrectly interpreted as a boolean.</p>
+  <p>Review the logic of the call, and if necessary, replace the function call with <code>RtlEqualMemory</code>.</p>
+</recommendation>
+
+<example>
+<p>The following example is a typical one where an identity comparison is intended, but the wrong API is being used.</p>
+<sample src="IncorrectUsageOfRtlCompareMemoryBad.c" />
+
+<p>In this example, the fix is to replace the call to <code>RtlCompareMemory</code> with <code>RtlEqualMemory</code>.</p>
+
+<sample src="IncorrectUsageOfRtlCompareMemoryGood.c" />
+
+</example>
+<references>
+    <li>Books online <a href="https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-rtlcomparememory">RtlCompareMemory function (wdm.h)</a></li>
+    <li>Books online <a href="https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-rtlequalmemory">RtlEqualMemory macro (wdm.h)</a></li>
+</references>
+
+</qhelp>

cpp/ql/src/Microsoft/Likely Bugs/Drivers/IncorrectUsageOfRtlCompareMemory.ql

@@ -0,0 +1,69 @@
+/**
+ * @id cpp/microsoft/public/drivers/incorrect-usage-of-rtlcomparememory
+ * @name Incorrect usage of RtlCompareMemory
+ * @description `RtlCompareMemory` routine compares two blocks of memory and returns the number of bytes that match, not a boolean value indicating a full comparison like `RtlEqualMemory` does.
+ *     This query detects the return value of `RtlCompareMemory` being handled as a boolean.
+ * @security.severity Important
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ *       kernel
+ */
+
+import cpp
+
+predicate isLiteralABooleanMacro(Literal l) {
+  exists(MacroInvocation mi | mi.getExpr() = l |
+    mi.getMacroName() in ["true", "false", "TRUE", "FALSE"]
+  )
+}
+
+from FunctionCall fc, Function f, Expr e, string msg
+where
+  f.getQualifiedName() = "RtlCompareMemory" and
+  f.getACallToThisFunction() = fc and
+  (
+    exists(UnaryLogicalOperation ulo | e = ulo |
+      ulo.getAnOperand() = fc and
+      msg = "as an operand in an unary logical operation"
+    )
+    or
+    exists(BinaryLogicalOperation blo | e = blo |
+      blo.getAnOperand() = fc and
+      msg = "as an operand in a binary logical operation"
+    )
+    or
+    exists(Conversion conv | e = conv |
+      (
+        conv.getType().hasName("bool") or
+        conv.getType().hasName("BOOLEAN") or
+        conv.getType().hasName("_Bool")
+      ) and
+      conv.getUnconverted() = fc and
+      msg = "as a boolean"
+    )
+    or
+    exists(IfStmt s | e = s.getControllingExpr() |
+      s.getControllingExpr() = fc and
+      msg = "as the controlling expression in an If statement"
+    )
+    or
+    exists(EqualityOperation bao, Expr e2 | e = bao |
+      bao.hasOperands(fc, e2) and
+      isLiteralABooleanMacro(e2) and
+      msg =
+        "as an operand in an equality operation where the other operand is a boolean value (high precision result)"
+    )
+    or
+    exists(EqualityOperation bao, Expr e2 | e = bao |
+      bao.hasOperands(fc, e2) and
+      (e2.(Literal).getValue().toInt() = 1 or e2.(Literal).getValue().toInt() = 0) and
+      not isLiteralABooleanMacro(e2) and
+      msg =
+        "as an operand in an equality operation where the other operand is likely a boolean value (lower precision result, needs to be reviewed)"
+    )
+  )
+select e,
+  "This $@ is being handled $@ instead of the number of matching bytes. Please review the usage of this function and consider replacing it with `RtlEqualMemory`.",
+  fc, "call to `RtlCompareMemory`", e, msg

cpp/ql/src/Microsoft/Likely Bugs/Drivers/IncorrectUsageOfRtlCompareMemoryBad.c

@@ -0,0 +1,5 @@
+//bug, the code assumes RtlCompareMemory is comparing for identical values & return false if not identical
+if (!RtlCompareMemory(pBuffer, ptr, 16)) 
+{
+	return FALSE;
+}

cpp/ql/src/Microsoft/Likely Bugs/Drivers/IncorrectUsageOfRtlCompareMemoryGood.c

@@ -0,0 +1,5 @@
+//fixed
+if (!RtlEqualMemory(pBuffer, ptr, 16))
+{
+	return FALSE;
+}

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/ArgumentIsSizeofOrOperation.qhelp

@@ -0,0 +1,22 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>If the argument for a <code>sizeof</code> call is a binary operation or a <code>sizeof</code> call, it is typically a sign that there is a confusion on the usage of the sizeof usage.</p>
+  </overview>
+
+<recommendation>
+  <p>Any findings from this rule may indicate that the <code>sizeof</code> is being used incorrectly.</p>
+  <p>Review the logic of the call.</p>
+</recommendation>
+
+<example>
+  <p>The following example shows a case where <code>sizeof</code> a binary operation by mistake.</p>
+  <sample src="ArgumentIsSizeofOrOperationBad.c" />
+
+  <p>In this example, the fix is to multiply the result of <code>sizeof</code> by the number of elements.</p>
+  <sample src="ArgumentIsSizeofOrOperationGood.c" />
+</example>
+
+</qhelp>

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/ArgumentIsSizeofOrOperation.ql

@@ -0,0 +1,62 @@
+/**
+ * @id cpp/microsoft/public/sizeof/sizeof-or-operation-as-argument
+ * @name Usage of an expression that is a binary operation, or sizeof call passed as an argument to a sizeof call
+ * @description When the `expr` passed to `sizeof` is a binary operation, or a sizeof call, this is typically a sign that there is a confusion on the usage of sizeof.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ */
+
+import cpp
+import SizeOfTypeUtils
+
+/**
+ * Windows SDK corecrt_math.h defines a macro _CLASS_ARG that
+ * intentionally misuses sizeof to determine the size of a floating point type.
+ * Explicitly ignoring any hit in this macro.
+ */
+predicate isPartOfCrtFloatingPointMacroExpansion(Expr e) {
+  exists(MacroInvocation mi |
+    mi.getMacroName() = "_CLASS_ARG" and
+    mi.getMacro().getFile().getBaseName() = "corecrt_math.h" and
+    mi.getAnExpandedElement() = e
+  )
+}
+
+/**
+ * Determines if the sizeOfExpr is ignorable.
+ */
+predicate ignorableSizeof(SizeofExprOperator sizeofExpr) {
+  // a common pattern found is to sizeof a binary operation to check a type
+  // to then perfomr an onperaiton for a 32 or 64 bit type.
+  // these cases often look like sizeof(x) >=4
+  // more generally we see binary operations frequently used in different type
+  // checks, where the sizeof is part of some comparison operation of a switch statement guard.
+  // sizeof as an argument is also similarly used, but seemingly less frequently.
+  exists(ComparisonOperation comp | comp.getAnOperand() = sizeofExpr)
+  or
+  exists(ConditionalStmt s | s.getControllingExpr() = sizeofExpr)
+  or
+  // another common practice is to use bit-wise operations in sizeof to allow the compiler to
+  // 'pack' the size appropriate but get the size of the result out of a sizeof operation.
+  sizeofExpr.getExprOperand() instanceof BinaryBitwiseOperation
+}
+
+from SizeofExprOperator sizeofExpr, string message, Expr op
+where
+  exists(string tmpMsg |
+    (
+      op instanceof BinaryOperation and tmpMsg = "binary operator"
+      or
+      op instanceof SizeofOperator and tmpMsg = "sizeof"
+    ) and
+    if sizeofExpr.isInMacroExpansion()
+    then message = tmpMsg + "(in a macro expansion)"
+    else message = tmpMsg
+  ) and
+  op = sizeofExpr.getExprOperand() and
+  not isPartOfCrtFloatingPointMacroExpansion(op) and
+  not ignorableSizeof(sizeofExpr)
+select sizeofExpr, "$@: $@ of $@ inside sizeof.", sizeofExpr, message,
+  sizeofExpr.getEnclosingFunction(), "Usage", op, message

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/ArgumentIsSizeofOrOperationBad.c

@@ -0,0 +1,5 @@
+#define SIZEOF_CHAR sizeof(char)
+
+char* buffer;
+// bug - the code is really going to allocate sizeof(size_t) instead o fthe intended sizeof(char) * 10
+buffer = (char*) malloc(sizeof(SIZEOF_CHAR * 10));

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/ArgumentIsSizeofOrOperationGood.c

@@ -0,0 +1,4 @@
+#define SIZEOF_CHAR sizeof(char)
+
+char* buffer;
+buffer = (char*) malloc(SIZEOF_CHAR * 10);

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/SizeOfConstIntMacro.qhelp

@@ -0,0 +1,26 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>If the argument for a <code>sizeof</code> call is a macro that expands to a constant integer type, it is a likely indication that the macro operation may be misused or that the argument was selected by mistake (i.e. typo).</p> 
+    <p>This query detects if the argument for <code>sizeof</code> is a macro that expands to a constant integer value.</p>
+    <p>NOTE: This rule will ignore multicharacter literal values that are exactly 4 bytes long as it matches the length of <code>int</code> and may be expected.</p>
+  </overview>
+
+<recommendation>
+  <p>Any findings from this rule may indicate that the <code>sizeof</code> is being used incorrectly.</p>
+  <p>Review the logic of the call.</p>
+</recommendation>
+
+<example>
+<p>The following example shows a case where <code>sizeof</code> a constant was used instead of the <code>sizeof</code> of a structure by mistake as the names are similar.</p>
+<sample src="SizeOfConstIntMacroBad.c" />
+
+<p>In this example, the fix is to replace the argument for <code>sizeof</code> with the structure name.</p>
+
+<sample src="SizeOfConstIntMacroGood.c" />
+
+</example>
+
+</qhelp>

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/SizeOfConstIntMacro.ql

@@ -0,0 +1,54 @@
+/**
+ * @id cpp/microsoft/public/sizeof/const-int-argument
+ * @name Passing a constant integer macro to sizeof
+ * @description The expression passed to sizeof is a macro that expands to an integer constant. A data type was likely intended instead.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ */
+
+import cpp
+import SizeOfTypeUtils
+
+predicate isExprAConstInteger(Expr e, MacroInvocation mi) {
+  exists(Type type |
+    type = e.getExplicitlyConverted().getType() and
+    isTypeDangerousForSizeof(type) and
+    // Special case for wide-char literals when the compiler doesn't recognize wchar_t (i.e. L'\\', L'\0')
+    // Accounting for parenthesis "()" around the value
+    not exists(Macro m | m = mi.getMacro() |
+      m.getBody().toString().regexpMatch("^[\\s(]*L'.+'[\\s)]*$")
+    ) and
+    // Special case for token pasting operator
+    not exists(Macro m | m = mi.getMacro() | m.getBody().toString().regexpMatch("^.*\\s*##\\s*.*$")) and
+    // Special case for multichar literal integers that are exactly 4 character long (i.e. 'val1')
+    not exists(Macro m | m = mi.getMacro() |
+      e.getType().toString() = "int" and
+      m.getBody().toString().regexpMatch("^'.{4}'$")
+    ) and
+    e.isConstant()
+  )
+}
+
+int countMacros(Expr e) { result = count(MacroInvocation mi | mi.getExpr() = e | mi) }
+
+predicate isSizeOfExprOperandMacroInvocationAConstInteger(
+  SizeofExprOperator sizeofExpr, MacroInvocation mi
+) {
+  exists(Expr e |
+    e = mi.getExpr() and
+    e = sizeofExpr.getExprOperand() and
+    isExprAConstInteger(e, mi) and
+    // Special case for FPs that involve an inner macro that resolves to 0 such as _T('\0')
+    not exists(int macroCount | macroCount = countMacros(e) |
+      macroCount > 1 and e.(Literal).getValue().toInt() = 0
+    )
+  )
+}
+
+from SizeofExprOperator sizeofExpr, MacroInvocation mi
+where isSizeOfExprOperandMacroInvocationAConstInteger(sizeofExpr, mi)
+select sizeofExpr,
+  "$@: sizeof of integer macro $@ will always return the size of the underlying integer type.",
+  sizeofExpr, sizeofExpr.getEnclosingFunction().getName(), mi.getMacro(), mi.getMacro().getName()

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/SizeOfConstIntMacroBad.c

@@ -0,0 +1,12 @@
+#define SOMESTRUCT_ERRNO_THAT_MATTERS 0x8000000d
+
+typedef struct {
+	int a;
+	bool b;
+} SOMESTRUCT_THAT_MATTERS;
+
+//bug, the code is using SOMESTRUCT_ERRNO_THAT_MATTERS by mistake instead of SOMESTRUCT_THAT_MATTERS
+if (somedata.length >= sizeof(SOMESTRUCT_ERRNO_THAT_MATTERS)) 
+{
+	/// Do something
+}

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/SizeOfConstIntMacroGood.c

@@ -0,0 +1,11 @@
+#define SOMESTRUCT_ERRNO_THAT_MATTERS 0x8000000d
+
+typedef struct {
+	int a;
+	bool b;
+} SOMESTRUCT_THAT_MATTERS;
+
+if (somedata.length >= sizeof(SOMESTRUCT_THAT_MATTERS)) 
+{
+	/// Do something
+}

cpp/ql/src/Microsoft/Likely Bugs/SizeOfMisuse/SizeOfTypeUtils.qll

@@ -0,0 +1,45 @@
+import cpp
+
+/**
+ * Holds if `type` is a `Type` that typically should not be used for `sizeof` in macros or function return values.
+ */
+predicate isTypeDangerousForSizeof(Type type) {
+  (
+    type instanceof IntegralOrEnumType and
+    // ignore string literals
+    not type instanceof WideCharType and
+    not type instanceof CharType
+  )
+}
+
+/**
+ * Holds if `type` is a `Type` that typically should not be used for `sizeof` in macros or function return values.
+ * This predicate extends the types detected in exchange of precision.
+ * For higher precision, please use `isTypeDangerousForSizeof`
+ */
+predicate isTypeDangerousForSizeofLowPrecision(Type type) {
+  (
+    // UINT8/BYTE are typedefs to char, so we treat them separately.
+    // WCHAR is sometimes a typedef to UINT16, so we treat it separately too.
+    type.getName() = "UINT8"
+    or
+    type.getName() = "BYTE"
+    or
+    not type.getName() = "WCHAR" and
+    exists(Type ut |
+      ut = type.getUnderlyingType() and
+      ut instanceof IntegralOrEnumType and
+      not ut instanceof WideCharType and
+      not ut instanceof CharType
+    )
+  )
+}
+
+/**
+ * Holds if the `Function` return type is dangerous as input for `sizeof`.
+ */
+class FunctionWithTypeDangerousForSizeofLowPrecision extends Function {
+  FunctionWithTypeDangerousForSizeofLowPrecision() {
+    exists(Type type | type = this.getType() | isTypeDangerousForSizeofLowPrecision(type))
+  }
+}

cpp/ql/src/Microsoft/Security/Cryptography/BannedEncryption.qhelp

@@ -0,0 +1,46 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+<overview>
+<p> 
+    Finds explicit uses of symmetric encryption algorithms that are weak, obsolete, or otherwise unapproved.
+</p>
+<p>
+    Encryption algorithms such as DES, (uses keys of 56 bits only), RC2 (uses keys of 128 bits only), and TripleDES (provides at most 112 bits of security) are considered to be weak.
+</p>
+<p>
+    These cryptographic algorithms do not provide as much security assurance as more modern counterparts.
+</p>
+</overview>
+
+<recommendation>
+<p>
+    For Microsoft internal security standards:
+</p>
+<p>
+    For WinCrypt, switch to ALG_SID_AES, ALG_SID_AES_128, ALG_SID_AES_192, or ALG_SID_AES_256.
+</p>
+<p>
+    For BCrypt, switch to AES or any algorithm other than RC2, RC4, DES, DESX, 3DES, 3DES_112. AES_GMAC and AES_CMAC require crypto board review.
+</p>
+</recommendation>
+
+<example>
+<p>Violations:</p>
+
+<sample src="examples/WeakEncryption/WeakEncryption1.cpp" />
+<sample src="examples/WeakEncryption/WeakEncryption3.cpp" />
+
+<p>Solutions:</p>
+
+<sample src="examples/WeakEncryption/WeakEncryption2.cpp" />
+<sample src="examples/WeakEncryption/WeakEncryption4.cpp" />
+</example>
+
+<references>
+<li>Microsoft Docs: <a href="https://learn.microsoft.com/en-us/security/engineering/cryptographic-recommendations">Microsoft SDL Cryptographic Recommendations</a>.</li>
+</references>
+
+</qhelp>

cpp/ql/src/Microsoft/Security/Cryptography/BannedEncryption.ql

@@ -0,0 +1,58 @@
+/**
+ * @name Weak cryptography
+ * @description Finds explicit uses of symmetric encryption algorithms that are weak, obsolete, or otherwise unapproved.
+ * @kind problem
+ * @id cpp/microsoft/public/weak-crypto/banned-encryption-algorithms
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ *       external/cwe/cwe-327
+ */
+
+import cpp
+import CryptoFilters
+import CryptoDataflowCapi
+import CryptoDataflowCng
+import experimental.cryptography.Concepts
+
+predicate isCapiOrCNGBannedAlg(Expr e, string msg) {
+  exists(FunctionCall fc |
+    CapiCryptCreateEncryptionBanned::flow(DataFlow::exprNode(e),
+      DataFlow::exprNode(fc.getArgument(1)))
+    or
+    BCryptOpenAlgorithmProviderBannedEncryption::flow(DataFlow::exprNode(e),
+      DataFlow::exprNode(fc.getArgument(1)))
+  ) and
+  msg =
+    "Call to a cryptographic function with a banned symmetric encryption algorithm: " +
+      e.getValueText()
+}
+
+predicate isGeneralBannedAlg(SymmetricEncryptionAlgorithm alg, Expr confSink, string msg) {
+  // Handle unknown cases in a separate query
+  not alg.getEncryptionName() = unknownAlgorithm() and
+  exists(string resMsg |
+    (
+      not alg.getEncryptionName().matches("AES%") and
+      resMsg = "Use of banned symmetric encryption algorithm: " + alg.getEncryptionName() + "."
+    ) and
+    (
+      if alg.hasConfigurationSink() and alg.configurationSink() != alg
+      then (
+        confSink = alg.configurationSink() and msg = resMsg + " Algorithm used at sink: $@."
+      ) else (
+        confSink = alg and msg = resMsg
+      )
+    )
+  )
+}
+
+from Expr sink, Expr confSink, string msg
+where
+  (
+    isCapiOrCNGBannedAlg(sink, msg) and confSink = sink
+    or
+    isGeneralBannedAlg(sink, confSink, msg)
+  ) and
+  not isSrcSinkFiltered(sink, confSink)
+select sink, msg, confSink, confSink.toString()

cpp/ql/src/Microsoft/Security/Cryptography/BannedModesCAPI.qhelp

@@ -0,0 +1,29 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+<overview>
+<p> Violation - Use of one of the following unsafe encryption modes that is not approved: ECB, OFB, CFB, CTR, CCM, or GCM.</p>
+<p> These modes are vulnerable to attacks and may cause exposure of sensitive information. For example, using <code> ECB </code> to encrypt a plaintext block always produces a same cipher text, so it can easily tell if two encrypted messages are identical. Using approved modes can avoid these unnecessary risks.</p>
+</overview>
+
+<recommendation>
+<p> - Use only approved modes CBC, CTS and XTS.</p>
+</recommendation>
+
+<example>
+<p>Violation:</p>
+
+<sample src="examples/BannedModesCAPI/BannedModesCAPI1.cpp" />
+
+<p>Solution:</p>
+
+<sample src="examples/BannedModesCAPI/BannedModesCAPI2.cpp" />
+</example>
+
+<references>
+<li>Microsoft Docs: <a href="https://learn.microsoft.com/en-us/security/engineering/cryptographic-recommendations">Microsoft SDL Cryptographic Recommendations</a>.</li>
+</references>
+
+</qhelp>

cpp/ql/src/Microsoft/Security/Cryptography/BannedModesCAPI.ql

@@ -0,0 +1,40 @@
+/**
+ * @name Weak cryptography
+ * @description Finds explicit uses of block cipher chaining mode algorithms that are not approved. (CAPI)
+ * @kind problem
+ * @id cpp/microsoft/public/weak-crypto/capi/banned-modes
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ *       external/cwe/cwe-327
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.new.DataFlow
+import CryptoDataflowCapi
+
+module CapiSetBlockCipherConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source.asExpr().isConstant() and
+    // KP_MODE
+    // CRYPT_MODE_CBC   1  - Cipher block chaining    - Microsoft-Only: Only mode allowed by Crypto Board from this list (CBC-MAC)
+    // CRYPT_MODE_ECB   2  - Electronic code book     - Generally not recommended for usage in cryptographic protocols at all
+    // CRYPT_MODE_OFB   3  - Output feedback mode     - Microsoft-Only: Banned, usage requires Crypto Board review
+    // CRYPT_MODE_CFB   4  - Cipher feedback mode     - Microsoft-Only: Banned, usage requires Crypto Board review
+    // CRYPT_MODE_CTS   5  - Ciphertext stealing mode - Microsoft-Only: CTS is approved by Crypto Board, but should probably use CNG and not CAPI
+    source.asExpr().getValue().toInt() != 1
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(CapiCryptCryptSetKeyParamtoKPMODE call | sink.asIndirectExpr() = call.getArgument(2))
+  }
+}
+
+module CapiSetBlockCipherTrace = DataFlow::Global<CapiSetBlockCipherConfiguration>;
+
+from CapiCryptCryptSetKeyParamtoKPMODE call, DataFlow::Node src, DataFlow::Node sink
+where
+  sink.asIndirectExpr() = call.getArgument(2) and
+  CapiSetBlockCipherTrace::flow(src, sink)
+select call,
+  "Call to 'CryptSetKeyParam' function with argument dwParam = KP_MODE is setting up a banned block cipher mode."

cpp/ql/src/Microsoft/Security/Cryptography/BannedModesCNG.qhelp

@@ -0,0 +1,31 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+<overview>
+<p> Violation - Use of one of the following unsafe encryption modes that is not approved: ECB, OFB, CFB, CTR, CCM, or GCM.</p>
+<p> These modes are vulnerable to attacks and may cause exposure of sensitive information. For example, using <code> ECB </code> to encrypt a plaintext block always produces a same cipher text, so it can easily tell if two encrypted messages are identical. Using approved modes can avoid these unnecessary risks.</p>
+</overview>
+
+<recommendation>
+<p> - Use only approved modes CBC, CTS and XTS.</p>
+</recommendation>
+
+<example>
+<p>Violation:</p>
+
+<sample src="examples/BannedModesCNG/BannedModesCNG1.cpp" />
+
+<p>Solution:</p>
+
+<sample src="examples/BannedModesCNG/BannedModesCNG2.cpp" />
+</example>
+
+
+<references>
+<li>Microsoft Docs: <a href="https://learn.microsoft.com/en-us/security/engineering/cryptographic-recommendations">Microsoft SDL Cryptographic Recommendations</a>.</li>
+</references>
+
+
+</qhelp>

cpp/ql/src/Microsoft/Security/Cryptography/BannedModesCNG.ql

@@ -0,0 +1,23 @@
+/**
+ * @name Weak cryptography
+ * @description Finds explicit uses of block cipher chaining mode algorithms that are not approved. (CNG)
+ * @kind problem
+ * @id cpp/microsoft/public/weak-crypto/cng/banned-modes
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ *       external/cwe/cwe-327
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.new.DataFlow
+import CryptoDataflowCng
+
+from CngBCryptSetPropertyParamtoKChainingMode call, DataFlow::Node src, DataFlow::Node sink
+where
+  sink.asIndirectArgument() = call.getArgument(2) and
+  CngBCryptSetPropertyChainingBannedModeIndirectParameter::flow(src, sink)
+  or
+  sink.asExpr() = call.getArgument(2) and CngBCryptSetPropertyChainingBannedMode::flow(src, sink)
+select call,
+  "Call to 'BCryptSetProperty' function with argument pszProperty = \"ChainingMode\" is setting up a banned block cipher mode."

cpp/ql/src/Microsoft/Security/Cryptography/CryptoDataflowCapi.qll

@@ -0,0 +1,97 @@
+/**
+ * Provides classes and predicates for identifying expressions that are use Crypto API (CAPI).
+ */
+
+import cpp
+private import semmle.code.cpp.dataflow.new.DataFlow
+
+/**
+ * Dataflow that detects a call to CryptSetKeyParam dwParam = KP_MODE (CAPI)
+ */
+module CapiCryptCryptSetKeyParamtoKPMODEConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source.asExpr().getValue().toInt() = 4 // KP_MODE
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(FunctionCall call |
+      // CryptSetKeyParam 2nd argument specifies the key parameter to set
+      sink.asExpr() = call.getArgument(1) and
+      call.getTarget().hasGlobalName("CryptSetKeyParam")
+    )
+  }
+}
+
+module CapiCryptCryptSetKeyParamtoKPMODE =
+  DataFlow::Global<CapiCryptCryptSetKeyParamtoKPMODEConfiguration>;
+
+/**
+ * A function call to CryptSetKeyParam with dwParam = KP_MODE (CAPI)
+ */
+class CapiCryptCryptSetKeyParamtoKPMODE extends FunctionCall {
+  CapiCryptCryptSetKeyParamtoKPMODE() {
+    exists(Expr var |
+      CapiCryptCryptSetKeyParamtoKPMODE::flow(DataFlow::exprNode(var),
+        DataFlow::exprNode(this.getArgument(1)))
+    )
+  }
+}
+
+// CAPI-specific DataFlow configuration
+module CapiCryptCreateHashBannedConfiguration implements DataFlow::ConfigSig {
+  // This mechnism will verify for approved set of values to call, rejecting anythign that is not in the list.
+  // NOTE: This mechanism is not guaranteed to work with CSPs that do not use the same algorithms defined in Wincrypt.h
+  //
+  predicate isSource(DataFlow::Node source) {
+    // Verify if source matched the mask for CAPI ALG_CLASS_HASH == 32768
+    source.asExpr().getValue().toInt().bitShiftRight(13) = 4 and
+    // The following hash algorithms are safe to use, anything else is considered banned
+    not (
+      source.asExpr().getValue().toInt().bitXor(32768) = 12 or // ALG_SID_SHA_256
+      source.asExpr().getValue().toInt().bitXor(32768) = 13 or // ALG_SID_SHA_384
+      source.asExpr().getValue().toInt().bitXor(32768) = 14 // ALG_SID_SHA_512
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(FunctionCall call |
+      // CryptCreateHash 2nd argument specifies the hash algorithm to be used.
+      sink.asExpr() = call.getArgument(1) and
+      call.getTarget().hasGlobalName("CryptCreateHash")
+    )
+  }
+}
+
+module CapiCryptCreateHashBanned = DataFlow::Global<CapiCryptCreateHashBannedConfiguration>;
+
+// CAPI-specific DataFlow configuration
+module CapiCryptCreateEncryptionBannedConfiguration implements DataFlow::ConfigSig {
+  // This mechanism will verify for approved set of values to call, rejecting anything that is not in the list.
+  // NOTE: This mechanism is not guaranteed to work with CSPs that do not use the same algorithms defined in Wincrypt.h
+  //
+  predicate isSource(DataFlow::Node source) {
+    // Verify if source matched the mask for CAPI ALG_CLASS_DATA_ENCRYPT == 24576
+    source.asExpr().getValue().toInt().bitShiftRight(13) = 3 and
+    // The following algorithms are safe to use, anything else is considered banned
+    not (
+      source.asExpr().getValue().toInt().bitXor(26112) = 14 or // ALG_SID_AES_128
+      source.asExpr().getValue().toInt().bitXor(26112) = 15 or // ALG_SID_AES_192
+      source.asExpr().getValue().toInt().bitXor(26112) = 16 or // ALG_SID_AES_256
+      source.asExpr().getValue().toInt().bitXor(26112) = 17 // ALG_SID_AES
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(FunctionCall call |
+      // CryptGenKey or CryptDeriveKey 2nd argument specifies the hash algorithm to be used.
+      sink.asExpr() = call.getArgument(1) and
+      (
+        call.getTarget().hasGlobalName("CryptGenKey") or
+        call.getTarget().hasGlobalName("CryptDeriveKey")
+      )
+    )
+  }
+}
+
+module CapiCryptCreateEncryptionBanned =
+  DataFlow::Global<CapiCryptCreateEncryptionBannedConfiguration>;

cpp/ql/src/Microsoft/Security/Cryptography/CryptoDataflowCng.qll

@@ -0,0 +1,137 @@
+/**
+ * Provides classes and predicates for identifying expressions that are use crypto API Next Generation (CNG).
+ */
+
+import cpp
+private import semmle.code.cpp.dataflow.new.DataFlow
+
+/**
+ * Dataflow that detects a call to BCryptSetProperty pszProperty = ChainingMode (CNG)
+ */
+module CngBCryptSetPropertyParamtoKChainingModeConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source.asExpr().getValue().toString().matches("ChainingMode")
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(FunctionCall call |
+      // BCryptSetProperty 2nd argument specifies the key parameter to set
+      sink.asExpr() = call.getArgument(1) and
+      call.getTarget().hasGlobalName("BCryptSetProperty")
+    )
+  }
+}
+
+module CngBCryptSetPropertyParamtoKChainingMode =
+  DataFlow::Global<CngBCryptSetPropertyParamtoKChainingModeConfiguration>;
+
+/**
+ * A function call to BCryptSetProperty pszProperty = ChainingMode (CNG)
+ */
+class CngBCryptSetPropertyParamtoKChainingMode extends FunctionCall {
+  CngBCryptSetPropertyParamtoKChainingMode() {
+    exists(Expr var |
+      CngBCryptSetPropertyParamtoKChainingMode::flow(DataFlow::exprNode(var),
+        DataFlow::exprNode(this.getArgument(1)))
+    )
+  }
+}
+
+predicate isChaniningModeCbc(DataFlow::Node source) {
+  // Verify if algorithm is in the approved list.
+  exists(string s | s = source.asExpr().getValue().toString() |
+    s.regexpMatch("ChainingMode[A-Za-z0-9/]+") and
+    // Property Strings
+    // BCRYPT_CHAIN_MODE_NA    L"ChainingModeN/A"  - The algorithm does not support chaining
+    // BCRYPT_CHAIN_MODE_CBC   L"ChainingModeCBC"  - Microsoft-Only: Only mode allowed by Crypto Board from this list (CBC-MAC)
+    // BCRYPT_CHAIN_MODE_ECB   L"ChainingModeECB"  - Generally not recommended for usage in cryptographic protocols at all
+    // BCRYPT_CHAIN_MODE_CFB   L"ChainingModeCFB"  - Microsoft-Only: Banned, usage requires Crypto Board review
+    // BCRYPT_CHAIN_MODE_CCM   L"ChainingModeCCM"  - Microsoft-Only: Banned, usage requires Crypto Board review
+    // BCRYPT_CHAIN_MODE_GCM   L"ChainingModeGCM"  - Microsoft-Only: Only for TLS, other usage requires Crypto Board review
+    not s.matches("ChainingModeCBC")
+  )
+}
+
+module CngBCryptSetPropertyChainingBannedModeConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { isChaniningModeCbc(source) }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(CngBCryptSetPropertyParamtoKChainingMode call |
+      // BCryptOpenAlgorithmProvider 3rd argument sets the chaining mode value
+      sink.asExpr() = call.getArgument(2)
+    )
+  }
+}
+
+module CngBCryptSetPropertyChainingBannedMode =
+  DataFlow::Global<CngBCryptSetPropertyChainingBannedModeConfiguration>;
+
+module CngBCryptSetPropertyChainingBannedModeIndirectParameterConfiguration implements
+  DataFlow::ConfigSig
+{
+  predicate isSource(DataFlow::Node source) { isChaniningModeCbc(source) }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(CngBCryptSetPropertyParamtoKChainingMode call |
+      // CryptSetKeyParam 3rd argument specifies the mode (KP_MODE)
+      sink.asIndirectExpr() = call.getArgument(2)
+    )
+  }
+}
+
+module CngBCryptSetPropertyChainingBannedModeIndirectParameter =
+  DataFlow::Global<CngBCryptSetPropertyChainingBannedModeIndirectParameterConfiguration>;
+
+// CNG-specific DataFlow configuration
+module BCryptOpenAlgorithmProviderBannedHashConfiguration implements DataFlow::ConfigSig {
+  // NOTE: Unlike the CAPI scenario, CNG will use this method to load and initialize
+  //       a cryptographic provider for any type of algorithm,not only hash.
+  //       Therefore, we have to take a banned-list instead of approved list approach.
+  //
+  predicate isSource(DataFlow::Node source) {
+    // Verify if algorithm is marked as banned.
+    source.asExpr().getValue().toString().matches("MD_")
+    or
+    source.asExpr().getValue().toString().matches("SHA1")
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(FunctionCall call |
+      // BCryptOpenAlgorithmProvider 2nd argument specifies the algorithm to be used
+      sink.asExpr() = call.getArgument(1) and
+      call.getTarget().hasGlobalName("BCryptOpenAlgorithmProvider")
+    )
+  }
+}
+
+module BCryptOpenAlgorithmProviderBannedHash =
+  DataFlow::Global<BCryptOpenAlgorithmProviderBannedHashConfiguration>;
+
+// CNG-specific DataFlow configuration
+module BCryptOpenAlgorithmProviderBannedEncryptionConfiguration implements DataFlow::ConfigSig {
+  // NOTE: Unlike the CAPI scenario, CNG will use this method to load and initialize
+  //       a cryptographic provider for any type of algorithm,not only encryption.
+  //       Therefore, we have to take a banned-list instead of approved list approach.
+  //
+  predicate isSource(DataFlow::Node source) {
+    // Verify if algorithm is marked as banned.
+    source.asExpr().getValue().toString().matches("RC_") or
+    source.asExpr().getValue().toString().matches("DES") or
+    source.asExpr().getValue().toString().matches("DESX") or
+    source.asExpr().getValue().toString().matches("3DES") or
+    source.asExpr().getValue().toString().matches("3DES_112") or
+    source.asExpr().getValue().toString().matches("AES_GMAC") or // Microsoft Only: Requires Cryptoboard review
+    source.asExpr().getValue().toString().matches("AES_CMAC") // Microsoft Only: Requires Cryptoboard review
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(FunctionCall call |
+      // BCryptOpenAlgorithmProvider 2nd argument specifies the algorithm to be used
+      sink.asExpr() = call.getArgument(1) and
+      call.getTarget().hasGlobalName("BCryptOpenAlgorithmProvider")
+    )
+  }
+}
+
+module BCryptOpenAlgorithmProviderBannedEncryption =
+  DataFlow::Global<BCryptOpenAlgorithmProviderBannedEncryptionConfiguration>;

cpp/ql/src/Microsoft/Security/Cryptography/CryptoFilters.qll

@@ -0,0 +1,45 @@
+import cpp
+
+/**
+ * Determines if an element should be filtered (ignored)
+ * from any result set.
+ *
+ * The current strategy is to determine if the element
+ * resides in a path that appears to be a library (in particular openssl).
+ *
+ * It is therefore important that the element being examined represents
+ * a use or configuration of cryptography in the user code.
+ * E.g., if a global variable were defined in an OpenSSL library
+ * representing a bad/vuln algorithm, and this global were assessed
+ * it would appear to be ignorable, as it exists in a a filtered library.
+ * The use of that global must be examined with this filter.
+ *
+ * ASSUMPTION/CAVEAT: note if an openssl library wraps a dangerous crypo use
+ *    this filter approach will ignore the wrapper call, unless it is also flagged
+ *    as dangerous. e.g., SomeWraper(){ ... <md5 use> ...}
+ *    The wrapper if defined in openssl would result in ignoring
+ *    the use of MD5 internally, since it's use is entirely in openssl.
+ *
+ *    TODO: these caveats need to be reassessed in the future.
+ */
+predicate isUseFiltered(Element e) {
+  e.getFile().getAbsolutePath().toLowerCase().matches("%openssl%")
+}
+
+/**
+ * Filtered only if both src and sink are considered filtered.
+ *
+ * This approach is meant to partially address some of the implications of
+ * `isUseFiltered`. Specifically, if an algorithm is specified by a user
+ * and some how passes to a user inside openssl, then this filter
+ * would not ignore that the user was specifying the use of something dangerous.
+ *
+ * e.g., if a wrapper in openssl existed of the form SomeWrapper(string alg, ...){ ... <operation using alg> ...}
+ * and the user did something like SomeWrapper("MD5", ...), this would not be ignored.
+ *
+ * The source in the above example would the algorithm, and the sink is the configuration sink
+ * of the algorithm.
+ */
+predicate isSrcSinkFiltered(Element src, Element sink) {
+  isUseFiltered(src) and isUseFiltered(sink)
+}

cpp/ql/src/Microsoft/Security/Cryptography/HardcodedIVCNG.qhelp

@@ -0,0 +1,23 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>An initialization vector (IV) is an input to a cryptographic primitive being used to provide the initial state. The IV is typically required to be random or pseudorandom (randomized scheme), but sometimes an IV only needs to be unpredictable or unique (stateful scheme).</p>
+    <p>Randomization is crucial for some encryption schemes to achieve semantic security, a property whereby repeated usage of the scheme under the same key does not allow an attacker to infer relationships between (potentially similar) segments of the encrypted message.</p>
+  </overview>
+
+  <recommendation>
+    <p>All symmetric block ciphers must also be used with an appropriate initialization vector (IV) according to the mode of operation being used.</p>
+    <p>If using a randomized scheme such as CBC, it is recommended to use cryptographically secure pseudorandom number generator such as <code>BCryptGenRandom</code>.</p>
+  </recommendation>
+  
+  <references>
+    <li>
+      <a href="https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptencrypt">BCryptEncrypt function (bcrypt.h)</a>
+      <a href="https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptgenrandom">BCryptGenRandom function (bcrypt.h)</a>
+      <a href="https://en.wikipedia.org/wiki/Initialization_vector">Initialization vector (Wikipedia)</a>
+    </li>
+  </references>
+
+</qhelp>

cpp/ql/src/Microsoft/Security/Cryptography/HardcodedIVCNG.ql

@@ -0,0 +1,58 @@
+/**
+ * @name Weak cryptography
+ * @description Finds usage of a static (hardcoded) IV. (CNG)
+ * @kind problem
+ * @id cpp/microsoft/public/weak-crypto/cng/hardcoded-iv
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ *       external/cwe/cwe-327
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.new.DataFlow
+
+/**
+ * Gets const element of `ArrayAggregateLiteral`.
+ */
+Expr getConstElement(ArrayAggregateLiteral lit) {
+  exists(int n |
+    result = lit.getElementExpr(n, _) and
+    result.isConstant()
+  )
+}
+
+/**
+ * Gets the last element in an `ArrayAggregateLiteral`.
+ */
+Expr getLastElement(ArrayAggregateLiteral lit) {
+  exists(int n |
+    result = lit.getElementExpr(n, _) and
+    not exists(lit.getElementExpr(n + 1, _))
+  )
+}
+
+module CngBCryptEncryptHardcodedIVConfiguration implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    exists(AggregateLiteral lit |
+      getLastElement(lit) = source.asDefinition() and
+      exists(getConstElement(lit))
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(FunctionCall call |
+      // BCryptEncrypt 5h argument specifies the IV
+      sink.asIndirectExpr() = call.getArgument(4) and
+      call.getTarget().hasGlobalName("BCryptEncrypt")
+    )
+  }
+}
+
+module Flow = DataFlow::Global<CngBCryptEncryptHardcodedIVConfiguration>;
+
+from DataFlow::Node sl, DataFlow::Node fc, AggregateLiteral lit
+where
+  Flow::flow(sl, fc) and
+  getLastElement(lit) = sl.asDefinition()
+select lit, "Calling BCryptEncrypt with a hard-coded IV on function "

cpp/ql/src/Microsoft/Security/Cryptography/WeakKDFBannedHashAlgorithm.qhelp

@@ -0,0 +1,14 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Use of KDF algorithm BCryptDeriveKeyPBKDF2 uses insecure hash from BCryptOpenAlgorithmProvider.</p>
+</overview>
+
+<recommendation>
+<p>Use SHA 256, 384, or 512.</p>
+</recommendation>
+
+</qhelp>

cpp/ql/src/Microsoft/Security/Cryptography/WeakKDFBannedHashAlgorithm.ql

@@ -0,0 +1,85 @@
+/**
+ * @name KDF may only use SHA256/384/512 in generating a key.
+ * @description KDF may only use SHA256/384/512 in generating a key.
+ * @kind problem
+ * @id cpp/microsoft/public/kdf-insecure-hash
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.new.DataFlow
+
+module BannedHashAlgorithmConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    // Verify if algorithm is marked as banned.
+    not source.asExpr().getValue().toString().matches("SHA256") and
+    not source.asExpr().getValue().toString().matches("SHA384") and
+    not source.asExpr().getValue().toString().matches("SHA512")
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(FunctionCall call |
+      // Argument 1 (0-based) specified the algorithm ID.
+      // NTSTATUS BCryptOpenAlgorithmProvider(
+      //     [out] BCRYPT_ALG_HANDLE *phAlgorithm,
+      //     [in]  LPCWSTR           pszAlgId,
+      //     [in]  LPCWSTR           pszImplementation,
+      //     [in]  ULONG             dwFlags
+      //   );
+      sink.asExpr() = call.getArgument(1) and
+      call.getTarget().hasGlobalName("BCryptOpenAlgorithmProvider")
+    )
+  }
+}
+
+module BannedHashAlgorithmTrace = DataFlow::Global<BannedHashAlgorithmConfig>;
+
+module BCRYPT_ALG_HANDLE_Config implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    exists(FunctionCall call |
+      // Argument 0 (0-based) specified the algorithm handle
+      // NTSTATUS BCryptOpenAlgorithmProvider(
+      //     [out] BCRYPT_ALG_HANDLE *phAlgorithm,
+      //     [in]  LPCWSTR           pszAlgId,
+      //     [in]  LPCWSTR           pszImplementation,
+      //     [in]  ULONG             dwFlags
+      //   );
+      source.asDefiningArgument() = call.getArgument(0) and
+      call.getTarget().hasGlobalName("BCryptOpenAlgorithmProvider")
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    // Algorithm handle is the 0th (0-based) argument of the call
+    //    NTSTATUS BCryptDeriveKeyPBKDF2(
+    //      [in]           BCRYPT_ALG_HANDLE hPrf,
+    //      [in, optional] PUCHAR            pbPassword,
+    //      [in]           ULONG             cbPassword,
+    //      [in, optional] PUCHAR            pbSalt,
+    //      [in]           ULONG             cbSalt,
+    //      [in]           ULONGLONG         cIterations,
+    //      [out]          PUCHAR            pbDerivedKey,
+    //      [in]           ULONG             cbDerivedKey,
+    //      [in]           ULONG             dwFlags
+    // );
+    exists(Call c | c.getTarget().getName() = "BCryptDeriveKeyPBKDF2" |
+      c.getArgument(0) = sink.asExpr()
+    )
+  }
+}
+
+module BCRYPT_ALG_HANDLE_Trace = DataFlow::Global<BCRYPT_ALG_HANDLE_Config>;
+
+from DataFlow::Node src1, DataFlow::Node src2, DataFlow::Node sink1, DataFlow::Node sink2
+where
+  BannedHashAlgorithmTrace::flow(src1, sink1) and
+  exists(Call c |
+    c.getAnArgument() = sink1.asExpr() and src2.asDefiningArgument() = c.getAnArgument()
+  |
+    BCRYPT_ALG_HANDLE_Trace::flow(src2, sink2)
+  )
+select sink2.asExpr(),
+  "BCRYPT_ALG_HANDLE is passed to this to KDF derived from insecure hashing function $@. Must use SHA256 or higher.",
+  src1.asExpr(), src1.asExpr().getValue()

cpp/ql/src/Microsoft/Security/Cryptography/WeakKDFLowIterationCount.qhelp

@@ -0,0 +1,14 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Use of KDF algorithm BCryptDeriveKeyPBKDF2 uses low iteration count (less than 100k).</p>
+</overview>
+
+<recommendation>
+<p>Use a minimum of 100,000 for iteration count.</p>
+</recommendation>
+
+</qhelp>

cpp/ql/src/Microsoft/Security/Cryptography/WeakKDFLowIterationCount.ql

@@ -0,0 +1,51 @@
+/**
+ * @name Use iteration count at least 100k to prevent brute force attacks
+ * @description When deriving cryptographic keys from user-provided inputs such as password, use sufficient iteration count (at least 100k).
+ * This query traces constants of <100k to the iteration count parameter of CNG's BCryptDeriveKeyPBKDF2.
+ * This query traces constants of less than the min length to the target parameter.
+ * NOTE: if the constant is modified, or if a non-constant gets to the iteration count, this query will not flag these cases.
+ *      The rationale currently is that this query is meant to validate common uses of key derivation.
+ *      Non-common uses (modifying the iteration count somehow or getting the count from outside sources) are assumed to be intentional.
+ * @kind problem
+ * @id cpp/microsoft/public/kdf-low-iteration-count
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.new.DataFlow
+
+module IterationCountDataFlowConfig implements DataFlow::ConfigSig {
+  /**
+   * Defines the source for iteration count when it's coming from a fixed value
+   * Any expression that has an assigned value < 100000 could be a source.
+   */
+  predicate isSource(DataFlow::Node src) { src.asExpr().getValue().toInt() < 100000 }
+
+  predicate isSink(DataFlow::Node sink) {
+    // iterations count is the 5th (0-based) argument of the call
+    //    NTSTATUS BCryptDeriveKeyPBKDF2(
+    //      [in]           BCRYPT_ALG_HANDLE hPrf,
+    //      [in, optional] PUCHAR            pbPassword,
+    //      [in]           ULONG             cbPassword,
+    //      [in, optional] PUCHAR            pbSalt,
+    //      [in]           ULONG             cbSalt,
+    //      [in]           ULONGLONG         cIterations,
+    //      [out]          PUCHAR            pbDerivedKey,
+    //      [in]           ULONG             cbDerivedKey,
+    //      [in]           ULONG             dwFlags
+    // );
+    exists(Call c | c.getTarget().getName() = "BCryptDeriveKeyPBKDF2" |
+      c.getArgument(5) = sink.asExpr()
+    )
+  }
+}
+
+module IterationCountDataFlow = DataFlow::Global<IterationCountDataFlowConfig>;
+
+from DataFlow::Node src, DataFlow::Node sink
+where IterationCountDataFlow::flow(src, sink)
+select sink.asExpr(),
+  "Iteration count $@ is passed to this to KDF. Use at least 100000 iterations when deriving cryptographic key from password.",
+  src, src.asExpr().getValue()

cpp/ql/src/Microsoft/Security/Cryptography/WeakKDFSmallKeyLength.qhelp

@@ -0,0 +1,14 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Use of KDF algorithm BCryptDeriveKeyPBKDF2 uses small key size (less than 16 bytes).</p>
+</overview>
+
+<recommendation>
+<p>Use a minimum of 16 bytes for key size.</p>
+</recommendation>
+
+</qhelp>

cpp/ql/src/Microsoft/Security/Cryptography/WeakKDFSmallKeyLength.ql

@@ -0,0 +1,46 @@
+/**
+ * @name Small KDF derived key length.
+ * @description KDF derived keys should be a minimum of 128 bits (16 bytes).
+ * This query traces constants of less than the min length to the target parameter.
+ * NOTE: if the constant is modified, or if a non-constant gets to the target, this query will not flag these cases.
+ *      The rationale currently is that this query is meant to validate common uses of key derivation.
+ *      Non-common uses (modifying the values somehow or getting the count from outside sources) are assumed to be intentional.
+ * @kind problem
+ * @id cpp/microsoft/public/kdf-small-key-size
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.new.DataFlow
+
+module KeyLenConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src.asExpr().getValue().toInt() < 16 }
+
+  predicate isSink(DataFlow::Node sink) {
+    // Key length size is the 7th (0-based) argument of the call
+    //    NTSTATUS BCryptDeriveKeyPBKDF2(
+    //      [in]           BCRYPT_ALG_HANDLE hPrf,
+    //      [in, optional] PUCHAR            pbPassword,
+    //      [in]           ULONG             cbPassword,
+    //      [in, optional] PUCHAR            pbSalt,
+    //      [in]           ULONG             cbSalt,
+    //      [in]           ULONGLONG         cIterations,
+    //      [out]          PUCHAR            pbDerivedKey,
+    //      [in]           ULONG             cbDerivedKey,
+    //      [in]           ULONG             dwFlags
+    // );
+    exists(Call c | c.getTarget().getName() = "BCryptDeriveKeyPBKDF2" |
+      c.getArgument(7) = sink.asExpr()
+    )
+  }
+}
+
+module KeyLenTrace = DataFlow::Global<KeyLenConfig>;
+
+from DataFlow::Node src, DataFlow::Node sink
+where KeyLenTrace::flow(src, sink)
+select sink.asExpr(),
+  "Key size $@ is passed to this to KDF. Use at least 16 bytes for key length when deriving cryptographic key from password.",
+  src.asExpr(), src.asExpr().getValue()

cpp/ql/src/Microsoft/Security/Cryptography/WeakKDFSmallSaltSize.qhelp

@@ -0,0 +1,15 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Use of KDF algorithm BCryptDeriveKeyPBKDF2 uses small salt size (less than 16 bytes).</p>
+</overview>
+
+<recommendation>
+<p>Use a minimum of 16 bytes for salt size.</p>
+</recommendation>
+
+
+</qhelp>

cpp/ql/src/Microsoft/Security/Cryptography/WeakKDFSmallSaltSize.ql

@@ -0,0 +1,46 @@
+/**
+ * @name Small KDF salt length.
+ * @description KDF salts should be a minimum of 128 bits (16 bytes).
+ * This query traces constants of less than the min length to the target parameter.
+ * NOTE: if the constant is modified, or if a non-constant gets to the target, this query will not flag these cases.
+ *      The rationale currently is that this query is meant to validate common uses of key derivation.
+ *      Non-common uses (modifying the values somehow or getting the count from outside sources) are assumed to be intentional.
+ * @kind problem
+ * @id cpp/microsoft/public/kdf-small-salt-size
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.new.DataFlow
+
+module SaltLenConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src.asExpr().getValue().toInt() < 16 }
+
+  predicate isSink(DataFlow::Node sink) {
+    // Key length size is the 7th (0-based) argument of the call
+    //    NTSTATUS BCryptDeriveKeyPBKDF2(
+    //      [in]           BCRYPT_ALG_HANDLE hPrf,
+    //      [in, optional] PUCHAR            pbPassword,
+    //      [in]           ULONG             cbPassword,
+    //      [in, optional] PUCHAR            pbSalt,
+    //      [in]           ULONG             cbSalt,
+    //      [in]           ULONGLONG         cIterations,
+    //      [out]          PUCHAR            pbDerivedKey,
+    //      [in]           ULONG             cbDerivedKey,
+    //      [in]           ULONG             dwFlags
+    // );
+    exists(Call c | c.getTarget().getName() = "BCryptDeriveKeyPBKDF2" |
+      c.getArgument(4) = sink.asExpr()
+    )
+  }
+}
+
+module SaltLenTrace = DataFlow::Global<SaltLenConfig>;
+
+from DataFlow::Node src, DataFlow::Node sink
+where SaltLenTrace::flow(src, sink)
+select sink.asExpr(),
+  "Salt size $@ is passed to this to KDF. Use at least 16 bytes for salt size when deriving cryptographic key from password.",
+  src, src.asExpr().getValue()

cpp/ql/src/Microsoft/Security/Cryptography/examples/BannedModesCAPI/BannedModesCAPI1.cpp

@@ -0,0 +1,11 @@
+#include <windows.h> 
+#include <stdio.h>
+#include <wincrypt.h>
+
+int main(){
+    DWORD ivLen;
+    HCRYPTKEY hKey; 
+
+    //BAD
+    CryptGetKeyParam(hKey, CRYPT_MODE_ECB, NULL, &ivLen, 0);
+}

cpp/ql/src/Microsoft/Security/Cryptography/examples/BannedModesCAPI/BannedModesCAPI2.cpp

@@ -0,0 +1,11 @@
+#include <windows.h> 
+#include <stdio.h>
+#include <wincrypt.h>
+
+int main(){
+    DWORD ivLen;
+    HCRYPTKEY hKey; 
+
+    //OKAY
+    CryptGetKeyParam(hKey, CRYPT_MODE_CBC, NULL, &ivLen, 0);
+}

cpp/ql/src/Microsoft/Security/Cryptography/examples/BannedModesCNG/BannedModesCNG1.cpp

@@ -0,0 +1,14 @@
+#include <windows.h> 
+#include <stdio.h>
+#include <bcrypt.h>
+
+int main(){
+    BCRYPT_ALG_HANDLE aes;
+
+    //BAD
+    status = BCryptSetProperty(aes, 
+                BCRYPT_CHAINING_MODE, 
+                (PBYTE)BCRYPT_CHAIN_MODE_ECB,
+                sizeof(BCRYPT_CHAIN_MODE_ECB),
+                0);
+}

cpp/ql/src/Microsoft/Security/Cryptography/examples/BannedModesCNG/BannedModesCNG2.cpp

@@ -0,0 +1,14 @@
+#include <windows.h> 
+#include <stdio.h>
+#include <bcrypt.h>
+
+int main(){
+    BCRYPT_ALG_HANDLE aes;
+
+    //OKAY
+    status = BCryptSetProperty(aes, 
+                BCRYPT_CHAINING_MODE, 
+                (PBYTE)BCRYPT_CHAIN_MODE_CBC,
+                sizeof(BCRYPT_CHAIN_MODE_CBC),
+                0);
+}

cpp/ql/src/Microsoft/Security/Cryptography/examples/WeakEncryption/WeakEncryption1.cpp

@@ -0,0 +1,14 @@
+#include <windows.h> 
+#include <stdio.h>
+#include <wincrypt.h>
+
+int main(){
+    HCRYPTPROV hCryptProv;
+    HCRYPTKEY hKey; 
+
+    //BAD
+    if(CryptGenKey( hCryptProv, CALG_DES_128, KEYLENGTH | CRYPT_EXPORTABLE, &hKey))
+    {
+        printf("A session key has been created.\n");
+    }
+}

cpp/ql/src/Microsoft/Security/Cryptography/examples/WeakEncryption/WeakEncryption2.cpp

@@ -0,0 +1,14 @@
+#include <windows.h> 
+#include <stdio.h>
+#include <wincrypt.h>
+
+int main(){
+    HCRYPTPROV hCryptProv;
+    HCRYPTKEY hKey; 
+
+    //OKAY
+    if(CryptGenKey( hCryptProv, CALG_AES_128, KEYLENGTH | CRYPT_EXPORTABLE, &hKey))
+    {
+        printf("A session key has been created.\n");
+    }
+}