Compare commits

..

57 Commits

Author SHA1 Message Date
Paolo Tranquilli
93e05319cf CI: fix some actions alerts 2025-08-08 17:51:36 +02:00
Chuan-kai Lin
72563ec5a4 Merge pull request #20080 from d10c/d10c/diff-informed-phase-3-ruby
Ruby: Diff-informed queries: phase 3 (non-trivial locations)
2025-08-07 07:37:40 -07:00
Anders Schack-Mulligen
3674966946 Merge pull request #20121 from aschackmull/guards/wrapperguard
Guards: Improve support for wrapped guards
2025-08-07 15:41:04 +02:00
Anders Schack-Mulligen
2909def9b6 Guards: Rename predicate. 2025-08-07 14:51:50 +02:00
Anders Schack-Mulligen
b51c0e7cb6 Java: Add change note. 2025-08-07 14:51:50 +02:00
Anders Schack-Mulligen
d9cfe14729 Java: Accept qltest change. 2025-08-07 14:51:49 +02:00
Anders Schack-Mulligen
a40ae3a11a Guards: Slight join-order improvement. 2025-08-07 14:51:49 +02:00
Anders Schack-Mulligen
ec513ead0d Guards: Add support for extending BarrierGuards with wrapped invocations. 2025-08-07 14:51:48 +02:00
Anders Schack-Mulligen
f90b6ab005 Guards: Add support for wrappers that may throw exceptions. 2025-08-07 14:51:48 +02:00
Anders Schack-Mulligen
b156bd5ce2 Guards: Rename predicate. 2025-08-07 14:51:48 +02:00
Anders Schack-Mulligen
0c31a80f3c Guards: Generalise wrapper guards. 2025-08-07 14:51:47 +02:00
Anders Schack-Mulligen
6e52df1639 Guards: Rename module. 2025-08-07 14:51:47 +02:00
Anders Schack-Mulligen
1bdaa2420d Java: Simplify Guards instantiation a bit. 2025-08-07 14:51:46 +02:00
Anders Schack-Mulligen
3aaf48de11 Guards: Remove CustomGuard nesting in Guards instantiation. 2025-08-07 14:51:46 +02:00
Tom Hvitved
dfe4401f13 Merge pull request #20169 from hvitved/javascript/legacy-summary-steps
JS: Generate legacy flow steps for all flow summaries
2025-08-06 18:52:39 +02:00
Tom Hvitved
ed3a33fdc6 Merge pull request #20177 from hvitved/rust/type-inference-where
Rust: Improve handling of where clauses in type inference and path resolution
2025-08-06 15:52:56 +02:00
Geoffrey White
d215ea16da Merge pull request #19802 from geoffw0/sqlx
Rust: Update SqlxQuery, SqlxExecute to use getCanonicalPath
2025-08-06 14:52:03 +01:00
Simon Friis Vindum
b50a76693a Rust: Handle multiple type bounds for the same type parameter in getTypeBound 2025-08-06 11:15:28 +02:00
Simon Friis Vindum
0cfb22ff3f Rust: Add example with multiple where clause items for the same type parameter 2025-08-06 11:15:24 +02:00
Simon Friis Vindum
b302f3f98f Rust: Improve handling of where clauses in type inference and path resolution 2025-08-06 11:08:18 +02:00
Simon Friis Vindum
766083290c Rust: Add tests with where clause 2025-08-06 11:08:13 +02:00
Tom Hvitved
d201ce1705 Merge pull request #20155 from paldepind/rust/type-inference-certain
Rust: Add predicate for certain type information
2025-08-06 10:55:34 +02:00
Tom Hvitved
1f15fc8a35 Merge pull request #20173 from hvitved/rust/type-mention-remove-restriction
Rust: Remove restriction in `PathTypeMention`
2025-08-06 10:13:23 +02:00
Tom Hvitved
eb3c054b0f JS: Generate legacy flow steps for all flow summaries 2025-08-06 09:38:49 +02:00
Geoffrey White
0d4f8765a6 Merge pull request #20167 from geoffw0/mdlcleanup
Rust: Clean up some odds and ends
2025-08-05 19:25:46 +01:00
Tom Hvitved
a396f9345e Rust: Remove restriction in PathTypeMention 2025-08-05 15:05:43 +02:00
Anders Schack-Mulligen
1823355fae Merge pull request #20171 from aschackmull/java/nullness-fn
Java: document nullness false negative as qltest
2025-08-05 14:17:09 +02:00
Anders Schack-Mulligen
94274288d3 Merge pull request #20127 from aschackmull/java/joinorder3
Java: Improve a couple of join-orders
2025-08-05 14:15:42 +02:00
Anders Schack-Mulligen
c59d20a668 Merge pull request #20163 from aschackmull/java/postdom-normal
Java: Assume normal termination in post-dominance.
2025-08-05 14:01:04 +02:00
Anders Schack-Mulligen
23aac0ac51 Java: document nullness false negative as qltest 2025-08-05 13:49:51 +02:00
Tom Hvitved
6e90823bd9 Merge pull request #20158 from hvitved/csharp/has-callable-constructor
C#: Include constructors in `ValueOrRefType.hasCallable`
2025-08-05 12:59:29 +02:00
Anders Schack-Mulligen
273429d14a Java: Accept qltest output 2025-08-05 10:32:53 +02:00
Tom Hvitved
b426d84e1c Merge pull request #20164 from hvitved/rust/fix-bad-join
Rust: Fix bad join
2025-08-05 09:55:51 +02:00
Geoffrey White
dcda6db88b Rust: Lets not try to maintain this list. 2025-08-04 19:51:34 +01:00
Geoffrey White
0a49b65887 Rust: Make the rust/cleartext-transmission alert message more consistent with similar queries. 2025-08-04 19:47:33 +01:00
Chuan-kai Lin
e2b8d7b1ea Merge pull request #20166 from github/post-release-prep/codeql-cli-2.22.3
Post-release preparation for codeql-cli-2.22.3
2025-08-04 11:38:38 -07:00
Geoffrey White
6c024a5f9e Rust: Remove unnecessary pattern matching in cleartext logging query sinks (probably inherited from another query or language where it is used). 2025-08-04 19:28:40 +01:00
github-actions[bot]
fb4b0aac53 Post-release preparation for codeql-cli-2.22.3 2025-08-04 17:18:08 +00:00
Tom Hvitved
651e1624a6 Rust: Fix bad join
```
Evaluated relational algebra for predicate _Crate::Crate.getSourceFile/0#dispred#e7adf9d7_Crate::Generated::Crate.getName/0#dispred#f4d3b3bf_Pa__#join_rhs@5a04a7t0 with tuple counts:
        34471980   ~0%    {3} r1 = JOIN `PathResolution::isSourceFile/1#803de032` WITH `Crate::Crate.getSourceFile/0#dispred#e7adf9d7` CARTESIAN PRODUCT OUTPUT Rhs.0, Lhs.0, Rhs.1
        34471980  ~37%    {4}    | JOIN WITH `Crate::Generated::Crate.getName/0#dispred#f4d3b3bf` ON FIRST 1 OUTPUT Lhs.1, Lhs.2, Rhs.1, _
                          {3}    | REWRITE WITH NOT [Tmp.3 := "std", TEST InOut.2 != Tmp.3, Tmp.3 := "core", TEST InOut.2 != Tmp.3] KEEPING 3
           93420  ~91%    {3}    | SCAN OUTPUT In.1, _, In.0
           93420  ~87%    {3}    | REWRITE WITH Out.1 := "prelude"
                          return r1
```
2025-08-04 17:33:26 +02:00
Anders Schack-Mulligen
0a27a8c255 Java: Assume normal termination in post-dominance. 2025-08-04 15:08:26 +02:00
Simon Friis Vindum
3ba285c298 Rust: Implement certain type information for annotation and simple calls 2025-08-04 14:06:38 +02:00
Simon Friis Vindum
c3349bbb04 Rust: Add type inference example with cycle blowup 2025-08-04 14:06:37 +02:00
Tom Hvitved
361ef0f50d C#: Include constructors in ValueOrRefType.hasCallable 2025-08-04 13:51:17 +02:00
Geoffrey White
01d24c4f83 Merge branch 'main' into sqlx 2025-07-31 16:02:36 +01:00
Anders Schack-Mulligen
6c8275298b Java: Improve ObjFlow performance. 2025-07-25 14:41:06 +02:00
Anders Schack-Mulligen
5ca35afb8c Java: Improve joinorder in getErasedRepr. 2025-07-25 13:34:11 +02:00
Anders Schack-Mulligen
e3021f4a65 Java: Untangle code a bit to improve join order. 2025-07-25 13:33:14 +02:00
Geoffrey White
67c170ffc1 Merge branch 'main' into sqlx 2025-07-24 15:25:35 +01:00
Nora Dimitrijević
4b6135c0f7 [DIFF-INFORMED] Ruby: MissingFullAnchor
https://github.com/d10c/codeql/blob/d10c/diff-informed-phase-3/ruby/ql/src/queries/security/cwe-020/MissingFullAnchor.ql#L18
2025-07-17 14:44:02 +02:00
Geoffrey White
27bea33508 Rust: Accept consistency check change. 2025-07-17 12:44:31 +01:00
Geoffrey White
69064b7f7f Rust: Update the model. 2025-07-17 12:20:34 +01:00
Geoffrey White
944fd2aa11 Rust: Add explicit types in some (not all) of the test cases. 2025-07-17 10:45:40 +01:00
Geoffrey White
62b7d84638 Rust: Add Sqlx as MaD sinks instead. 2025-07-16 16:36:42 +01:00
Geoffrey White
87deab861f Rust: Remove Sqlx.qll. 2025-07-16 16:23:50 +01:00
Geoffrey White
6f5e4ef5b9 Merge branch 'main' into sqlx 2025-07-16 15:59:42 +01:00
Geoffrey White
f3b5cc79ff Merge branch 'main' into sqlx 2025-07-08 13:58:19 +01:00
Geoffrey White
dc08274aa2 Rust: Update SqlxQuery, SqlxExecute from getResolvedPath -> getCanonicalPath. 2025-06-17 15:56:18 +01:00
4261 changed files with 6399 additions and 213857 deletions

5
.gitattributes vendored
View File

@@ -88,8 +88,3 @@
# 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

View File

@@ -12,6 +12,9 @@ on:
required: false
default: openssl-3.5.0
permissions:
contents: read
jobs:
build:
strategy:

View File

@@ -14,10 +14,7 @@ jobs:
check:
name: Check framework coverage differences and comment
runs-on: ubuntu-latest
if: >
${{ github.event.workflow_run.event == 'pull_request' &&
github.event.workflow_run.conclusion == 'success' }}
if: github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success'
steps:
- name: Dump GitHub context
env:

View File

@@ -1,152 +0,0 @@
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 }}

View File

@@ -1,32 +0,0 @@
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

View File

@@ -1,28 +0,0 @@
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 }}

View File

@@ -1,91 +0,0 @@
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 }}

3
.gitmodules vendored
View File

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

View File

@@ -29,5 +29,3 @@ 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.

View File

@@ -1,41 +0,0 @@
<!-- 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 -->

View File

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

View File

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

View File

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

View File

@@ -115,10 +115,6 @@ 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%")

View File

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

View File

@@ -1,11 +0,0 @@
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
}

View File

@@ -1,41 +0,0 @@
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);
}

View File

@@ -1,79 +0,0 @@
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;

View File

@@ -1,19 +0,0 @@
/**
* 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
}

View File

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

View File

@@ -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>

View File

@@ -4,8 +4,8 @@
* value of 365, it may be a sign that leap years are not taken
* into account.
* @kind problem
* @problem.severity error
* @id cpp/microsoft/public/leap-year/adding-365-days-per-year
* @problem.severity warning
* @id cpp/leap-year/adding-365-days-per-year
* @precision medium
* @tags leap-year
* correctness
@@ -13,13 +13,11 @@
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,
"$@: 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()
"An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios.",
source, source.toString()

View File

@@ -1,17 +0,0 @@
/**
* @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)"

View File

@@ -3,7 +3,7 @@
*/
import cpp
import semmle.code.cpp.dataflow.new.TaintTracking
import semmle.code.cpp.ir.dataflow.TaintTracking
import semmle.code.cpp.commons.DateTime
/**
@@ -41,271 +41,6 @@ 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.
*/
@@ -338,7 +73,48 @@ abstract class LeapYearFieldAccess extends YearFieldAccess {
this.isModified() and
exists(Operation op |
op.getAnOperand() = this and
not isNormalizationOperation(op)
(
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
)
)
}
@@ -379,7 +155,9 @@ 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 = leapYearCheckFieldAccess(_)
this.isUsedInMod4Operation() and
this.additionalModulusCheckForLeapYear(400) and
this.additionalModulusCheckForLeapYear(100)
}
}
@@ -397,9 +175,19 @@ class StructTmLeapYearFieldAccess extends LeapYearFieldAccess {
StructTmLeapYearFieldAccess() { this.getTarget().getName() = "tm_year" }
override predicate isUsedInCorrectLeapYearCheck() {
this = leapYearCheckFieldAccess(_) and
/* There is some data flow from some conversion arithmetic to this expression. */
LocalConvertedYearByOffsetToLeapYearCheckFlow::flow(_, DataFlow::exprNode(this))
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)
)
}
}
@@ -418,10 +206,10 @@ class ChecksForLeapYearFunctionCall extends FunctionCall {
}
/**
* A `DataFlow` configuraiton for finding a variable access that would flow into
* Data flow configuration for finding a variable access that would flow into
* a function call that includes an operation to check for leap year.
*/
private module LeapYearCheckFlowConfig implements DataFlow::ConfigSig {
private module LeapYearCheckConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source.asExpr() instanceof VariableAccess }
predicate isSink(DataFlow::Node sink) {
@@ -429,10 +217,11 @@ private module LeapYearCheckFlowConfig implements DataFlow::ConfigSig {
}
}
module LeapYearCheckFlow = DataFlow::Global<LeapYearCheckFlowConfig>;
module LeapYearCheckFlow = DataFlow::Global<LeapYearCheckConfig>;
/**
* A `DataFlow` configuration for finding an operation with hardcoded 365 that will flow into a `_FILETIME` field.
* Data flow configuration for finding an operation with hardcoded 365 that will flow into
* a `FILEINFO` field.
*/
private module FiletimeYearArithmeticOperationCheckConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
@@ -457,72 +246,46 @@ module FiletimeYearArithmeticOperationCheckFlow =
DataFlow::Global<FiletimeYearArithmeticOperationCheckConfig>;
/**
* A `DataFlow` configuration for finding an operation with hardcoded 365 that will flow into any known date/time field.
* Taint 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) {
// 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())
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
)
}
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 and
sink.asExpr() = aexpr.getRValue()
aexpr.getLValue() = fa
)
}
}
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"
]
}
}

View File

@@ -1,26 +0,0 @@
<!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>

View File

@@ -1,28 +0,0 @@
/**
* @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"

View File

@@ -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="examples/UncheckedLeapYearAfterYearModificationBad.c" />
<sample src="UncheckedLeapYearAfterYearModificationBad.c" />
<p>To fix this bug, check the result for leap year.</p>
<sample src="examples/UncheckedLeapYearAfterYearModificationGood.c" />
<sample src="UncheckedLeapYearAfterYearModificationGood.c" />
</example>
<references>

View File

@@ -1,9 +1,9 @@
/**
* @name Year field changed using an arithmetic operation without checking for leap year (AntiPattern 1)
* @name Year field changed using an arithmetic operation without checking for leap year
* @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/microsoft/public/leap-year/unchecked-after-arithmetic-year-modification
* @id cpp/leap-year/unchecked-after-arithmetic-year-modification
* @precision medium
* @tags leap-year
* correctness
@@ -12,16 +12,13 @@
import cpp
import LeapYear
/**
* 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
from Variable var, LeapYearFieldAccess yfa
where
exists(VariableAccess va |
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
@@ -44,10 +41,8 @@ predicate isYearModifedWithoutExplicitLeapYearCheck(Variable var, YearWriteOp yw
LeapYearCheckFlow::flow(DataFlow::exprNode(yfacheck), DataFlow::exprNode(fc.getAnArgument()))
)
or
// 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
|
// If there is a successor or predecessor that sets the month = 1
exists(MonthFieldAccess mfa, AssignExpr ae |
mfa.getQualifier() = var.getAnAccess() and
mfa.isModified() and
(
@@ -55,87 +50,10 @@ predicate isYearModifedWithoutExplicitLeapYearCheck(Variable var, YearWriteOp yw
yfa.getBasicBlock() = mfa.getBasicBlock().getASuccessor+()
) and
ae = mfa.getEnclosingElement() and
ae.getAnOperand().getValue().toInt() = val
ae.getAnOperand().getValue().toInt() = 1
)
)
)
}
/**
* 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()
select yfa,
"Field $@ on variable $@ has been modified, but no appropriate check for LeapYear was found.",
yfa.getTarget(), yfa.getTarget().toString(), var, var.toString()

View File

@@ -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="examples/UncheckedLeapYearAfterYearModificationBad.c" />
<sample src="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="examples/UncheckedLeapYearAfterYearModificationGood.c" />
<sample src="UncheckedLeapYearAfterYearModificationGood.c" />
</example>
<references>

View File

@@ -1,11 +1,11 @@
/**
* @name Unchecked return value for time conversion function (AntiPattern 6)
* @name Unchecked return value for time conversion function
* @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/microsoft/public/leap-year/unchecked-return-value-for-time-conversion-function
* @id cpp/leap-year/unchecked-return-value-for-time-conversion-function
* @precision medium
* @tags leap-year
* correctness
@@ -14,6 +14,51 @@
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
@@ -59,6 +104,5 @@ where
)
)
select fcall,
"$@: 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()
"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()

View File

@@ -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="examples/UnsafeArrayForDaysOfYearBad.c" />
<sample src="UnsafeArrayForDaysOfYearBad.c" />
<p>When using arrays, allocate the correct number of elements to match the year.</p>
<sample src="examples/UnsafeArrayForDaysOfYearGood.c" />
<sample src="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>

View File

@@ -1,62 +1,41 @@
/**
* @name Unsafe array for days of the year (AntiPattern 4)
* @name Unsafe array for days of the year
* @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/microsoft/public/leap-year/unsafe-array-for-days-of-the-year
* @id cpp/leap-year/unsafe-array-for-days-of-the-year
* @precision low
* @tags leap-year
* correctness
* @tags security
* leap-year
*/
import cpp
/* 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.
*/
class LeapYearUnsafeDaysOfTheYearArrayType extends ArrayType {
LeapYearUnsafeDaysOfTheYearArrayType() { this.getArraySize() = 365 }
}
bindingset[size]
predicate isElementAnArrayOfFixedSize(
Element element, Type t, Declaration f, string allocType, int size
) {
from Element element, string allocType
where
exists(NewArrayExpr nae |
element = nae and
nae.getAllocatedType().(ArrayType).getArraySize() = size and
allocType = "an array allocation" and
f = nae.getEnclosingFunction() and
t = nae.getAllocatedType().(ArrayType).getBaseType()
nae.getAllocatedType() instanceof LeapYearUnsafeDaysOfTheYearArrayType and
allocType = "an array allocation"
)
or
exists(Variable var |
var = element and
var.getType().(ArrayType).getArraySize() = size and
allocType = "an array allocation" and
f = var and
t = var.getType().(ArrayType).getBaseType()
var.getType() instanceof LeapYearUnsafeDaysOfTheYearArrayType and
allocType = "an array allocation"
)
or
exists(ConstructorCall cc |
element = cc and
cc.getTarget().hasName("vector") and
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()
cc.getArgument(0).getValue().toInt() = 365 and
allocType = "a std::vector allocation"
)
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.",
f, f.toString()
"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."

View File

@@ -1,21 +0,0 @@
// 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
}

View File

@@ -1,20 +0,0 @@
<!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>

View File

@@ -1,31 +0,0 @@
/**
* @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"

View File

@@ -1,9 +0,0 @@
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;
}

View File

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

View File

@@ -1,29 +0,0 @@
<!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>

View File

@@ -1,69 +0,0 @@
/**
* @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

View File

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

View File

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

View File

@@ -1,22 +0,0 @@
<!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>

View File

@@ -1,62 +0,0 @@
/**
* @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

View File

@@ -1,5 +0,0 @@
#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));

View File

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

View File

@@ -1,26 +0,0 @@
<!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>

View File

@@ -1,54 +0,0 @@
/**
* @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()

View File

@@ -1,12 +0,0 @@
#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
}

View File

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

View File

@@ -1,45 +0,0 @@
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))
}
}

View File

@@ -1,46 +0,0 @@
<!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>

View File

@@ -1,58 +0,0 @@
/**
* @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()

View File

@@ -1,29 +0,0 @@
<!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>

View File

@@ -1,40 +0,0 @@
/**
* @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."

View File

@@ -1,31 +0,0 @@
<!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>

View File

@@ -1,23 +0,0 @@
/**
* @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."

View File

@@ -1,97 +0,0 @@
/**
* 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>;

View File

@@ -1,137 +0,0 @@
/**
* 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>;

View File

@@ -1,45 +0,0 @@
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)
}

View File

@@ -1,23 +0,0 @@
<!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>

View File

@@ -1,58 +0,0 @@
/**
* @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 "

View File

@@ -1,14 +0,0 @@
<!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>

View File

@@ -1,85 +0,0 @@
/**
* @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()

View File

@@ -1,14 +0,0 @@
<!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>

View File

@@ -1,51 +0,0 @@
/**
* @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()

View File

@@ -1,14 +0,0 @@
<!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>

View File

@@ -1,46 +0,0 @@
/**
* @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()

View File

@@ -1,15 +0,0 @@
<!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>

View File

@@ -1,46 +0,0 @@
/**
* @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()

View File

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

View File

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

View File

@@ -1,14 +0,0 @@
#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);
}

View File

@@ -1,14 +0,0 @@
#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);
}

View File

@@ -1,14 +0,0 @@
#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");
}
}

View File

@@ -1,14 +0,0 @@
#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");
}
}

View File

@@ -1,12 +0,0 @@
#include <windows.h>
#include <stdio.h>
#include <bcrypt.h>
int main(){
BCRYPT_ALG_HANDLE hAlg;
NTSTATUS status;
//BAD
status = BCryptOpenAlgorithmProvider(&hAlg, BCRYPT_DES_ALGORITHM, MS_PRIMITIVE_PROVIDER, 0);
}

View File

@@ -1,12 +0,0 @@
#include <windows.h>
#include <stdio.h>
#include <bcrypt.h>
int main(){
BCRYPT_ALG_HANDLE hAlg;
NTSTATUS status;
//OKAY
status = BCryptOpenAlgorithmProvider(&hAlg, BCRYPT_AES_ALGORITHM, MS_PRIMITIVE_PROVIDER, 0);
}

View File

@@ -1,15 +0,0 @@
typedef enum {
exampleSomeValue,
exampleSomeOtherValue,
exampleValueMax
} EXAMPLE_VALUES;
/*...*/
int variable = someStructure->example;
if (variable >= exampleValueMax)
{
/* ... Some action ... */
}
// ...
Status = someArray[variable](/*...*/);

View File

@@ -1,24 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>This rule finds code where an enumerated type (<code>enum</code>) is used to check for an upper boundary, but not the lower boundary, and the value is used as an index to access an array.</p>
<p>By default an enum variable is signed, and therefore it is important to ensure that it cannot take on a negative value. When the enum is subsequently used to index an array, or worse still an array of function pointers, then a negative enum value would lead to potentially arbitrary memory being read, used and/or executed.</p>
</overview>
<recommendation>
<p>In the majority of cases the fix is simply to add the required lower bounds check to ensure that the enum has a positive value.</p>
</recommendation>
<example>
<p>The following example a value is passed and gets cast to an enumerated type and only partially bounds checked.</p>
<sample src="UncheckedBoundsEnumAsIndex.c" />
<p>In this example, the result of the out-of-bounds may allow for arbitrary code execution.</p>
<p>To fix the problem in this example, you need to add an additional check to the guarding if statement to verify that the index is a positive value.</p>
</example>
<references>
</references>
</qhelp>

View File

@@ -1,122 +0,0 @@
/**
* @name EnumIndex
* @description Code using enumerated types as indexes into arrays will often check for
* an upper bound to ensure the index is not out of range.
* By default an enum variable is signed, and therefore it is important to ensure
* that it cannot take on a negative value. When the enum is subsequently used
* to index an array, or worse still an array of function pointers, then a negative
* enum value would lead to potentially arbitrary memory being read, used and/or executed.
* @kind problem
* @problem.severity error
* @precision high
* @id cpp/microsoft/public/enum-index
* @tags security
* external/cwe/cwe-125
* external/microsoft/c33010
*/
import cpp
import semmle.code.cpp.controlflow.Guards
private import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
/**
* Holds if `ec` is the upper bound of an enum
*/
predicate isUpperBoundEnumValue(EnumConstant ec) {
not exists(EnumConstant ec2, Enum enum | enum = ec2.getEnclosingElement() |
enum = ec.getEnclosingElement() and
ec2.getValue().toInt() > ec.getValue().toInt()
)
}
/**
* Holds if 'eca' is an access to the upper bound of an enum
*/
predicate isUpperBoundEnumAccess(EnumConstantAccess eca) {
exists(EnumConstant ec |
varbind(eca, ec) and
isUpperBoundEnumValue(ec)
)
}
/**
* Holds if the expression `e` is accessing the enum constant `ec`
*/
predicate isExpressionAccessingUpperboundEnum(Expr e, EnumConstantAccess ec) {
isExpressionAccessingUpperboundEnum(e.getAChild(), ec)
or
ec = e and
isUpperBoundEnumAccess(ec)
}
/**
* Holds if `e` is a child of an If statement
*/
predicate isPartOfAnIfStatement(Expr e) {
isPartOfAnIfStatement(e.getAChild())
or
exists(IfStmt ifs | ifs.getAChild() = e)
}
/**
* Holds if the variable access `offsetExpr` upper bound is guarded by an If statement GuardCondition
* that is using the upper bound of an enum to check the upper bound of `offsetExpr`
*/
predicate hasUpperBoundDefinedByEnum(VariableAccess offsetExpr) {
exists(BasicBlock controlled, StackVariable offsetVar, SsaDefinition def |
controlled.contains(offsetExpr) and
linearBoundControlsEnum(controlled, def, offsetVar, Lesser()) and
offsetExpr = def.getAUse(offsetVar)
)
}
pragma[noinline]
predicate linearBoundControlsEnum(
BasicBlock controlled, SsaDefinition def, StackVariable offsetVar, RelationDirection direction
) {
exists(GuardCondition guard |
exists(boolean branch |
guard.controls(controlled, branch) and
cmpWithLinearBound(guard, def.getAUse(offsetVar), direction, branch)
) and
exists(EnumConstantAccess enumca | isExpressionAccessingUpperboundEnum(guard, enumca)) and
isPartOfAnIfStatement(guard)
)
}
/**
* Holds if the variable access `offsetExpr` lower bound is guarded
*/
predicate hasLowerBound(VariableAccess offsetExpr) {
exists(BasicBlock controlled, StackVariable offsetVar, SsaDefinition def |
controlled.contains(offsetExpr) and
linearBoundControls(controlled, def, offsetVar, Greater()) and
offsetExpr = def.getAUse(offsetVar)
)
}
pragma[noinline]
predicate linearBoundControls(
BasicBlock controlled, SsaDefinition def, StackVariable offsetVar, RelationDirection direction
) {
exists(GuardCondition guard, boolean branch |
guard.controls(controlled, branch) and
cmpWithLinearBound(guard, def.getAUse(offsetVar), direction, branch) and
isPartOfAnIfStatement(guard)
)
}
from VariableAccess offset, ArrayExpr array
where
offset = array.getArrayOffset() and
hasUpperBoundDefinedByEnum(offset) and
not hasLowerBound(offset) and
exists(IntegralType t |
t = offset.getUnderlyingType() and
not t.isUnsigned()
) and
lowerBound(offset.getFullyConverted()) < 0
select offset,
"When accessing array " + array.getArrayBase() + " with index " + offset +
", the upper bound of an enum is used to check the upper bound of the array, but the lower bound is not checked."

View File

@@ -1,29 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Hard-coding security protocols rather than specifying the system default is risky because the protocol may become deprecated in future.</p>
<p>The <code>grbitEnabledProtocols</code> member of the <code>SCHANNEL_CRED</code> struct contains a bit string that represents the protocols supported by connections made with credentials acquired by using this structure. If this member is zero, Schannel selects the protocol. Applications should set <code>grbitEnabledProtocols</code> to zero and use the protocol versions enabled on the system by default.</p>
</overview>
<recommendation>
<p> - Set the <code>grbitEnabledProtocols</code> member of the <code>SCHANNEL_CRED</code> struct to <code>0</code>.</p>
</recommendation>
<example>
<p>Violation:</p>
<sample src="examples/HardCodedSecurityProtocol/HardCodedSecurityProtocol1.cpp" />
<p>Solution:</p>
<sample src="examples/HardCodedSecurityProtocol/HardCodedSecurityProtocol2.cpp" />
</example>
<references>
<li>Microsoft Docs: <a href="https://learn.microsoft.com/en-us/windows/win32/api/schannel/ns-schannel-schannel_cred">SCHANNEL_CRED structure</a>.</li>
</references>
</qhelp>

View File

@@ -1,19 +0,0 @@
/**
* @name Hard-coded use of a security protocol
* @description Hard-coding the security protocol used rather than specifying the system default is
* risky because the protocol may become deprecated in future.
* @kind problem
* @problem.severity warning
* @id cpp/microsoft/public/hardcoded-security-protocol
*/
import cpp
import HardCodedSecurityProtocol
from ProtocolConstant constantValue, DataFlow::Node grbitEnabledProtocolsAssignment
where
GrbitEnabledConstantTace::flow(DataFlow::exprNode(constantValue), grbitEnabledProtocolsAssignment) and
constantValue.isHardcodedProtocol()
select constantValue,
"Hard-coded use of security protocol " + getConstantName(constantValue) + " set here $@.",
grbitEnabledProtocolsAssignment, grbitEnabledProtocolsAssignment.toString()

View File

@@ -1,140 +0,0 @@
import cpp
import semmle.code.cpp.dataflow.new.TaintTracking
/**
* A constant representing one or more security protocols for the `grbitEnabledProtocols` field.
*/
class ProtocolConstant extends Expr {
ProtocolConstant() {
this.isConstant() and
GrbitEnabledConstantTace::flow(DataFlow::exprNode(this), _) and
(
this instanceof Literal
or
this = any(ConstantMacroInvocation mi).getExpr()
or
// This is a workaround for folded constants, which currently have no
// dataflow node representation. Attach to the outermost dataflow node
// where a literal exists as a child that has no dataflow node representation.
exists(Literal l |
this.getAChild*() = l and
not exists(DataFlow::Node n | n.asExpr() = l)
)
)
}
/** Gets the bitmask represented by this constant. */
int getBitmask() { result = this.getValue().toInt() }
/** Holds if this constant only represents TLS1.3 protocols. */
predicate isTLS1_3Only() {
// Flags for TLS1.3 are 0x00001000 and 0x00002000
// 12288 = 0x00001000 | 0x00002000
this.getBitmask().bitAnd(12288.bitNot()) = 0 and
not this.isSystemDefault()
}
/** Holds if this constant only represents TLS1.2 protocols. */
predicate isTLS1_2Only() {
// Flags for TLS1.2 are 0x00000400 and 0x00000800
// 3072 = 0x00000400 | 0x00000800
this.getBitmask().bitAnd(3072.bitNot()) = 0 and
not this.isSystemDefault()
}
/** Holds if this constant only represents TLS1.1 protocols. */
predicate isTLS1_1Only() {
// Flags for TLS1.1 are 0x00000100 and 0x00000200
// 768 = 0x00000100 | 0x00000200
this.getBitmask().bitAnd(768.bitNot()) = 0 and
not this.isSystemDefault()
}
/** Holds if this constant only represents TLS1.0 protocols. */
predicate isTLS1_0Only() {
// Flags for TLS1.0 are 0x00000040 and 0x00000080
// 192 = 0x00000040 | 0x00000080
this.getBitmask().bitAnd(192.bitNot()) = 0 and
not this.isSystemDefault()
}
/** Holds if this constant only represents TLS1.1 protocols. */
predicate isSSL3Only() {
// Flags for SSL3 are 0x00000010 and 0x00000020
// 48 = 0x00000010 | 0x00000020
this.getBitmask().bitAnd(48.bitNot()) = 0 and
not this.isSystemDefault()
}
/** Holds if this constant only represents SSL2 protocols. */
predicate isSSL2Only() {
// Flags for TLS1.0 are 0x00000004 and 0x00000008
// 12 = 0x00000004 | 0x00000008
this.getBitmask().bitAnd(12.bitNot()) = 0 and
not this.isSystemDefault()
}
/** Holds if this constant only represents PCT1 protocols. */
predicate isPCT1Only() {
// Flags for PCT are 0x00000001 and 0x00000002
// 3 = 0x00000001 | 0x00000002
this.getBitmask().bitAnd(3.bitNot()) = 0 and
not this.isSystemDefault()
}
/** Holds if this constant only represents any combination of TLS-related protocols. */
predicate isHardcodedProtocol() {
// 16383 = SP_PROT_TLS1_3 | SP_PROT_TLS1_2 | SP_PROT_TLS1_1 | SP_PROT_TLS1_3
// | SP_PROT_TLS1 | SP_PROT_SSL3 | SP_PROT_SSL2 | SP_PROT_PCT1
this.getBitmask().bitAnd(16383.bitNot()) = 0 and
not this.isSystemDefault()
}
/** Holds if this constant represents the system default protocol. */
predicate isSystemDefault() { this.getBitmask() = 0 }
}
/**
* A data flow configuration that tracks from constant values to assignments to the
* `grbitEnabledProtocols` field on the SCHANNEL_CRED structure.
*/
module GrbitEnabledConstantConfiguration implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source.asExpr().isConstant() }
predicate isSink(DataFlow::Node sink) {
exists(Field grbitEnabledProtocols |
grbitEnabledProtocols.hasName("grbitEnabledProtocols") and
sink.asExpr() = grbitEnabledProtocols.getAnAssignedValue()
)
}
predicate isBarrier(DataFlow::Node node) {
// Do not flow through other macro invocations if they would, themselves, be represented
node.asExpr() = any(ConstantMacroInvocation mi).getExpr().getAChild+()
or
// Do not flow through complements, as they change the meaning
node.asExpr() instanceof ComplementExpr
}
}
module GrbitEnabledConstantTace = TaintTracking::Global<GrbitEnabledConstantConfiguration>;
/**
* A macro that represents a constant value.
*/
class ConstantMacroInvocation extends MacroInvocation {
ConstantMacroInvocation() {
exists(this.getExpr().getValue()) and
not this.getMacro().getHead().matches("%(%)%")
}
}
/**
* Gets the name of the constant `val`, if it is a constant.
*/
string getConstantName(Expr val) {
exists(val.getValue()) and
if exists(ConstantMacroInvocation mi | mi.getExpr() = val)
then result = any(ConstantMacroInvocation mi | mi.getExpr() = val).getMacroName()
else result = val.toString()
}

View File

@@ -1,29 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Older protocol versions of TLS are less secure than TLS 1.2 and TLS 1.3 and are more likely to have new vulnerabilities. Avoid older protocol versions to minimize risk.</p>
<p>The <code>grbitEnabledProtocols</code> member of the <code>SCHANNEL_CRED</code> struct contains a bit string that represents the protocols supported by connections made with credentials acquired by using this structure. If this member is zero, Schannel selects the protocol. Applications should set <code>grbitEnabledProtocols</code> to zero and use the protocol versions enabled on the system by default.</p>
</overview>
<recommendation>
<p> - Set the <code>grbitEnabledProtocols</code> member of the <code>SCHANNEL_CRED</code> struct to <code>0</code>.</p>
</recommendation>
<example>
<p>Violation:</p>
<sample src="examples/UseOfDeprecatedSecurityProtocol/UseOfDeprecatedSecurityProtocol1.cpp" />
<p>Solution:</p>
<sample src="examples/UseOfDeprecatedSecurityProtocol/UseOfDeprecatedSecurityProtocol2.cpp" />
</example>
<references>
<li>Microsoft Docs: <a href="https://learn.microsoft.com/en-us/windows/win32/api/schannel/ns-schannel-schannel_cred">SCHANNEL_CRED structure</a>.</li>
</references>
</qhelp>

View File

@@ -1,21 +0,0 @@
/**
* @name Hard-coded use of a deprecated security protocol
* @description Using a deprecated security protocol rather than the system default is risky.
* @kind problem
* @problem.severity error
* @id cpp/microsoft/public/use-of-deprecated-security-protocol
*/
import cpp
import HardCodedSecurityProtocol
from ProtocolConstant constantValue, DataFlow::Node grbitEnabledProtocolsAssignment
where
GrbitEnabledConstantTace::flow(DataFlow::exprNode(constantValue), grbitEnabledProtocolsAssignment) and
// If the system default hasn't been specified, and TLS2 has not been specified, then this is a deprecated security protocol
not constantValue.isSystemDefault() and
not constantValue.isTLS1_2Only() and
not constantValue.isTLS1_3Only()
select constantValue,
"Hard-coded use of deprecated security protocol " + getConstantName(constantValue) +
" set here $@.", constantValue, getConstantName(constantValue)

View File

@@ -1,18 +0,0 @@
#include <windows.h>
#include <ntsecapi.h>
#include <stdio.h>
#include <sspi.h>
#include <schnlsp.h>
void HardCodedSecurityProtocolGood()
{
SCHANNEL_CRED credData;
ZeroMemory(&credData, sizeof(credData));
// BAD: hardcoded protocols
credData.grbitEnabledProtocols = SP_PROT_TLS1_2;
credData.grbitEnabledProtocols = SP_PROT_TLS1_3;
return;
}

View File

@@ -1,17 +0,0 @@
#include <windows.h>
#include <ntsecapi.h>
#include <stdio.h>
#include <sspi.h>
#include <schnlsp.h>
void HardCodedSecurityProtocolGood()
{
SCHANNEL_CRED credData;
ZeroMemory(&credData, sizeof(credData));
// GOOD: system default protocol
credData.grbitEnabledProtocols = 0;
return;
}

View File

@@ -1,23 +0,0 @@
#include <windows.h>
#include <ntsecapi.h>
#include <stdio.h>
#include <sspi.h>
#include <schnlsp.h>
void UseOfDeprecatedSecurityProtocolGood()
{
SCHANNEL_CRED credData;
ZeroMemory(&credData, sizeof(credData));
// BAD: Deprecated protocols
credData.grbitEnabledProtocols = SP_PROT_PCT1_SERVER;
credData.grbitEnabledProtocols = SP_PROT_SSL2_SERVER;
credData.grbitEnabledProtocols = SP_PROT_SSL3_SERVER;
credData.grbitEnabledProtocols = SP_PROT_TLS1_1;
credData.grbitEnabledProtocols = SP_PROT_TLS1_1_SERVER;
credData.grbitEnabledProtocols = SP_PROT_TLS1_1_CLIENT;
credData.grbitEnabledProtocols = SP_PROT_SSL3TLS1;
return;
}

View File

@@ -1,17 +0,0 @@
#include <windows.h>
#include <ntsecapi.h>
#include <stdio.h>
#include <sspi.h>
#include <schnlsp.h>
void HardCodedSecurityProtocolGood()
{
SCHANNEL_CRED credData;
ZeroMemory(&credData, sizeof(credData));
// GOOD: system default protocol
credData.grbitEnabledProtocols = 0;
return;
}

View File

@@ -51,10 +51,7 @@ predicate offsetIsAlwaysInBounds(ArrayExpr arrayExpr, VariableAccess offsetExpr)
}
module ImproperArrayIndexValidationConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
isFlowSource(source, _) and
not source.getLocation().getFile().getRelativePath().regexpMatch("(.*/)?tests?/.*")
}
predicate isSource(DataFlow::Node source) { isFlowSource(source, _) }
predicate isBarrier(DataFlow::Node node) {
node = DataFlow::BarrierGuard<guardChecks/3>::getABarrierNode()
@@ -76,8 +73,7 @@ module ImproperArrayIndexValidationConfig implements DataFlow::ConfigSig {
module ImproperArrayIndexValidation = TaintTracking::Global<ImproperArrayIndexValidationConfig>;
from
ImproperArrayIndexValidation::PathNode source,
ImproperArrayIndexValidation::PathNode sink,
ImproperArrayIndexValidation::PathNode source, ImproperArrayIndexValidation::PathNode sink,
string sourceType
where
ImproperArrayIndexValidation::flowPath(source, sink) and

View File

@@ -2,7 +2,7 @@
* @name Weak cryptography
* @description Finds explicit uses of symmetric encryption algorithms that are weak, unknown, or otherwise unaccepted.
* @kind problem
* @id cpp/experimental/weak-crypto/banned-encryption-algorithms
* @id cpp/weak-crypto/banned-encryption-algorithms
* @problem.severity error
* @precision high
* @tags external/cwe/cwe-327

View File

@@ -1,5 +1,5 @@
name: codeql/cpp-queries
version: 1.4.5
version: 1.4.6-dev
groups:
- cpp
- queries

View File

@@ -1,7 +1,5 @@
| test.cpp:175:29:175:51 | ... & ... | $@: This arithmetic operation $@ uses a constant value of 365 ends up modifying the date/time located at $@, without considering leap year scenarios. | test.cpp:159:6:159:17 | antipattern2 | antipattern2 | test.cpp:172:2:172:47 | ... += ... | ... += ... | test.cpp:175:29:175:51 | ... & ... | ... & ... |
| test.cpp:176:30:176:45 | ... >> ... | $@: This arithmetic operation $@ uses a constant value of 365 ends up modifying the date/time located at $@, without considering leap year scenarios. | test.cpp:159:6:159:17 | antipattern2 | antipattern2 | test.cpp:172:2:172:47 | ... += ... | ... += ... | test.cpp:176:30:176:45 | ... >> ... | ... >> ... |
| test.cpp:195:15:195:24 | ... / ... | $@: This arithmetic operation $@ uses a constant value of 365 ends up modifying the date/time located at $@, without considering leap year scenarios. | test.cpp:185:8:185:13 | mkTime | mkTime | test.cpp:195:15:195:24 | ... / ... | ... / ... | test.cpp:195:15:195:24 | ... / ... | ... / ... |
| test.cpp:219:29:219:51 | ... & ... | $@: This arithmetic operation $@ uses a constant value of 365 ends up modifying the date/time located at $@, without considering leap year scenarios. | test.cpp:203:6:203:19 | checkedExample | checkedExample | test.cpp:216:2:216:47 | ... += ... | ... += ... | test.cpp:219:29:219:51 | ... & ... | ... & ... |
| test.cpp:220:30:220:45 | ... >> ... | $@: This arithmetic operation $@ uses a constant value of 365 ends up modifying the date/time located at $@, without considering leap year scenarios. | test.cpp:203:6:203:19 | checkedExample | checkedExample | test.cpp:216:2:216:47 | ... += ... | ... += ... | test.cpp:220:30:220:45 | ... >> ... | ... >> ... |
| test.cpp:247:29:247:51 | ... & ... | $@: This arithmetic operation $@ uses a constant value of 365 ends up modifying the date/time located at $@, without considering leap year scenarios. | test.cpp:230:6:230:18 | antipattern2A | antipattern2A | test.cpp:240:20:240:23 | 365 | 365 | test.cpp:247:29:247:51 | ... & ... | ... & ... |
| test.cpp:248:30:248:45 | ... >> ... | $@: This arithmetic operation $@ uses a constant value of 365 ends up modifying the date/time located at $@, without considering leap year scenarios. | test.cpp:230:6:230:18 | antipattern2A | antipattern2A | test.cpp:240:20:240:23 | 365 | 365 | test.cpp:248:30:248:45 | ... >> ... | ... >> ... |
| test.cpp:173:29:173:51 | ... & ... | An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios. | test.cpp:170:2:170:47 | ... += ... | ... += ... |
| test.cpp:174:30:174:45 | ... >> ... | An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios. | test.cpp:170:2:170:47 | ... += ... | ... += ... |
| test.cpp:193:15:193:24 | ... / ... | An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios. | test.cpp:193:15:193:24 | ... / ... | ... / ... |
| test.cpp:217:29:217:51 | ... & ... | An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios. | test.cpp:214:2:214:47 | ... += ... | ... += ... |
| test.cpp:218:30:218:45 | ... >> ... | An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios. | test.cpp:214:2:214:47 | ... += ... | ... += ... |

View File

@@ -153,9 +153,7 @@ GetFileTime(
LPFILETIME lpLastWriteTime
);
/**
* AntiPattern2 - datetime.AddDays(±365)
*/
void antipattern2()
{
// get the current time as a FILETIME
@@ -225,28 +223,3 @@ void checkedExample()
// handle error...
}
}
void antipattern2A()
{
// get the current time as a FILETIME
SYSTEMTIME st; FILETIME ft;
GetSystemTime(&st);
SystemTimeToFileTime(&st, &ft);
// convert to a quadword (64-bit integer) to do arithmetic
ULONGLONG qwLongTime;
qwLongTime = (((ULONGLONG)ft.dwHighDateTime) << 32) + ft.dwLowDateTime;
int days_in_year = 365;
// add a year by calculating the ticks in 365 days
// (which may be incorrect when crossing a leap day)
qwLongTime += days_in_year * 24 * 60 * 60 * 10000000LLU;
// copy back to a FILETIME
ft.dwLowDateTime = (DWORD)(qwLongTime & 0xFFFFFFFF); // BAD
ft.dwHighDateTime = (DWORD)(qwLongTime >> 32); // BAD
// convert back to SYSTEMTIME for display or other usage
FileTimeToSystemTime(&ft, &st);
}

View File

@@ -1,3 +0,0 @@
| test.cpp:183:23:183:35 | ... == ... | Possible Insufficient Leap Year check (AntiPattern 5) |
| test.cpp:190:24:190:40 | ... == ... | Possible Insufficient Leap Year check (AntiPattern 5) |
| test.cpp:245:6:245:18 | ... == ... | Possible Insufficient Leap Year check (AntiPattern 5) |

View File

@@ -1 +0,0 @@
Likely Bugs/Leap Year/AntiPattern5InvalidLeapYearCheck.ql

Some files were not shown because too many files have changed in this diff Show More