mirror of
https://github.com/github/codeql.git
synced 2026-05-23 23:57:06 +02:00
Compare commits
57 Commits
testing-po
...
redsun82/a
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
93e05319cf | ||
|
|
72563ec5a4 | ||
|
|
3674966946 | ||
|
|
2909def9b6 | ||
|
|
b51c0e7cb6 | ||
|
|
d9cfe14729 | ||
|
|
a40ae3a11a | ||
|
|
ec513ead0d | ||
|
|
f90b6ab005 | ||
|
|
b156bd5ce2 | ||
|
|
0c31a80f3c | ||
|
|
6e52df1639 | ||
|
|
1bdaa2420d | ||
|
|
3aaf48de11 | ||
|
|
dfe4401f13 | ||
|
|
ed3a33fdc6 | ||
|
|
d215ea16da | ||
|
|
b50a76693a | ||
|
|
0cfb22ff3f | ||
|
|
b302f3f98f | ||
|
|
766083290c | ||
|
|
d201ce1705 | ||
|
|
1f15fc8a35 | ||
|
|
eb3c054b0f | ||
|
|
0d4f8765a6 | ||
|
|
a396f9345e | ||
|
|
1823355fae | ||
|
|
94274288d3 | ||
|
|
c59d20a668 | ||
|
|
23aac0ac51 | ||
|
|
6e90823bd9 | ||
|
|
273429d14a | ||
|
|
b426d84e1c | ||
|
|
dcda6db88b | ||
|
|
0a49b65887 | ||
|
|
e2b8d7b1ea | ||
|
|
6c024a5f9e | ||
|
|
fb4b0aac53 | ||
|
|
651e1624a6 | ||
|
|
0a27a8c255 | ||
|
|
3ba285c298 | ||
|
|
c3349bbb04 | ||
|
|
361ef0f50d | ||
|
|
01d24c4f83 | ||
|
|
6c8275298b | ||
|
|
5ca35afb8c | ||
|
|
e3021f4a65 | ||
|
|
67c170ffc1 | ||
|
|
4b6135c0f7 | ||
|
|
27bea33508 | ||
|
|
69064b7f7f | ||
|
|
944fd2aa11 | ||
|
|
62b7d84638 | ||
|
|
87deab861f | ||
|
|
6f5e4ef5b9 | ||
|
|
f3b5cc79ff | ||
|
|
dc08274aa2 |
5
.gitattributes
vendored
5
.gitattributes
vendored
@@ -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
|
||||
3
.github/workflows/build-ripunzip.yml
vendored
3
.github/workflows/build-ripunzip.yml
vendored
@@ -12,6 +12,9 @@ on:
|
||||
required: false
|
||||
default: openssl-3.5.0
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
build:
|
||||
strategy:
|
||||
|
||||
@@ -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:
|
||||
|
||||
152
.github/workflows/microsoft-codeql-pack-publish.yml
vendored
152
.github/workflows/microsoft-codeql-pack-publish.yml
vendored
@@ -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 }}
|
||||
|
||||
32
.github/workflows/powershell-pr-check.yml
vendored
32
.github/workflows/powershell-pr-check.yml
vendored
@@ -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
|
||||
28
.github/workflows/sync-main-tags.yml
vendored
28
.github/workflows/sync-main-tags.yml
vendored
@@ -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 }}
|
||||
91
.github/workflows/sync-main.yml
vendored
91
.github/workflows/sync-main.yml
vendored
@@ -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
3
.gitmodules
vendored
@@ -1,3 +0,0 @@
|
||||
[submodule "iac"]
|
||||
path = iac
|
||||
url = https://github.com/advanced-security/codeql-extractor-iac
|
||||
@@ -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.
|
||||
|
||||
|
||||
|
||||
41
SECURITY.md
41
SECURITY.md
@@ -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 -->
|
||||
@@ -1,5 +1,5 @@
|
||||
name: codeql/actions-all
|
||||
version: 0.4.14
|
||||
version: 0.4.15-dev
|
||||
library: true
|
||||
warnOnImplicitThis: true
|
||||
dependencies:
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
name: codeql/actions-queries
|
||||
version: 0.6.6
|
||||
version: 0.6.7-dev
|
||||
library: false
|
||||
warnOnImplicitThis: true
|
||||
groups: [actions, queries]
|
||||
|
||||
@@ -1,4 +0,0 @@
|
||||
---
|
||||
category: feature
|
||||
---
|
||||
* Added a new class `AdditionalCallTarget` for specifying additional call targets.
|
||||
@@ -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%")
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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);
|
||||
}
|
||||
@@ -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;
|
||||
@@ -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
|
||||
}
|
||||
@@ -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() }
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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>
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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)"
|
||||
@@ -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"
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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>
|
||||
@@ -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"
|
||||
@@ -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>
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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>
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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>
|
||||
|
||||
@@ -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."
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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>
|
||||
@@ -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"
|
||||
@@ -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;
|
||||
}
|
||||
@@ -1,9 +0,0 @@
|
||||
unsigned short CheckForInt16OverflowCorrectCode(unsigned short v, unsigned short b)
|
||||
{
|
||||
if (v + b > 0x00FFFF)
|
||||
{
|
||||
// ... do something
|
||||
}
|
||||
|
||||
return v + b;
|
||||
}
|
||||
@@ -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>
|
||||
@@ -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
|
||||
@@ -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;
|
||||
}
|
||||
@@ -1,5 +0,0 @@
|
||||
//fixed
|
||||
if (!RtlEqualMemory(pBuffer, ptr, 16))
|
||||
{
|
||||
return FALSE;
|
||||
}
|
||||
@@ -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>
|
||||
@@ -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
|
||||
@@ -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));
|
||||
@@ -1,4 +0,0 @@
|
||||
#define SIZEOF_CHAR sizeof(char)
|
||||
|
||||
char* buffer;
|
||||
buffer = (char*) malloc(SIZEOF_CHAR * 10);
|
||||
@@ -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>
|
||||
@@ -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()
|
||||
@@ -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
|
||||
}
|
||||
@@ -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
|
||||
}
|
||||
@@ -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))
|
||||
}
|
||||
}
|
||||
@@ -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>
|
||||
@@ -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()
|
||||
@@ -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>
|
||||
@@ -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."
|
||||
@@ -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>
|
||||
@@ -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."
|
||||
@@ -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>;
|
||||
@@ -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>;
|
||||
@@ -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)
|
||||
}
|
||||
@@ -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>
|
||||
@@ -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 "
|
||||
@@ -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>
|
||||
@@ -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()
|
||||
@@ -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>
|
||||
@@ -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()
|
||||
@@ -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>
|
||||
@@ -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()
|
||||
@@ -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>
|
||||
@@ -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()
|
||||
@@ -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);
|
||||
}
|
||||
@@ -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);
|
||||
}
|
||||
@@ -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);
|
||||
}
|
||||
@@ -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);
|
||||
}
|
||||
@@ -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");
|
||||
}
|
||||
}
|
||||
@@ -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");
|
||||
}
|
||||
}
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,15 +0,0 @@
|
||||
typedef enum {
|
||||
exampleSomeValue,
|
||||
exampleSomeOtherValue,
|
||||
exampleValueMax
|
||||
} EXAMPLE_VALUES;
|
||||
|
||||
/*...*/
|
||||
|
||||
int variable = someStructure->example;
|
||||
if (variable >= exampleValueMax)
|
||||
{
|
||||
/* ... Some action ... */
|
||||
}
|
||||
// ...
|
||||
Status = someArray[variable](/*...*/);
|
||||
@@ -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>
|
||||
@@ -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."
|
||||
@@ -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>
|
||||
@@ -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()
|
||||
@@ -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()
|
||||
}
|
||||
@@ -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>
|
||||
@@ -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)
|
||||
@@ -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;
|
||||
}
|
||||
@@ -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;
|
||||
}
|
||||
@@ -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;
|
||||
}
|
||||
@@ -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;
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
name: codeql/cpp-queries
|
||||
version: 1.4.5
|
||||
version: 1.4.6-dev
|
||||
groups:
|
||||
- cpp
|
||||
- queries
|
||||
|
||||
@@ -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 | ... += ... | ... += ... |
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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) |
|
||||
@@ -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
Reference in New Issue
Block a user