Compare commits

..

10 Commits

Author SHA1 Message Date
Max Schaefer
9b9e567919 Revert "JS: Recognize DomSanitizer from @angular/core"
This reverts commit ff1d0cc4c7.
2022-02-15 10:25:55 +00:00
Henry Mercer
eee33dfdda Remove NoSQL sinks since September 2018 2022-02-15 10:25:55 +00:00
Esben Sparre Andreasen
644274adfd Remove additional Xss sinks 2022-02-15 10:25:55 +00:00
Esben Sparre Andreasen
86ee0e7789 Remove additional SQL sinks 2022-02-15 10:25:55 +00:00
Esben Sparre Andreasen
af02c1ed5b Remove additional path-injection sinks 2022-02-15 10:25:55 +00:00
Esben Sparre Andreasen
c5ce60c872 Add benjamin-button.md 2022-02-15 10:25:55 +00:00
Esben Sparre Andreasen
39aabc6427 Remove pseudo-properties 2022-02-15 10:25:55 +00:00
Esben Sparre Andreasen
b308ab5130 Remove 2020 sinks from SqlInjection.ql 2022-02-15 10:25:55 +00:00
Esben Sparre Andreasen
df224b1e6e Remove 2020 sinks from Xss.ql 2022-02-15 10:25:54 +00:00
Esben Sparre Andreasen
164b7deb96 Remove 2020 sinks from TaintedPath.ql 2022-02-15 10:25:54 +00:00
1707 changed files with 116482 additions and 189470 deletions

View File

@@ -4,7 +4,6 @@
"*/ql/lib/qlpack.yml",
"*/ql/test/qlpack.yml",
"*/ql/examples/qlpack.yml",
"*/ql/consistency-queries/qlpack.yml",
"cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/qlpack.yml",
"javascript/ql/experimental/adaptivethreatmodeling/lib/qlpack.yml",
"javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/qlpack.yml",
@@ -15,6 +14,8 @@
"misc/legacy-support/*/qlpack.yml",
"misc/suite-helpers/qlpack.yml",
"ruby/extractor-pack/codeql-extractor.yml",
"ruby/ql/consistency-queries/qlpack.yml",
"ql/ql/consistency-queries/qlpack.yml",
"ql/extractor-pack/codeql-extractor.yml"
],
"versionPolicies": {

6
.gitattributes vendored
View File

@@ -52,12 +52,6 @@
java/ql/test/stubs/**/*.java linguist-generated=true
java/ql/test/experimental/stubs/**/*.java linguist-generated=true
# For some languages, upgrade script testing references really old dbscheme
# files from legacy upgrades that have CRLF line endings. Since upgrade
# resolution relies on object hashes, we must suppress line ending conversion
# for those testing dbscheme files.
*/ql/lib/upgrades/initial/*.dbscheme -text
# Generated test files - these are synced from the standard JavaScript libraries using
# `javascript/ql/experimental/adaptivethreatmodeling/test/update_endpoint_test_files.py`.
javascript/ql/experimental/adaptivethreatmodeling/test/endpoint_large_scale/autogenerated/**/*.js linguist-generated=true -merge

View File

@@ -6,11 +6,8 @@ on:
paths:
- "*/ql/src/**/*.ql"
- "*/ql/src/**/*.qll"
- "*/ql/lib/**/*.ql"
- "*/ql/lib/**/*.qll"
- "!**/experimental/**"
- "!ql/**"
- ".github/workflows/check-change-note.yml"
jobs:
check-change-note:

View File

@@ -1,43 +0,0 @@
name: "Publish framework coverage as metrics"
on:
schedule:
- cron: '5 0 * * *'
push:
branches:
- main
workflow_dispatch:
pull_request:
branches:
- main
paths:
- ".github/workflows/csv-coverage-metrics.yml"
jobs:
publish:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v2
- name: Setup CodeQL
uses: ./.github/actions/fetch-codeql
- name: Create empty database
run: |
DATABASE="${{ runner.temp }}/java-database"
PROJECT="${{ runner.temp }}/java-project"
mkdir -p "$PROJECT/src/tmp/empty"
echo "class Empty {}" >> "$PROJECT/src/tmp/empty/Empty.java"
codeql database create "$DATABASE" --language=java --source-root="$PROJECT" --command 'javac src/tmp/empty/Empty.java'
- name: Capture coverage information
run: |
DATABASE="${{ runner.temp }}/java-database"
codeql database analyze --format=sarif-latest --output=metrics.sarif -- "$DATABASE" ./java/ql/src/Metrics/Summaries/FrameworkCoverage.ql
- uses: actions/upload-artifact@v2
with:
name: metrics.sarif
path: metrics.sarif
retention-days: 20
- name: Upload SARIF file
uses: github/codeql-action/upload-sarif@v1
with:
sarif_file: metrics.sarif

View File

@@ -39,12 +39,6 @@ jobs:
- uses: ./.github/actions/fetch-codeql
- name: Install pack dependencies
run: |
for pack in modelbuilding src; do
codeql pack install --mode verify -- "${pack}"
done
- name: Check QL compilation
run: |
codeql query compile \
@@ -63,9 +57,6 @@ jobs:
- uses: ./.github/actions/fetch-codeql
- name: Install pack dependencies
run: codeql pack install -- test
- name: Run QL tests
run: |
codeql test run \

View File

@@ -31,13 +31,13 @@ jobs:
uses: actions/cache@v2
with:
path: ${{ runner.temp }}/query-pack.zip
key: queries-${{ hashFiles('ql/**/*.ql*') }}-${{ hashFiles('ql/**/qlpack.yml') }}-${{ hashFiles('ql/ql/src/ql.dbscheme*') }}-${{ steps.get-codeql-version.outputs.version }}
key: queries-${{ hashFiles('ql/**/*.ql*') }}-${{ hashFiles('ql/ql/src/ql.dbscheme*') }}-${{ steps.get-codeql-version.outputs.version }}
- name: Build query pack
if: steps.cache-queries.outputs.cache-hit != 'true'
run: |
cd ql/ql/src
"${CODEQL}" pack create
cd .codeql/pack/codeql/ql/0.0.0
cd .codeql/pack/codeql/ql-all/0.0.0
zip "${PACKZIP}" -r .
env:
CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}

View File

@@ -52,18 +52,9 @@ jobs:
codeql dataset import -S ql/lib/upgrades/initial/ruby.dbscheme testdb empty.trap
codeql dataset upgrade testdb --additional-packs ql/lib
diff -q testdb/ruby.dbscheme ql/lib/ruby.dbscheme
- name: Check DB downgrade scripts
run: |
echo >empty.trap
rm -rf testdb; codeql dataset import -S ql/lib/ruby.dbscheme testdb empty.trap
codeql resolve upgrades --format=lines --allow-downgrades --additional-packs downgrades \
--dbscheme=ql/lib/ruby.dbscheme --target-dbscheme=downgrades/initial/ruby.dbscheme |
xargs codeql execute upgrades testdb
diff -q testdb/ruby.dbscheme downgrades/initial/ruby.dbscheme
qltest:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
slice: ["1/2", "2/2"]
steps:

View File

@@ -1,29 +0,0 @@
name: Validate change notes
on:
push:
paths:
- "*/ql/*/change-notes/**/*"
- ".github/workflows/validate-change-notes.yml"
branches:
- main
- "rc/*"
pull_request:
paths:
- "*/ql/*/change-notes/**/*"
- ".github/workflows/validate-change-notes.yml"
jobs:
check-change-note:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v2
- name: Setup CodeQL
uses: ./.github/actions/fetch-codeql
- name: Fail if there are any errors with existing change notes
run: |
codeql pack release --groups cpp,csharp,java,javascript,python,ruby,-examples,-test,-experimental

View File

@@ -1,29 +0,0 @@
# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
exclude: /test/.*$(?<!\.ql)(?<!\.qll)(?<!\.qlref)
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v3.2.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
- repo: local
hooks:
- id: codeql-format
name: Fix QL file formatting
files: \.qll?$
language: system
entry: codeql query format --in-place
- id: sync-files
name: Fix files required to be identical
language: system
entry: python3 config/sync-files.py --latest
pass_filenames: false
- id: qhelp
name: Check query help generation
files: \.qhelp$
language: system
entry: python3 misc/scripts/check-qhelp.py

View File

@@ -42,11 +42,7 @@ If you have an idea for a query that you would like to share with other CodeQL u
- The queries and libraries must be autoformatted, for example using the "Format Document" command in [CodeQL for Visual Studio Code](https://help.semmle.com/codeql/codeql-for-vscode/procedures/about-codeql-for-vscode.html).
If you prefer, you can either:
1. install the [pre-commit framework](https://pre-commit.com/) and install the configured hooks on this repo via `pre-commit install`, or
2. use this [pre-commit hook](misc/scripts/pre-commit) that automatically checks whether your files are correctly formatted.
See the [pre-commit hook installation guide](docs/pre-commit-hook-setup.md) for instructions on the two approaches.
If you prefer, you can use this [pre-commit hook](misc/scripts/pre-commit) that automatically checks whether your files are correctly formatted. See the [pre-commit hook installation guide](docs/pre-commit-hook-setup.md) for instructions on how to install the hook.
4. **Compilation**
@@ -67,6 +63,6 @@ After the experimental query is merged, we welcome pull requests to improve it.
## Using your personal data
If you contribute to this project, we will record your name and email address (as provided by you with your contributions) as part of the code repositories, which are public. We might also use this information to contact you in relation to your contributions, as well as in the normal course of software development. We also store records of CLA agreements signed in the past, but no longer require contributors to sign a CLA. Under GDPR legislation, we do this on the basis of our legitimate interest in creating the CodeQL product.
If you contribute to this project, we will record your name and email address (as provided by you with your contributions) as part of the code repositories, which are public. We might also use this information to contact you in relation to your contributions, as well as in the normal course of software development. We also store records of CLA agreements signed in the past, but no longer require contributors to sign a CLA. Under GDPR legislation, we do this on the basis of our legitimate interest in creating the CodeQL product.
Please do get in touch (privacy@github.com) if you have any questions about this or our data protection policies.

51
benjamin-button.md Normal file
View File

@@ -0,0 +1,51 @@
# benjamin-buttons.md
This file describes the changes that have been applied to
the library to make it behave as if it was younger.
## TaintedPath.ql
Sinks added between 2020-01-01 and 2020-10-06 have been removed. Found by looking at:
- the commit titles of https://github.com/github/codeql/commits/main/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected
- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+sink
Sinks added between 2018-08-02 and 2020-01-01 have been removed. Found by looking at:
- the commit titles of https://github.com/github/codeql/commits/main/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected
- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+sink
- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+pathinjection
- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+tainted-path
Sinks from the "graceful-fs" and "fs-extra" (added before the open-sourcing squash).
## Xss.ql
Sinks added between 2020-01-01 and 2020-10-06 have been removed. Found by looking at:
- the commit titles of https://github.com/github/codeql/commits/main/javascript/ql/test/query-tests/Security/CWE-079/Xss.expected
- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+sink
- recursive type tracking for `jQuery::dollar`, `DOM::domValueRef`.
## SqlInjection.ql
Sinks added between 2020-01-01 and 2020-10-06 have been removed. Found by looking at:
- the commit titles of https://github.com/github/codeql/commits/main/javascript/ql/test/query-tests/Security/CWE-089
- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+sink
Sinks added between 2018-08-02 and 2020-01-01 have been removed. Found by looking at:
- the commit titles of https://github.com/github/codeql/commits/main/javascript/ql/test/query-tests/Security/CWE-089
- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+sink
- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+sql
TypeTracking in SQL.qll (added before the open-sourcing squash)
The model of `mssql` and `sequelize` (added before the open-sourcing squash)
## PseudoProperties
Pseudo-properties (`$name$`) used in type-tracking and global dataflow configurations have been disabled.
Found by searching for `"\$.*\$"`.

View File

@@ -426,6 +426,7 @@
"python/ql/src/Lexical/CommentedOutCodeMetricOverview.inc.qhelp"
],
"FLinesOfDuplicatedCodeCommon.inc.qhelp": [
"cpp/ql/src/Metrics/Files/FLinesOfDuplicatedCodeCommon.inc.qhelp",
"java/ql/src/Metrics/Files/FLinesOfDuplicatedCodeCommon.inc.qhelp",
"javascript/ql/src/Metrics/FLinesOfDuplicatedCodeCommon.inc.qhelp",
"python/ql/src/Metrics/FLinesOfDuplicatedCodeCommon.inc.qhelp"
@@ -464,8 +465,7 @@
],
"SensitiveDataHeuristics Python/JS": [
"javascript/ql/lib/semmle/javascript/security/internal/SensitiveDataHeuristics.qll",
"python/ql/lib/semmle/python/security/internal/SensitiveDataHeuristics.qll",
"ruby/ql/lib/codeql/ruby/security/internal/SensitiveDataHeuristics.qll"
"python/ql/lib/semmle/python/security/internal/SensitiveDataHeuristics.qll"
],
"ReDoS Util Python/JS/Ruby": [
"javascript/ql/lib/semmle/javascript/security/performance/ReDoSUtil.qll",
@@ -501,11 +501,5 @@
"javascript/ql/lib/tutorial.qll",
"python/ql/lib/tutorial.qll",
"ruby/ql/lib/tutorial.qll"
],
"AccessPathSyntax": [
"csharp/ql/lib/semmle/code/csharp/dataflow/internal/AccessPathSyntax.qll",
"java/ql/lib/semmle/code/java/dataflow/internal/AccessPathSyntax.qll",
"javascript/ql/lib/semmle/javascript/frameworks/data/internal/AccessPathSyntax.qll",
"ruby/ql/lib/codeql/ruby/dataflow/internal/AccessPathSyntax.qll"
]
}

View File

@@ -31,7 +31,6 @@
+ semmlecode-cpp-queries/Critical/NewArrayDeleteMismatch.ql: /Correctness/Common Errors
+ semmlecode-cpp-queries/Critical/NewDeleteArrayMismatch.ql: /Correctness/Common Errors
+ semmlecode-cpp-queries/Critical/NewFreeMismatch.ql: /Correctness/Common Errors
+ semmlecode-cpp-queries/Likely Bugs/Memory Management/UsingExpiredStackAddress.ql: /Correctness/Common Errors
# Use of Libraries
+ semmlecode-cpp-queries/Likely Bugs/Memory Management/SuspiciousCallToMemset.ql: /Correctness/Use of Libraries
+ semmlecode-cpp-queries/Likely Bugs/Memory Management/SuspiciousSizeof.ql: /Correctness/Use of Libraries

View File

@@ -34,7 +34,6 @@
+ semmlecode-cpp-queries/Critical/NewArrayDeleteMismatch.ql: /Correctness/Common Errors
+ semmlecode-cpp-queries/Critical/NewDeleteArrayMismatch.ql: /Correctness/Common Errors
+ semmlecode-cpp-queries/Critical/NewFreeMismatch.ql: /Correctness/Common Errors
+ semmlecode-cpp-queries/Likely Bugs/Memory Management/UsingExpiredStackAddress.ql: /Correctness/Common Errors
# Exceptions
+ semmlecode-cpp-queries/Best Practices/Exceptions/AccidentalRethrow.ql: /Correctness/Exceptions
+ semmlecode-cpp-queries/Best Practices/Exceptions/CatchingByValue.ql: /Correctness/Exceptions

View File

@@ -1,2 +0,0 @@
description: Remove unused legacy relations
compatibility: backwards

View File

@@ -1,3 +0,0 @@
description: Add relation for tracking variables from structured binding declarations
compatibility: full
is_structured_binding.rel: delete

File diff suppressed because it is too large Load Diff

View File

@@ -1,4 +0,0 @@
name: codeql/cpp-downgrades
groups: cpp
downgrades: .
library: true

View File

@@ -1,29 +1,3 @@
## 0.0.11
### Minor Analysis Improvements
* Many queries now support structured bindings, as structured bindings are now handled in the IR translation.
## 0.0.10
### New Features
* Added a `isStructuredBinding` predicate to the `Variable` class which holds when the variable is declared as part of a structured binding declaration.
## 0.0.9
## 0.0.8
### Deprecated APIs
* The `codeql/cpp-upgrades` CodeQL pack has been removed. All upgrades scripts have been merged into the `codeql/cpp-all` CodeQL pack.
### Minor Analysis Improvements
* `FormatLiteral::getMaxConvertedLength` now uses range analysis to provide a
more accurate length for integers formatted with `%x`
## 0.0.7
## 0.0.6

View File

@@ -0,0 +1,4 @@
---
category: deprecated
---
* The `codeql/cpp-upgrades` CodeQL pack has been removed. All upgrades scripts have been merged into the `codeql/cpp-all` CodeQL pack.

View File

@@ -0,0 +1,5 @@
---
category: minorAnalysis
---
* `FormatLiteral::getMaxConvertedLength` now uses range analysis to provide a
more accurate length for integers formatted with `%x`

View File

@@ -1,5 +0,0 @@
## 0.0.10
### New Features
* Added a `isStructuredBinding` predicate to the `Variable` class which holds when the variable is declared as part of a structured binding declaration.

View File

@@ -1,5 +0,0 @@
## 0.0.11
### Minor Analysis Improvements
* Many queries now support structured bindings, as structured bindings are now handled in the IR translation.

View File

@@ -1,10 +0,0 @@
## 0.0.8
### Deprecated APIs
* The `codeql/cpp-upgrades` CodeQL pack has been removed. All upgrades scripts have been merged into the `codeql/cpp-all` CodeQL pack.
### Minor Analysis Improvements
* `FormatLiteral::getMaxConvertedLength` now uses range analysis to provide a
more accurate length for integers formatted with `%x`

View File

@@ -1,2 +0,0 @@
## 0.0.9

View File

@@ -1,2 +1,2 @@
---
lastReleaseVersion: 0.0.11
lastReleaseVersion: 0.0.7

View File

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

View File

@@ -169,12 +169,6 @@ class Variable extends Declaration, @variable {
variable_instantiation(underlyingElement(this), unresolveElement(v))
}
/**
* Holds if this variable is declated as part of a structured binding
* declaration. For example, `x` in `auto [x, y] = ...`.
*/
predicate isStructuredBinding() { is_structured_binding(underlyingElement(this)) }
/**
* Holds if this is a compiler-generated variable. For example, a
* [range-based for loop](http://en.cppreference.com/w/cpp/language/range-for)

View File

@@ -11,10 +11,10 @@ import cpp
*/
bindingset[input]
int parseOctal(string input) {
input.regexpMatch("0[0-7]+") and
input.charAt(0) = "0" and
result =
strictsum(int ix |
ix in [1 .. input.length()]
ix in [0 .. input.length()]
|
8.pow(input.length() - (ix + 1)) * input.charAt(ix).toInt()
)

View File

@@ -353,9 +353,9 @@ module FlowVar_internal {
// indirection.
result = def.getAUse(v)
or
exists(SsaDefinition descendantDef |
this.getASuccessorSsaVar+() = TSsaVar(descendantDef, _) and
result = descendantDef.getAUse(v)
exists(SsaDefinition descendentDef |
this.getASuccessorSsaVar+() = TSsaVar(descendentDef, _) and
result = descendentDef.getAUse(v)
)
)
or

View File

@@ -241,8 +241,8 @@ private module Cached {
// For compatibility, send flow from arguments to parameters, even for
// functions with no body.
exists(FunctionCall call, int i |
sink.asExpr() = call.getArgument(pragma[only_bind_into](i)) and
result = resolveCall(call).getParameter(pragma[only_bind_into](i))
sink.asExpr() = call.getArgument(i) and
result = resolveCall(call).getParameter(i)
)
or
// For compatibility, send flow into a `Variable` if there is flow to any

View File

@@ -1,270 +0,0 @@
/**
* This file provides a library for inter-procedural must-flow data flow analysis.
* Unlike `DataFlow.qll`, the analysis provided by this file checks whether data _must_ flow
* from a source to a _sink_.
*/
private import cpp
import semmle.code.cpp.ir.dataflow.DataFlow
private import semmle.code.cpp.ir.IR
/**
* A configuration of a data flow analysis that performs must-flow analysis. This is different
* from `DataFlow.qll` which performs may-flow analysis (i.e., it finds paths where the source _may_
* flow to the sink).
*
* Like in `DataFlow.qll`, each use of the `MustFlow.qll` library must define its own unique extension
* of this abstract class. To create a configuration, extend this class with a subclass whose
* characteristic predicate is a unique singleton string and override `isSource`, `isSink` (and
* `isAdditionalFlowStep` if additional steps are required).
*/
abstract class MustFlowConfiguration extends string {
bindingset[this]
MustFlowConfiguration() { any() }
/**
* Holds if `source` is a relevant data flow source.
*/
abstract predicate isSource(DataFlow::Node source);
/**
* Holds if `sink` is a relevant data flow sink.
*/
abstract predicate isSink(DataFlow::Node sink);
/**
* Holds if the additional flow step from `node1` to `node2` must be taken
* into account in the analysis.
*/
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
/**
* Holds if data must flow from `source` to `sink` for this configuration.
*
* The corresponding paths are generated from the end-points and the graph
* included in the module `PathGraph`.
*/
final predicate hasFlowPath(MustFlowPathNode source, MustFlowPathSink sink) {
this.isSource(source.getNode()) and
source.getASuccessor+() = sink
}
}
/** Holds if `node` flows from a source. */
pragma[nomagic]
private predicate flowsFromSource(DataFlow::Node node, MustFlowConfiguration config) {
config.isSource(node)
or
exists(DataFlow::Node mid |
step(mid, node, config) and
flowsFromSource(mid, pragma[only_bind_into](config))
)
}
/** Holds if `node` flows to a sink. */
pragma[nomagic]
private predicate flowsToSink(DataFlow::Node node, MustFlowConfiguration config) {
flowsFromSource(node, pragma[only_bind_into](config)) and
(
config.isSink(node)
or
exists(DataFlow::Node mid |
step(node, mid, config) and
flowsToSink(mid, pragma[only_bind_into](config))
)
)
}
cached
private module Cached {
/** Holds if `p` is the `n`'th parameter of the non-virtual function `f`. */
private predicate parameterOf(Parameter p, Function f, int n) {
not f.isVirtual() and f.getParameter(n) = p
}
/**
* Holds if `instr` is the `n`'th argument to a call to the non-virtual function `f`, and
* `init` is the corresponding initialization instruction that receives the value of `instr` in `f`.
*/
private predicate flowIntoParameter(
Function f, int n, CallInstruction call, Instruction instr, InitializeParameterInstruction init
) {
not f.isVirtual() and
call.getPositionalArgument(n) = instr and
f = call.getStaticCallTarget() and
getEnclosingNonVirtualFunctionInitializeParameter(init, f) and
init.getParameter().getIndex() = pragma[only_bind_into](pragma[only_bind_out](n))
}
/**
* Holds if `instr` is an argument to a call to the function `f`, and `init` is the
* corresponding initialization instruction that receives the value of `instr` in `f`.
*/
pragma[noinline]
private predicate getPositionalArgumentInitParam(
CallInstruction call, Instruction instr, InitializeParameterInstruction init, Function f
) {
exists(int n |
parameterOf(_, f, n) and
flowIntoParameter(f, pragma[only_bind_into](pragma[only_bind_out](n)), call, instr, init)
)
}
/**
* Holds if `instr` is the qualifier to a call to the non-virtual function `f`, and
* `init` is the corresponding initialization instruction that receives the value of
* `instr` in `f`.
*/
pragma[noinline]
private predicate getThisArgumentInitParam(
CallInstruction call, Instruction instr, InitializeParameterInstruction init, Function f
) {
not f.isVirtual() and
call.getStaticCallTarget() = f and
getEnclosingNonVirtualFunctionInitializeParameter(init, f) and
call.getThisArgument() = instr and
init.getIRVariable() instanceof IRThisVariable
}
/** Holds if `f` is the enclosing non-virtual function of `init`. */
private predicate getEnclosingNonVirtualFunctionInitializeParameter(
InitializeParameterInstruction init, Function f
) {
not f.isVirtual() and
init.getEnclosingFunction() = f
}
/** Holds if `f` is the enclosing non-virtual function of `init`. */
private predicate getEnclosingNonVirtualFunctionInitializeIndirection(
InitializeIndirectionInstruction init, Function f
) {
not f.isVirtual() and
init.getEnclosingFunction() = f
}
/**
* Holds if `instr` is an argument (or argument indirection) to a call, and
* `succ` is the corresponding initialization instruction in the call target.
*/
private predicate flowThroughCallable(Instruction argument, Instruction parameter) {
// Flow from an argument to a parameter
exists(CallInstruction call, InitializeParameterInstruction init | init = parameter |
getPositionalArgumentInitParam(call, argument, init, call.getStaticCallTarget())
or
getThisArgumentInitParam(call, argument, init, call.getStaticCallTarget())
)
or
// Flow from argument indirection to parameter indirection
exists(
CallInstruction call, ReadSideEffectInstruction read, InitializeIndirectionInstruction init
|
init = parameter and
read.getPrimaryInstruction() = call and
getEnclosingNonVirtualFunctionInitializeIndirection(init, call.getStaticCallTarget())
|
exists(int n |
read.getSideEffectOperand().getAnyDef() = argument and
read.getIndex() = pragma[only_bind_into](n) and
init.getParameter().getIndex() = pragma[only_bind_into](n)
)
or
call.getThisArgument() = argument and
init.getIRVariable() instanceof IRThisVariable
)
}
private predicate instructionToOperandStep(Instruction instr, Operand operand) {
operand.getDef() = instr
}
/**
* Holds if data flows from `operand` to `instr`.
*
* This predicate ignores flow through `PhiInstruction`s to create a 'must flow' relation.
*/
private predicate operandToInstructionStep(Operand operand, Instruction instr) {
instr.(CopyInstruction).getSourceValueOperand() = operand
or
instr.(ConvertInstruction).getUnaryOperand() = operand
or
instr.(CheckedConvertOrNullInstruction).getUnaryOperand() = operand
or
instr.(InheritanceConversionInstruction).getUnaryOperand() = operand
or
instr.(ChiInstruction).getTotalOperand() = operand
}
cached
predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
instructionToOperandStep(nodeFrom.asInstruction(), nodeTo.asOperand())
or
flowThroughCallable(nodeFrom.asInstruction(), nodeTo.asInstruction())
or
operandToInstructionStep(nodeFrom.asOperand(), nodeTo.asInstruction())
}
}
/** Holds if `nodeFrom` flows to `nodeTo`. */
private predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo, MustFlowConfiguration config) {
exists(config) and
Cached::step(nodeFrom, nodeTo)
or
config.isAdditionalFlowStep(nodeFrom, nodeTo)
}
private newtype TLocalPathNode =
MkLocalPathNode(DataFlow::Node n, MustFlowConfiguration config) {
flowsToSink(n, config) and
(
config.isSource(n)
or
exists(MustFlowPathNode mid | step(mid.getNode(), n, config))
)
}
/** A `Node` that is in a path from a source to a sink. */
class MustFlowPathNode extends TLocalPathNode {
DataFlow::Node n;
MustFlowPathNode() { this = MkLocalPathNode(n, _) }
/** Gets the underlying node. */
DataFlow::Node getNode() { result = n }
/** Gets a textual representation of this node. */
string toString() { result = n.toString() }
/** Gets the location of this element. */
Location getLocation() { result = n.getLocation() }
/** Gets a successor node, if any. */
MustFlowPathNode getASuccessor() {
step(this.getNode(), result.getNode(), this.getConfiguration())
}
/** Gets the associated configuration. */
MustFlowConfiguration getConfiguration() { this = MkLocalPathNode(_, result) }
}
private class MustFlowPathSink extends MustFlowPathNode {
MustFlowPathSink() { this.getConfiguration().isSink(this.getNode()) }
}
/**
* Provides the query predicates needed to include a graph in a path-problem query.
*/
module PathGraph {
private predicate reach(MustFlowPathNode n) {
n instanceof MustFlowPathSink or reach(n.getASuccessor())
}
/** Holds if `(a,b)` is an edge in the graph of data flow path explanations. */
query predicate edges(MustFlowPathNode a, MustFlowPathNode b) {
a.getASuccessor() = b and reach(b)
}
/** Holds if `n` is a node in the graph of data flow path explanations. */
query predicate nodes(MustFlowPathNode n, string key, string val) {
reach(n) and key = "semmle.label" and val = n.toString()
}
}

View File

@@ -200,7 +200,7 @@ class IRBlock extends IRBlockBase {
* post-dominate block `B`, but block `A` does post-dominate an immediate successor of block `B`.
*/
pragma[noinline]
final IRBlock postDominanceFrontier() {
final IRBlock postPominanceFrontier() {
this.postDominates(result.getASuccessor()) and
not this.strictlyPostDominates(result)
}

View File

@@ -106,12 +106,6 @@ private predicate filteredNumberableInstruction(Instruction instr) {
or
instr instanceof FieldAddressInstruction and
count(instr.(FieldAddressInstruction).getField()) != 1
or
instr instanceof InheritanceConversionInstruction and
(
count(instr.(InheritanceConversionInstruction).getBaseClass()) != 1 or
count(instr.(InheritanceConversionInstruction).getDerivedClass()) != 1
)
}
private predicate variableAddressValueNumber(
@@ -121,7 +115,8 @@ private predicate variableAddressValueNumber(
// The underlying AST element is used as value-numbering key instead of the
// `IRVariable` to work around a problem where a variable or expression with
// multiple types gives rise to multiple `IRVariable`s.
unique( | | instr.getIRVariable().getAST()) = ast
instr.getIRVariable().getAST() = ast and
strictcount(instr.getIRVariable().getAST()) = 1
}
private predicate initializeParameterValueNumber(
@@ -138,7 +133,8 @@ private predicate constantValueNumber(
ConstantInstruction instr, IRFunction irFunc, IRType type, string value
) {
instr.getEnclosingIRFunction() = irFunc and
unique( | | instr.getResultIRType()) = type and
strictcount(instr.getResultIRType()) = 1 and
instr.getResultIRType() = type and
instr.getValue() = value
}
@@ -155,7 +151,8 @@ private predicate fieldAddressValueNumber(
TValueNumber objectAddress
) {
instr.getEnclosingIRFunction() = irFunc and
unique( | | instr.getField()) = field and
instr.getField() = field and
strictcount(instr.getField()) = 1 and
tvalueNumber(instr.getObjectAddress()) = objectAddress
}
@@ -198,9 +195,9 @@ private predicate inheritanceConversionValueNumber(
) {
instr.getEnclosingIRFunction() = irFunc and
instr.getOpcode() = opcode and
tvalueNumber(instr.getUnary()) = operand and
unique( | | instr.getBaseClass()) = baseClass and
unique( | | instr.getDerivedClass()) = derivedClass
instr.getBaseClass() = baseClass and
instr.getDerivedClass() = derivedClass and
tvalueNumber(instr.getUnary()) = operand
}
private predicate loadTotalOverlapValueNumber(

View File

@@ -200,7 +200,7 @@ class IRBlock extends IRBlockBase {
* post-dominate block `B`, but block `A` does post-dominate an immediate successor of block `B`.
*/
pragma[noinline]
final IRBlock postDominanceFrontier() {
final IRBlock postPominanceFrontier() {
this.postDominates(result.getASuccessor()) and
not this.strictlyPostDominates(result)
}

View File

@@ -106,12 +106,6 @@ private predicate filteredNumberableInstruction(Instruction instr) {
or
instr instanceof FieldAddressInstruction and
count(instr.(FieldAddressInstruction).getField()) != 1
or
instr instanceof InheritanceConversionInstruction and
(
count(instr.(InheritanceConversionInstruction).getBaseClass()) != 1 or
count(instr.(InheritanceConversionInstruction).getDerivedClass()) != 1
)
}
private predicate variableAddressValueNumber(
@@ -121,7 +115,8 @@ private predicate variableAddressValueNumber(
// The underlying AST element is used as value-numbering key instead of the
// `IRVariable` to work around a problem where a variable or expression with
// multiple types gives rise to multiple `IRVariable`s.
unique( | | instr.getIRVariable().getAST()) = ast
instr.getIRVariable().getAST() = ast and
strictcount(instr.getIRVariable().getAST()) = 1
}
private predicate initializeParameterValueNumber(
@@ -138,7 +133,8 @@ private predicate constantValueNumber(
ConstantInstruction instr, IRFunction irFunc, IRType type, string value
) {
instr.getEnclosingIRFunction() = irFunc and
unique( | | instr.getResultIRType()) = type and
strictcount(instr.getResultIRType()) = 1 and
instr.getResultIRType() = type and
instr.getValue() = value
}
@@ -155,7 +151,8 @@ private predicate fieldAddressValueNumber(
TValueNumber objectAddress
) {
instr.getEnclosingIRFunction() = irFunc and
unique( | | instr.getField()) = field and
instr.getField() = field and
strictcount(instr.getField()) = 1 and
tvalueNumber(instr.getObjectAddress()) = objectAddress
}
@@ -198,9 +195,9 @@ private predicate inheritanceConversionValueNumber(
) {
instr.getEnclosingIRFunction() = irFunc and
instr.getOpcode() = opcode and
tvalueNumber(instr.getUnary()) = operand and
unique( | | instr.getBaseClass()) = baseClass and
unique( | | instr.getDerivedClass()) = derivedClass
instr.getBaseClass() = baseClass and
instr.getDerivedClass() = derivedClass and
tvalueNumber(instr.getUnary()) = operand
}
private predicate loadTotalOverlapValueNumber(

View File

@@ -71,8 +71,7 @@ newtype TInstructionTag =
AsmTag() or
AsmInputTag(int elementIndex) { exists(AsmStmt asm | exists(asm.getChild(elementIndex))) } or
ThisAddressTag() or
ThisLoadTag() or
StructuredBindingAccessTag()
ThisLoadTag()
class InstructionTag extends TInstructionTag {
final string toString() { result = "Tag" }
@@ -222,6 +221,4 @@ string getInstructionTagId(TInstructionTag tag) {
tag = ThisAddressTag() and result = "ThisAddress"
or
tag = ThisLoadTag() and result = "ThisLoad"
or
tag = StructuredBindingAccessTag() and result = "StructuredBindingAccess"
}

View File

@@ -3,7 +3,6 @@ private import semmle.code.cpp.ir.implementation.IRType
private import semmle.code.cpp.ir.implementation.Opcode
private import semmle.code.cpp.ir.implementation.internal.OperandTag
private import semmle.code.cpp.ir.internal.CppType
private import semmle.code.cpp.ir.internal.IRUtilities
private import semmle.code.cpp.ir.internal.TempVariableTag
private import InstructionTag
private import TranslatedCondition
@@ -814,9 +813,7 @@ abstract class TranslatedVariableAccess extends TranslatedNonConstantExpr {
}
class TranslatedNonFieldVariableAccess extends TranslatedVariableAccess {
TranslatedNonFieldVariableAccess() {
not expr instanceof FieldAccess and not isNonReferenceStructuredBinding(expr.getTarget())
}
TranslatedNonFieldVariableAccess() { not expr instanceof FieldAccess }
override Instruction getFirstInstruction() {
if exists(this.getQualifier())
@@ -863,71 +860,6 @@ class TranslatedFieldAccess extends TranslatedVariableAccess {
}
}
/**
* The IR translation of a variable access of a structured binding, where the type
* of the structured binding is not of a reference type, e.g., `x0` and `x1`
* in `auto [x0, x1] = xs` where `xs` is an array. Although the type of the
* structured binding is a non-reference type, the structured binding behaves
* like a reference. Hence, the translation requires a `VariableAddress` followed
* by a `Load` instead of only a `VariableAddress` as produced by
* `TranslatedVariableAccess`.
*/
class TranslatedStructuredBindingVariableAccess extends TranslatedNonConstantExpr {
override VariableAccess expr;
TranslatedStructuredBindingVariableAccess() { isNonReferenceStructuredBinding(expr.getTarget()) }
override Instruction getFirstInstruction() {
// Structured bindings cannot be qualified.
result = this.getInstruction(StructuredBindingAccessTag())
}
override TranslatedElement getChild(int id) {
// Structured bindings cannot be qualified.
none()
}
override Instruction getResult() { result = this.getInstruction(LoadTag()) }
override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
tag = StructuredBindingAccessTag() and
kind instanceof GotoEdge and
result = this.getInstruction(LoadTag())
or
tag = LoadTag() and
kind instanceof GotoEdge and
result = this.getParent().getChildSuccessor(this)
}
override Instruction getChildSuccessor(TranslatedElement child) { none() }
override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
tag = StructuredBindingAccessTag() and
opcode instanceof Opcode::VariableAddress and
resultType = getTypeForGLValue(this.getLValueReferenceType())
or
tag = LoadTag() and
opcode instanceof Opcode::Load and
resultType = getTypeForPRValue(this.getLValueReferenceType())
}
private LValueReferenceType getLValueReferenceType() {
// The extractor ensures `result` exists when `isNonReferenceStructuredBinding(expr.getTarget())` holds.
result.getBaseType() = expr.getUnspecifiedType()
}
override Instruction getInstructionRegisterOperand(InstructionTag tag, OperandTag operandTag) {
tag = LoadTag() and
operandTag instanceof AddressOperandTag and
result = this.getInstruction(StructuredBindingAccessTag())
}
override IRVariable getInstructionVariable(InstructionTag tag) {
tag = StructuredBindingAccessTag() and
result = getIRUserVariable(expr.getEnclosingFunction(), expr.getTarget())
}
}
class TranslatedFunctionAccess extends TranslatedNonConstantExpr {
override FunctionAccess expr;

View File

@@ -200,7 +200,7 @@ class IRBlock extends IRBlockBase {
* post-dominate block `B`, but block `A` does post-dominate an immediate successor of block `B`.
*/
pragma[noinline]
final IRBlock postDominanceFrontier() {
final IRBlock postPominanceFrontier() {
this.postDominates(result.getASuccessor()) and
not this.strictlyPostDominates(result)
}

View File

@@ -106,12 +106,6 @@ private predicate filteredNumberableInstruction(Instruction instr) {
or
instr instanceof FieldAddressInstruction and
count(instr.(FieldAddressInstruction).getField()) != 1
or
instr instanceof InheritanceConversionInstruction and
(
count(instr.(InheritanceConversionInstruction).getBaseClass()) != 1 or
count(instr.(InheritanceConversionInstruction).getDerivedClass()) != 1
)
}
private predicate variableAddressValueNumber(
@@ -121,7 +115,8 @@ private predicate variableAddressValueNumber(
// The underlying AST element is used as value-numbering key instead of the
// `IRVariable` to work around a problem where a variable or expression with
// multiple types gives rise to multiple `IRVariable`s.
unique( | | instr.getIRVariable().getAST()) = ast
instr.getIRVariable().getAST() = ast and
strictcount(instr.getIRVariable().getAST()) = 1
}
private predicate initializeParameterValueNumber(
@@ -138,7 +133,8 @@ private predicate constantValueNumber(
ConstantInstruction instr, IRFunction irFunc, IRType type, string value
) {
instr.getEnclosingIRFunction() = irFunc and
unique( | | instr.getResultIRType()) = type and
strictcount(instr.getResultIRType()) = 1 and
instr.getResultIRType() = type and
instr.getValue() = value
}
@@ -155,7 +151,8 @@ private predicate fieldAddressValueNumber(
TValueNumber objectAddress
) {
instr.getEnclosingIRFunction() = irFunc and
unique( | | instr.getField()) = field and
instr.getField() = field and
strictcount(instr.getField()) = 1 and
tvalueNumber(instr.getObjectAddress()) = objectAddress
}
@@ -198,9 +195,9 @@ private predicate inheritanceConversionValueNumber(
) {
instr.getEnclosingIRFunction() = irFunc and
instr.getOpcode() = opcode and
tvalueNumber(instr.getUnary()) = operand and
unique( | | instr.getBaseClass()) = baseClass and
unique( | | instr.getDerivedClass()) = derivedClass
instr.getBaseClass() = baseClass and
instr.getDerivedClass() = derivedClass and
tvalueNumber(instr.getUnary()) = operand
}
private predicate loadTotalOverlapValueNumber(

View File

@@ -11,15 +11,6 @@ private Type getDecayedType(Type type) {
result.(PointerType).getBaseType() = type.(ArrayType).getBaseType()
}
/**
* Holds if the sepcified variable is a structured binding with a non-reference
* type.
*/
predicate isNonReferenceStructuredBinding(Variable v) {
v.isStructuredBinding() and
not v.getUnspecifiedType() instanceof ReferenceType
}
/**
* Get the actual type of the specified variable, as opposed to the declared type.
* This returns the type of the variable after any pointer decay is applied, and
@@ -39,12 +30,7 @@ Type getVariableType(Variable v) {
result = v.getInitializer().getExpr().getType()
or
not exists(v.getInitializer()) and result = v.getType()
else
if isNonReferenceStructuredBinding(v)
then
// The extractor ensures `r` exists when `isNonReferenceStructuredBinding(v)` holds.
exists(LValueReferenceType r | r.getBaseType() = v.getUnspecifiedType() | result = r)
else result = v.getType()
else result = v.getType()
)
}

View File

@@ -135,11 +135,52 @@ externalData(
string value : string ref
);
/**
* The date of the snapshot.
*/
snapshotDate(unique date snapshotDate : date ref);
/**
* The source location of the snapshot.
*/
sourceLocationPrefix(string prefix : string ref);
/**
* Data used by the 'duplicate code' detection.
*/
duplicateCode(
unique int id : @duplication,
string relativePath : string ref,
int equivClass : int ref
);
/**
* Data used by the 'similar code' detection.
*/
similarCode(
unique int id : @similarity,
string relativePath : string ref,
int equivClass : int ref
);
/**
* Data used by the 'duplicate code' and 'similar code' detection.
*/
@duplication_or_similarity = @duplication | @similarity
/**
* Data used by the 'duplicate code' and 'similar code' detection.
*/
#keyset[id, offset]
tokens(
int id : @duplication_or_similarity ref,
int offset : int ref,
int beginLine : int ref,
int beginColumn : int ref,
int endLine : int ref,
int endColumn : int ref
);
/**
* Information about packages that provide code used during compilation.
* The `id` is just a unique identifier.
@@ -446,7 +487,6 @@ var_decl_specifiers(
int id: @var_decl ref,
string name: string ref
)
is_structured_binding(unique int id: @variable ref);
type_decls(
unique int id: @type_decl,

File diff suppressed because it is too large Load Diff

View File

@@ -1,6 +0,0 @@
description: Remove unused legacy relations
compatibility: full
snapshotDate.rel: delete
duplicateCode.rel: delete
similarCode.rel: delete
tokens.rel: delete

View File

@@ -1,2 +0,0 @@
description: Add relation for tracking variables from structured binding declarations
compatibility: backwards

File diff suppressed because it is too large Load Diff

View File

@@ -18,6 +18,7 @@ where
not lv1.isCompilerGenerated() and
not lv2.isCompilerGenerated() and
not lv1.getParentScope().(BlockStmt).isInMacroExpansion() and
not lv2.getParentScope().(BlockStmt).isInMacroExpansion()
not lv2.getParentScope().(BlockStmt).isInMacroExpansion() and
not lv1.getName() = "(unnamed local variable)"
select lv1, "Variable " + lv1.getName() + " hides another variable of the same name (on $@).", lv2,
"line " + lv2.getLocation().getStartLine().toString()

View File

@@ -1,63 +1,3 @@
## 0.0.11
### Breaking Changes
* The deprecated queries `cpp/duplicate-block`, `cpp/duplicate-function`, `cpp/duplicate-class`, `cpp/duplicate-file`, `cpp/mostly-duplicate-function`,`cpp/similar-file`, `cpp/duplicated-lines-in-files` have been removed.
### Deprecated Predicates and Classes
* The predicates and classes in the `CodeDuplication` library have been deprecated.
### New Queries
* A new query titled "Use of expired stack-address" (`cpp/using-expired-stack-address`) has been added.
This query finds accesses to expired stack-allocated memory that escaped via a global variable.
* A new `cpp/insufficient-key-size` query has been added to the default query suite for C/C++. The query finds uses of certain cryptographic algorithms where the key size is too small to provide adequate encryption strength.
### Minor Analysis Improvements
* The "Failure to use HTTPS URLs" (`cpp/non-https-url`) has been improved reducing false positive results, and its precision has been increased to 'high'.
* The `cpp/system-data-exposure` query has been modernized and has converted to a `path-problem` query. There are now fewer false positive results.
## 0.0.10
### Deprecated Classes
* The `CodeDuplication.Copy`, `CodeDuplication.DuplicateBlock`, and `CodeDuplication.SimilarBlock` classes have been deprecated.
## 0.0.9
### New Queries
* Added a new query, `cpp/open-call-with-mode-argument`, to detect when `open` or `openat` is called with the `O_CREAT` or `O_TMPFILE` flag but when the `mode` argument is omitted.
### Minor Analysis Improvements
* The "Cleartext transmission of sensitive information" (`cpp/cleartext-transmission`) query has been further improved to reduce false positive results, and upgraded from `medium` to `high` precision.
* The "Cleartext transmission of sensitive information" (`cpp/cleartext-transmission`) query now finds more results, where a password is stored in a struct field or class member variable.
* The `cpp/cleartext-storage-file` query has been improved, removing false positives where data is written to a standard output stream.
* The `cpp/cleartext-storage-buffer` query has been updated to use the `semmle.code.cpp.dataflow.TaintTracking` library.
* The `cpp/world-writable-file-creation` query now only detects `open` and `openat` calls with the `O_CREAT` or `O_TMPFILE` flag.
## 0.0.8
### New Queries
* The `security` tag has been added to the `cpp/return-stack-allocated-memory` query. As a result, its results will now appear by default.
* The "Uncontrolled data in arithmetic expression" (cpp/uncontrolled-arithmetic) query has been enhanced to reduce false positive results and its @precision increased to high.
* A new `cpp/very-likely-overruning-write` query has been added to the default query suite for C/C++. The query reports some results that were formerly flagged by `cpp/overruning-write`.
### Minor Analysis Improvements
* Fix an issue with the `cpp/declaration-hides-variable` query where it would report variables that are unnamed in a database.
* The `cpp/cleartext-storage-file` query has been upgraded with non-local taint flow and has been converted to a `path-problem` query.
* The `cpp/return-stack-allocated-memory` query has been improved to produce fewer false positives. The
query has also been converted to a `path-problem` query.
* The "Cleartext transmission of sensitive information" (`cpp/cleartext-transmission`) query has been improved in several ways to reduce false positive results.
* The "Potential improper null termination" (`cpp/improper-null-termination`) query now produces fewer false positive results around control flow branches and loops.
* Added exception for GLib's gboolean to cpp/ambiguously-signed-bit-field.
This change reduces the number of false positives in the query.
## 0.0.7
## 0.0.6

View File

@@ -18,71 +18,157 @@ import cpp
// recomputing the IR.
private import semmle.code.cpp.valuenumbering.GlobalValueNumbering
import semmle.code.cpp.ir.IR
import semmle.code.cpp.ir.dataflow.MustFlow
import PathGraph
import semmle.code.cpp.ir.dataflow.DataFlow::DataFlow
/** Holds if `f` has a name that we intrepret as evidence of intentionally returning the value of the stack pointer. */
predicate intentionallyReturnsStackPointer(Function f) {
f.getName().toLowerCase().matches(["%stack%", "%sp%"])
}
class ReturnStackAllocatedMemoryConfig extends MustFlowConfiguration {
ReturnStackAllocatedMemoryConfig() { this = "ReturnStackAllocatedMemoryConfig" }
override predicate isSource(DataFlow::Node source) {
// Holds if `source` is a node that represents the use of a stack variable
exists(VariableAddressInstruction var, Function func |
var = source.asInstruction() and
func = var.getEnclosingFunction() and
var.getASTVariable() instanceof StackVariable and
// Pointer-to-member types aren't properly handled in the dbscheme.
not var.getResultType() instanceof PointerToMemberType and
// Rule out FPs caused by extraction errors.
not any(ErrorExpr e).getEnclosingFunction() = func and
not intentionallyReturnsStackPointer(func)
)
}
override predicate isSink(DataFlow::Node sink) {
// Holds if `sink` is a node that represents the `StoreInstruction` that is subsequently used in
// a `ReturnValueInstruction`.
// We use the `StoreInstruction` instead of the instruction that defines the
// `ReturnValueInstruction`'s source value oprand because the former has better location information.
exists(StoreInstruction store |
store.getDestinationAddress().(VariableAddressInstruction).getIRVariable() instanceof
IRReturnVariable and
sink.asOperand() = store.getSourceValueOperand()
)
}
/**
* This configuration intentionally conflates addresses of fields and their object, and pointer offsets
* with their base pointer as this allows us to detect cases where an object's address flows to a
* return statement via a field. For example:
*
* ```cpp
* struct S { int x, y };
* int* test() {
* S s;
* return &s.x; // BAD: &s.x is an address of a variable on the stack.
* }
* ```
*/
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
node2.asInstruction().(FieldAddressInstruction).getObjectAddressOperand() = node1.asOperand()
or
node2.asInstruction().(PointerOffsetInstruction).getLeftOperand() = node1.asOperand()
}
/**
* Holds if `source` is a node that represents the use of a stack variable
*/
predicate isSource(Node source) {
exists(VariableAddressInstruction var, Function func |
var = source.asInstruction() and
func = var.getEnclosingFunction() and
var.getASTVariable() instanceof StackVariable and
// Pointer-to-member types aren't properly handled in the dbscheme.
not var.getResultType() instanceof PointerToMemberType and
// Rule out FPs caused by extraction errors.
not any(ErrorExpr e).getEnclosingFunction() = func and
not intentionallyReturnsStackPointer(func)
)
}
from
MustFlowPathNode source, MustFlowPathNode sink, VariableAddressInstruction var,
ReturnStackAllocatedMemoryConfig conf
/**
* Holds if `sink` is a node that represents the `StoreInstruction` that is subsequently used in
* a `ReturnValueInstruction`. We use the `StoreInstruction` instead of the instruction that defines the
* `ReturnValueInstruction`'s source value oprand because the former has better location information.
*/
predicate isSink(Node sink) {
exists(StoreInstruction store |
store.getDestinationAddress().(VariableAddressInstruction).getIRVariable() instanceof
IRReturnVariable and
sink.asOperand() = store.getSourceValueOperand()
)
}
/** Holds if `node1` _must_ flow to `node2`. */
predicate step(Node node1, Node node2) {
instructionToOperandStep(node1.asInstruction(), node2.asOperand())
or
operandToInstructionStep(node1.asOperand(), node2.asInstruction())
}
predicate instructionToOperandStep(Instruction instr, Operand operand) { operand.getDef() = instr }
/**
* Holds if `operand` flows to the result of `instr`.
*
* This predicate ignores flow through `PhiInstruction`s to create a 'must flow' relation. It also
* intentionally conflates addresses of fields and their object, and pointer offsets with their
* base pointer as this allows us to detect cases where an object's address flows to a return statement
* via a field. For example:
*
* ```cpp
* struct S { int x, y };
* int* test() {
* S s;
* return &s.x; // BAD: &s.x is an address of a variable on the stack.
* }
* ```
*/
predicate operandToInstructionStep(Operand operand, Instruction instr) {
instr.(CopyInstruction).getSourceValueOperand() = operand
or
instr.(ConvertInstruction).getUnaryOperand() = operand
or
instr.(CheckedConvertOrNullInstruction).getUnaryOperand() = operand
or
instr.(InheritanceConversionInstruction).getUnaryOperand() = operand
or
instr.(FieldAddressInstruction).getObjectAddressOperand() = operand
or
instr.(PointerOffsetInstruction).getLeftOperand() = operand
}
/** Holds if a source node flows to `n`. */
predicate branchlessLocalFlow0(Node n) {
isSource(n)
or
exists(Node mid |
branchlessLocalFlow0(mid) and
step(mid, n)
)
}
/** Holds if `n` is reachable through some source node, and `n` also eventually reaches a sink. */
predicate branchlessLocalFlow1(Node n) {
branchlessLocalFlow0(n) and
(
isSink(n)
or
exists(Node mid |
branchlessLocalFlow1(mid) and
step(n, mid)
)
)
}
newtype TLocalPathNode =
TLocalPathNodeMid(Node n) {
branchlessLocalFlow1(n) and
(
isSource(n) or
exists(LocalPathNodeMid mid | step(mid.getNode(), n))
)
}
abstract class LocalPathNode extends TLocalPathNode {
Node n;
/** Gets the underlying node. */
Node getNode() { result = n }
/** Gets a textual representation of this node. */
string toString() { result = n.toString() }
/** Gets the location of this element. */
Location getLocation() { result = n.getLocation() }
/** Gets a successor `LocalPathNode`, if any. */
LocalPathNode getASuccessor() { step(this.getNode(), result.getNode()) }
}
class LocalPathNodeMid extends LocalPathNode, TLocalPathNodeMid {
LocalPathNodeMid() { this = TLocalPathNodeMid(n) }
}
class LocalPathNodeSink extends LocalPathNodeMid {
LocalPathNodeSink() { isSink(this.getNode()) }
}
/**
* Holds if `source` is a source node, `sink` is a sink node, and there's flow
* from `source` to `sink` using `step` relation.
*/
predicate hasFlow(LocalPathNode source, LocalPathNodeSink sink) {
isSource(source.getNode()) and
source.getASuccessor+() = sink
}
predicate reach(LocalPathNode n) { n instanceof LocalPathNodeSink or reach(n.getASuccessor()) }
query predicate edges(LocalPathNode a, LocalPathNode b) { a.getASuccessor() = b and reach(b) }
query predicate nodes(LocalPathNode n, string key, string val) {
reach(n) and key = "semmle.label" and val = n.toString()
}
from LocalPathNode source, LocalPathNodeSink sink, VariableAddressInstruction var
where
conf.hasFlowPath(source, sink) and
source.getNode().asInstruction() = var and
// Only raise an alert if we're returning from the _same_ callable as the on that
// declared the stack variable.
var.getEnclosingFunction() = sink.getNode().getEnclosingCallable()
hasFlow(source, sink) and
source.getNode().asInstruction() = var
select sink.getNode(), source, sink, "May return stack-allocated memory from $@.", var.getAST(),
var.getAST().toString()

View File

@@ -1,25 +0,0 @@
static const int* xptr;
void localAddressEscapes() {
int x = 0;
xptr = &x;
}
void example1() {
localAddressEscapes();
const int* x = xptr; // BAD: This pointer points to expired stack allocated memory.
}
void localAddressDoesNotEscape() {
int x = 0;
xptr = &x;
// ...
// use `xptr`
// ...
xptr = nullptr;
}
void example2() {
localAddressDoesNotEscape();
const int* x = xptr; // GOOD: This pointer does not point to expired memory.
}

View File

@@ -1,49 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
This rule finds uses of pointers that likely point to local variables in
expired stack frames. A pointer to a local variable is only valid
until the function returns, after which it becomes a dangling pointer.
</p>
</overview>
<recommendation>
<ol>
<li>
If it is necessary to take the address of a local variable, then make
sure that the address is only stored in memory that does not outlive
the local variable. For example, it is safe to store the address in
another local variable. Similarly, it is also safe to pass the address
of a local variable to another function provided that the other
function only uses it locally and does not store it in non-local
memory.
</li>
<li>
If it is necessary to store an address which will outlive the
current function scope, then it should be allocated on the heap. Care
should be taken to make sure that the memory is deallocated when it is
no longer needed, particularly when using low-level memory management
routines such as <tt>malloc</tt>/<tt>free</tt> or
<tt>new</tt>/<tt>delete</tt>. Modern C++ applications often use smart
pointers, such as <tt>std::shared_ptr</tt>, to reduce the chance of
a memory leak.
</li>
</ol>
</recommendation>
<example>
<sample src="UsingExpiredStackAddress.cpp" />
</example>
<references>
<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/Dangling_pointer">Dangling pointer</a>.</li>
</references>
</qhelp>

View File

@@ -1,348 +0,0 @@
/**
* @name Use of expired stack-address
* @description Accessing the stack-allocated memory of a function
* after it has returned can lead to memory corruption.
* @kind path-problem
* @problem.severity error
* @security-severity 9.3
* @precision high
* @id cpp/using-expired-stack-address
* @tags reliability
* security
* external/cwe/cwe-825
*/
import cpp
// We don't actually use the global value numbering library in this query, but without it we end up
// recomputing the IR.
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
import semmle.code.cpp.ir.IR
predicate instructionHasVariable(VariableAddressInstruction vai, StackVariable var, Function f) {
var = vai.getASTVariable() and
f = vai.getEnclosingFunction() and
// Pointer-to-member types aren't properly handled in the dbscheme.
not vai.getResultType() instanceof PointerToMemberType and
// Rule out FPs caused by extraction errors.
not any(ErrorExpr e).getEnclosingFunction() = f
}
/**
* Holds if `source` is the base address of an address computation whose
* result is stored in `address`.
*/
predicate stackPointerFlowsToUse(Instruction address, VariableAddressInstruction source) {
address = source and
instructionHasVariable(source, _, _)
or
stackPointerFlowsToUse(address.(CopyInstruction).getSourceValue(), source)
or
stackPointerFlowsToUse(address.(ConvertInstruction).getUnary(), source)
or
stackPointerFlowsToUse(address.(CheckedConvertOrNullInstruction).getUnary(), source)
or
stackPointerFlowsToUse(address.(InheritanceConversionInstruction).getUnary(), source)
or
stackPointerFlowsToUse(address.(FieldAddressInstruction).getObjectAddress(), source)
or
stackPointerFlowsToUse(address.(PointerOffsetInstruction).getLeft(), source)
}
/**
* A HashCons-like table for comparing addresses that are
* computed relative to some global variable.
*/
newtype TGlobalAddress =
TGlobalVariable(GlobalOrNamespaceVariable v) {
// Pointer-to-member types aren't properly handled in the dbscheme.
not v.getUnspecifiedType() instanceof PointerToMemberType
} or
TLoad(TGlobalAddress address) {
address = globalAddress(any(LoadInstruction load).getSourceAddress())
} or
TConversion(string kind, TGlobalAddress address, Type fromType, Type toType) {
kind = "unchecked" and
exists(ConvertInstruction convert |
uncheckedConversionTypes(convert, fromType, toType) and
address = globalAddress(convert.getUnary())
)
or
kind = "checked" and
exists(CheckedConvertOrNullInstruction convert |
checkedConversionTypes(convert, fromType, toType) and
address = globalAddress(convert.getUnary())
)
or
kind = "inheritance" and
exists(InheritanceConversionInstruction convert |
inheritanceConversionTypes(convert, fromType, toType) and
address = globalAddress(convert.getUnary())
)
} or
TFieldAddress(TGlobalAddress address, Field f) {
exists(FieldAddressInstruction fai |
fai.getField() = f and
address = globalAddress(fai.getObjectAddress())
)
}
pragma[noinline]
predicate uncheckedConversionTypes(ConvertInstruction convert, Type fromType, Type toType) {
fromType = convert.getUnary().getResultType() and
toType = convert.getResultType()
}
pragma[noinline]
predicate checkedConversionTypes(CheckedConvertOrNullInstruction convert, Type fromType, Type toType) {
fromType = convert.getUnary().getResultType() and
toType = convert.getResultType()
}
pragma[noinline]
predicate inheritanceConversionTypes(
InheritanceConversionInstruction convert, Type fromType, Type toType
) {
fromType = convert.getUnary().getResultType() and
toType = convert.getResultType()
}
/** Gets the HashCons value of an address computed by `instr`, if any. */
TGlobalAddress globalAddress(Instruction instr) {
result = TGlobalVariable(instr.(VariableAddressInstruction).getASTVariable())
or
not instr instanceof LoadInstruction and
result = globalAddress(instr.(CopyInstruction).getSourceValue())
or
exists(LoadInstruction load | instr = load |
result = TLoad(globalAddress(load.getSourceAddress()))
)
or
exists(ConvertInstruction convert, Type fromType, Type toType | instr = convert |
uncheckedConversionTypes(convert, fromType, toType) and
result = TConversion("unchecked", globalAddress(convert.getUnary()), fromType, toType)
)
or
exists(CheckedConvertOrNullInstruction convert, Type fromType, Type toType | instr = convert |
checkedConversionTypes(convert, fromType, toType) and
result = TConversion("checked", globalAddress(convert.getUnary()), fromType, toType)
)
or
exists(InheritanceConversionInstruction convert, Type fromType, Type toType | instr = convert |
inheritanceConversionTypes(convert, fromType, toType) and
result = TConversion("inheritance", globalAddress(convert.getUnary()), fromType, toType)
)
or
exists(FieldAddressInstruction fai | instr = fai |
result = TFieldAddress(globalAddress(fai.getObjectAddress()), fai.getField())
)
or
result = globalAddress(instr.(PointerOffsetInstruction).getLeft())
}
/** Gets a `StoreInstruction` that may be executed after executing `store`. */
pragma[inline]
StoreInstruction getAStoreStrictlyAfter(StoreInstruction store) {
exists(IRBlock block, int index1, int index2 |
block.getInstruction(index1) = store and
block.getInstruction(index2) = result and
index2 > index1
)
or
exists(IRBlock block1, IRBlock block2 |
store.getBlock() = block1 and
result.getBlock() = block2 and
block1.getASuccessor+() = block2
)
}
/**
* Holds if `store` copies the address of `f`'s local variable `var`
* into the address `globalAddress`.
*/
predicate stackAddressEscapes(
StoreInstruction store, StackVariable var, TGlobalAddress globalAddress, Function f
) {
globalAddress = globalAddress(store.getDestinationAddress()) and
exists(VariableAddressInstruction vai |
instructionHasVariable(pragma[only_bind_into](vai), var, f) and
stackPointerFlowsToUse(store.getSourceValue(), vai)
) and
// Ensure there's no subsequent store that overrides the global address.
not globalAddress = globalAddress(getAStoreStrictlyAfter(store).getDestinationAddress())
}
predicate blockStoresToAddress(
IRBlock block, int index, StoreInstruction store, TGlobalAddress globalAddress
) {
block.getInstruction(index) = store and
globalAddress = globalAddress(store.getDestinationAddress())
}
/**
* Holds if `globalAddress` evaluates to the address of `var` (which escaped through `store` before
* returning through `call`) when control reaches `block`.
*/
predicate globalAddressPointsToStack(
StoreInstruction store, StackVariable var, CallInstruction call, IRBlock block,
TGlobalAddress globalAddress, boolean isCallBlock, boolean isStoreBlock
) {
(
if blockStoresToAddress(block, _, _, globalAddress)
then isStoreBlock = true
else isStoreBlock = false
) and
(
isCallBlock = true and
exists(Function f |
stackAddressEscapes(store, var, globalAddress, f) and
call.getStaticCallTarget() = f and
call.getBlock() = block
)
or
isCallBlock = false and
step(store, var, call, globalAddress, _, block)
)
}
pragma[inline]
int getInstructionIndex(Instruction instr, IRBlock block) { block.getInstruction(result) = instr }
predicate step(
StoreInstruction store, StackVariable var, CallInstruction call, TGlobalAddress globalAddress,
IRBlock pred, IRBlock succ
) {
exists(boolean isCallBlock, boolean isStoreBlock |
// Only recurse if there is no store to `globalAddress` in `mid`.
globalAddressPointsToStack(store, var, call, pred, globalAddress, isCallBlock, isStoreBlock)
|
// Post domination ensures that `block` is always executed after `mid`
// Domination ensures that `mid` is always executed before `block`
isStoreBlock = false and
succ.immediatelyPostDominates(pred) and
pred.immediatelyDominates(succ)
or
exists(CallInstruction anotherCall, int anotherCallIndex |
anotherCall = pred.getInstruction(anotherCallIndex) and
succ.getFirstInstruction() instanceof EnterFunctionInstruction and
succ.getEnclosingFunction() = anotherCall.getStaticCallTarget() and
(if isCallBlock = true then getInstructionIndex(call, _) < anotherCallIndex else any()) and
(
if isStoreBlock = true
then
forex(int storeIndex | blockStoresToAddress(pred, storeIndex, _, globalAddress) |
anotherCallIndex < storeIndex
)
else any()
)
)
)
}
newtype TPathElement =
TStore(StoreInstruction store) { globalAddressPointsToStack(store, _, _, _, _, _, _) } or
TCall(CallInstruction call, IRBlock block) {
globalAddressPointsToStack(_, _, call, block, _, _, _)
} or
TMid(IRBlock block) { step(_, _, _, _, _, block) } or
TSink(LoadInstruction load, IRBlock block) {
exists(TGlobalAddress address |
globalAddressPointsToStack(_, _, _, block, address, _, _) and
block.getAnInstruction() = load and
globalAddress(load.getSourceAddress()) = address
)
}
class PathElement extends TPathElement {
StoreInstruction asStore() { this = TStore(result) }
CallInstruction asCall(IRBlock block) { this = TCall(result, block) }
predicate isCall(IRBlock block) { exists(this.asCall(block)) }
IRBlock asMid() { this = TMid(result) }
LoadInstruction asSink(IRBlock block) { this = TSink(result, block) }
predicate isSink(IRBlock block) { exists(this.asSink(block)) }
string toString() {
result = [asStore().toString(), asCall(_).toString(), asMid().toString(), asSink(_).toString()]
}
predicate hasLocationInfo(
string filepath, int startline, int startcolumn, int endline, int endcolumn
) {
this.asStore()
.getLocation()
.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
or
this.asCall(_)
.getLocation()
.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
or
this.asMid().getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
or
this.asSink(_)
.getLocation()
.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
}
}
predicate isSink(LoadInstruction load, IRBlock block, int index, TGlobalAddress globalAddress) {
block.getInstruction(index) = load and
globalAddress(load.getSourceAddress()) = globalAddress
}
query predicate edges(PathElement pred, PathElement succ) {
// Store -> caller
globalAddressPointsToStack(pred.asStore(), _, succ.asCall(_), _, _, _, _)
or
// Call -> basic block
pred.isCall(succ.asMid())
or
// Special case for when the caller goes directly to the load with no steps
// across basic blocks (i.e., caller -> sink)
exists(IRBlock block |
pred.isCall(block) and
succ.isSink(block)
)
or
// Basic block -> basic block
step(_, _, _, _, pred.asMid(), succ.asMid())
or
// Basic block -> load
succ.isSink(pred.asMid())
}
from
StoreInstruction store, StackVariable var, LoadInstruction load, CallInstruction call,
IRBlock block, boolean isCallBlock, TGlobalAddress address, boolean isStoreBlock,
PathElement source, PathElement sink, int loadIndex
where
globalAddressPointsToStack(store, var, call, block, address, isCallBlock, isStoreBlock) and
isSink(load, block, loadIndex, address) and
(
// We know that we have a sequence:
// (1) store to `address` -> (2) return from `f` -> (3) load from `address`.
// But if (2) and (3) happen in the sam block we need to check the
// block indices to ensure that (3) happens after (2).
if isCallBlock = true
then
// If so, the load must happen after the call.
getInstructionIndex(call, _) < loadIndex
else any()
) and
(
// If there is a store to the address we need to make sure that the load we found was
// before that store (So that the load doesn't read an overwritten value).
if isStoreBlock = true
then
forex(int storeIndex | blockStoresToAddress(block, storeIndex, _, address) |
loadIndex < storeIndex
)
else any()
) and
source.asStore() = store and
sink.asSink(_) = load
select sink, source, sink, "Stack variable $@ escapes $@ and is used after it has expired.", var,
var.toString(), store, "here"

View File

@@ -17,47 +17,169 @@
import cpp
// We don't actually use the global value numbering library in this query, but without it we end up
// recomputing the IR.
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
import semmle.code.cpp.ir.IR
import semmle.code.cpp.ir.dataflow.MustFlow
import PathGraph
private import semmle.code.cpp.valuenumbering.GlobalValueNumbering
private import semmle.code.cpp.ir.IR
class UnsafeUseOfThisConfig extends MustFlowConfiguration {
UnsafeUseOfThisConfig() { this = "UnsafeUseOfThisConfig" }
bindingset[n, result]
int unbind(int n) { result >= n and result <= n }
override predicate isSource(DataFlow::Node source) { isSource(source, _, _) }
/** Holds if `p` is the `n`'th parameter of the non-virtual function `f`. */
predicate parameterOf(Parameter p, Function f, int n) {
not f.isVirtual() and f.getParameter(n) = p
}
override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
/**
* Holds if `instr` is the `n`'th argument to a call to the non-virtual function `f`, and
* `init` is the corresponding initiazation instruction that receives the value of `instr` in `f`.
*/
predicate flowIntoParameter(
CallInstruction call, Instruction instr, Function f, int n, InitializeParameterInstruction init
) {
not f.isVirtual() and
call.getPositionalArgument(n) = instr and
f = call.getStaticCallTarget() and
getEnclosingNonVirtualFunctionInitializeParameter(init, f) and
init.getParameter().getIndex() = unbind(n)
}
/**
* Holds if `instr` is an argument to a call to the function `f`, and `init` is the
* corresponding initialization instruction that receives the value of `instr` in `f`.
*/
pragma[noinline]
predicate getPositionalArgumentInitParam(
CallInstruction call, Instruction instr, InitializeParameterInstruction init, Function f
) {
exists(int n |
parameterOf(_, f, n) and
flowIntoParameter(call, instr, f, unbind(n), init)
)
}
/**
* Holds if `instr` is the qualifier to a call to the non-virtual function `f`, and
* `init` is the corresponding initiazation instruction that receives the value of
* `instr` in `f`.
*/
pragma[noinline]
predicate getThisArgumentInitParam(
CallInstruction call, Instruction instr, InitializeParameterInstruction init, Function f
) {
not f.isVirtual() and
call.getStaticCallTarget() = f and
getEnclosingNonVirtualFunctionInitializeParameter(init, f) and
call.getThisArgument() = instr and
init.getIRVariable() instanceof IRThisVariable
}
/** Holds if `instr` is a `this` pointer used by the call instruction `call`. */
predicate isSink(DataFlow::Node sink, CallInstruction call) {
predicate isSink(Instruction instr, CallInstruction call) {
exists(PureVirtualFunction func |
call.getStaticCallTarget() = func and
call.getThisArgument() = sink.asInstruction() and
call.getThisArgument() = instr and
// Weed out implicit calls to destructors of a base class
not func instanceof Destructor
)
}
/** Holds if `init` initializes the `this` pointer in class `c`. */
predicate isSource(DataFlow::Node source, string msg, Class c) {
exists(InitializeParameterInstruction init | init = source.asInstruction() |
(
exists(Constructor func |
not func instanceof CopyConstructor and
not func instanceof MoveConstructor and
func = init.getEnclosingFunction() and
msg = "construction"
)
or
init.getEnclosingFunction() instanceof Destructor and msg = "destruction"
) and
init.getIRVariable() instanceof IRThisVariable and
init.getEnclosingFunction().getDeclaringType() = c
predicate isSource(InitializeParameterInstruction init, string msg, Class c) {
(
exists(Constructor func |
not func instanceof CopyConstructor and
not func instanceof MoveConstructor and
func = init.getEnclosingFunction() and
msg = "construction"
)
or
init.getEnclosingFunction() instanceof Destructor and msg = "destruction"
) and
init.getIRVariable() instanceof IRThisVariable and
init.getEnclosingFunction().getDeclaringType() = c
}
/**
* Holds if `instr` flows to a sink (which is a use of the value of `instr` as a `this` pointer).
*/
predicate flowsToSink(Instruction instr, Instruction sink) {
flowsFromSource(instr) and
(
isSink(instr, _) and instr = sink
or
exists(Instruction mid |
successor(instr, mid) and
flowsToSink(mid, sink)
)
)
}
/** Holds if `instr` flows from a source. */
predicate flowsFromSource(Instruction instr) {
isSource(instr, _, _)
or
exists(Instruction mid |
successor(mid, instr) and
flowsFromSource(mid)
)
}
/** Holds if `f` is the enclosing non-virtual function of `init`. */
predicate getEnclosingNonVirtualFunctionInitializeParameter(
InitializeParameterInstruction init, Function f
) {
not f.isVirtual() and
init.getEnclosingFunction() = f
}
/** Holds if `f` is the enclosing non-virtual function of `init`. */
predicate getEnclosingNonVirtualFunctionInitializeIndirection(
InitializeIndirectionInstruction init, Function f
) {
not f.isVirtual() and
init.getEnclosingFunction() = f
}
/**
* Holds if `instr` is an argument (or argument indirection) to a call, and
* `succ` is the corresponding initialization instruction in the call target.
*/
predicate flowThroughCallable(Instruction instr, Instruction succ) {
// Flow from an argument to a parameter
exists(CallInstruction call, InitializeParameterInstruction init | init = succ |
getPositionalArgumentInitParam(call, instr, init, call.getStaticCallTarget())
or
getThisArgumentInitParam(call, instr, init, call.getStaticCallTarget())
)
or
// Flow from argument indirection to parameter indirection
exists(
CallInstruction call, ReadSideEffectInstruction read, InitializeIndirectionInstruction init
|
init = succ and
read.getPrimaryInstruction() = call and
getEnclosingNonVirtualFunctionInitializeIndirection(init, call.getStaticCallTarget())
|
exists(int n |
read.getSideEffectOperand().getAnyDef() = instr and
read.getIndex() = n and
init.getParameter().getIndex() = unbind(n)
)
or
call.getThisArgument() = instr and
init.getIRVariable() instanceof IRThisVariable
)
}
/** Holds if `instr` flows to `succ`. */
predicate successor(Instruction instr, Instruction succ) {
succ.(CopyInstruction).getSourceValue() = instr or
succ.(CheckedConvertOrNullInstruction).getUnary() = instr or
succ.(ChiInstruction).getTotal() = instr or
succ.(ConvertInstruction).getUnary() = instr or
succ.(InheritanceConversionInstruction).getUnary() = instr or
flowThroughCallable(instr, succ)
}
/**
* Holds if:
* - `source` is an initialization of a `this` pointer of type `sourceClass`, and
@@ -66,19 +188,22 @@ predicate isSource(DataFlow::Node source, string msg, Class c) {
* - `msg` is a string describing whether `source` is from a constructor or destructor.
*/
predicate flows(
MustFlowPathNode source, string msg, Class sourceClass, MustFlowPathNode sink,
CallInstruction call
Instruction source, string msg, Class sourceClass, Instruction sink, CallInstruction call
) {
exists(UnsafeUseOfThisConfig conf |
conf.hasFlowPath(source, sink) and
isSource(source.getNode(), msg, sourceClass) and
isSink(sink.getNode(), call)
)
isSource(source, msg, sourceClass) and
flowsToSink(source, sink) and
isSink(sink, call)
}
from
MustFlowPathNode source, MustFlowPathNode sink, CallInstruction call, string msg,
Class sourceClass
query predicate edges(Instruction a, Instruction b) { successor(a, b) and flowsToSink(b, _) }
query predicate nodes(Instruction n, string key, string val) {
flowsToSink(n, _) and
key = "semmle.label" and
val = n.toString()
}
from Instruction source, Instruction sink, CallInstruction call, string msg, Class sourceClass
where
flows(source, msg, sourceClass, sink, call) and
// Only raise an alert if there is no override of the pure virtual function in any base class.

View File

@@ -0,0 +1,6 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<include src="FLinesOfDuplicatedCodeCommon.inc.qhelp" />
</qhelp>

View File

@@ -0,0 +1,27 @@
/**
* @deprecated
* @name Duplicated lines in files
* @description The number of lines in a file, including code, comment
* and whitespace lines, which are duplicated in at least
* one other place.
* @kind treemap
* @treemap.warnOn highValues
* @metricType file
* @metricAggregate avg sum max
* @id cpp/duplicated-lines-in-files
* @tags testability
* modularity
*/
import external.CodeDuplication
from File f, int n
where
n =
count(int line |
exists(DuplicateBlock d | d.sourceFile() = f |
line in [d.sourceStartLine() .. d.sourceEndLine()]
) and
not whitelistedLineForDuplication(f, line)
)
select f, n order by n desc

View File

@@ -0,0 +1,35 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
This metric measures the number of lines in a file that are contained within a block that is duplicated elsewhere. These lines may include code, comments and whitespace, and the duplicate block may be in this file or in another file.
</p>
<p>
A file that contains many lines that are duplicated within the code base is problematic
for a number of reasons.
</p>
</overview>
<include src="DuplicationProblems.inc.qhelp" />
<recommendation>
<p>
Refactor files with lots of duplicated code to extract the common code into
a shared library or module.
</p>
</recommendation>
<references>
<li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Duplicate_code">Duplicate code</a>.</li>
<li>M. Fowler, <em>Refactoring</em>. Addison-Wesley, 1999.</li>
</references>
</qhelp>

View File

@@ -168,16 +168,6 @@ class NetworkRecv extends NetworkSendRecv {
override Recv target;
}
pragma[noinline]
predicate encryptionFunction(Function f) {
f.getName().toLowerCase().regexpMatch(".*(crypt|encode|decode|hash|securezero).*")
}
pragma[noinline]
predicate encryptionType(UserType t) {
t.getName().toLowerCase().regexpMatch(".*(crypt|encode|decode|hash|securezero).*")
}
/**
* An expression that is an argument or return value from an encryption /
* decryption call. This is quite inclusive to minimize false positives, for
@@ -187,7 +177,10 @@ predicate encryptionType(UserType t) {
class Encrypted extends Expr {
Encrypted() {
exists(FunctionCall fc |
encryptionFunction(fc.getTarget()) and
fc.getTarget()
.getName()
.toLowerCase()
.regexpMatch(".*(crypt|encode|decode|hash|securezero).*") and
(
this = fc or
this = fc.getAnArgument()
@@ -196,7 +189,7 @@ class Encrypted extends Expr {
or
exists(Type t |
this.getType().refersTo(t) and
encryptionType(t)
t.getName().toLowerCase().regexpMatch(".*(crypt|encode|decode|hash|securezero).*")
)
}
}

View File

@@ -3,7 +3,7 @@
* @description Non-HTTPS connections can be intercepted by third parties.
* @kind path-problem
* @problem.severity warning
* @precision high
* @precision medium
* @id cpp/non-https-url
* @tags security
* external/cwe/cwe-319
@@ -12,7 +12,6 @@
import cpp
import semmle.code.cpp.dataflow.TaintTracking
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
import DataFlow::PathGraph
/**
@@ -58,12 +57,7 @@ class HttpStringToUrlOpenConfig extends TaintTracking::Configuration {
override predicate isSource(DataFlow::Node src) {
// Sources are strings containing an HTTP URL not in a private domain.
src.asExpr() instanceof HttpStringLiteral and
// block taint starting at `strstr`, which is likely testing an existing URL, rather than constructing an HTTP URL.
not exists(FunctionCall fc |
fc.getTarget().getName() = ["strstr", "strcasestr"] and
fc.getArgument(1) = globalValueNumber(src.asExpr()).getAnExpr()
)
src.asExpr() instanceof HttpStringLiteral
}
override predicate isSink(DataFlow::Node sink) {

View File

@@ -1,8 +0,0 @@
void encrypt_with_openssl(EVP_PKEY_CTX *ctx) {
// BAD: only 1024 bits for an RSA key
EVP_PKEY_CTX_set_rsa_keygen_bits(ctx, 1024);
// GOOD: 2048 bits for an RSA key
EVP_PKEY_CTX_set_rsa_keygen_bits(ctx, 2048);
}

View File

@@ -1,38 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Using cryptographic algorithms with a small key size can leave data vulnerable to being decrypted.</p>
<p>Many cryptographic algorithms provided by cryptography libraries can be configured with key sizes that are
vulnerable to brute force attacks. Using such a key size means that an attacker may be able to easily decrypt the
encrypted data.</p>
</overview>
<recommendation>
<p>Ensure that you use a strong, modern cryptographic algorithm. Use at least AES-128 or RSA-2048.</p>
</recommendation>
<example>
<p>The following code shows an example of using the <code>openssl</code> library to generate an RSA key.
When creating a key, you must specify which key size to use. The first example uses 1024 bits, which is not
considered sufficient. The second example uses 2048 bits, which is currently considered sufficient.</p>
<sample src="InsufficientKeySize.c" />
</example>
<references>
<li>NIST, FIPS 140 Annex a: <a href="http://csrc.nist.gov/publications/fips/fips140-2/fips1402annexa.pdf">
Approved Security Functions</a>.</li>
<li>NIST, SP 800-131A: <a href="https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar2.pdf">
Transitions: Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Lengths</a>.</li>
<!-- LocalWords: CWE
-->
</references>
</qhelp>

View File

@@ -1,63 +0,0 @@
/**
* @name Use of a cryptographic algorithm with insufficient key size
* @description Using cryptographic algorithms with too small a key size can
* allow an attacker to compromise security.
* @kind path-problem
* @problem.severity error
* @precision high
* @id cpp/insufficient-key-size
* @tags security
* external/cwe/cwe-326
*/
import cpp
import semmle.code.cpp.ir.dataflow.DataFlow
import semmle.code.cpp.ir.IR
import DataFlow::PathGraph
// Gets the recommended minimum key size (in bits) of `func`, the name of an encryption function that accepts a key size as parameter `paramIndex`
int getMinimumKeyStrength(string func, int paramIndex) {
func =
[
"EVP_PKEY_CTX_set_dsa_paramgen_bits", "DSA_generate_parameters_ex",
"EVP_PKEY_CTX_set_rsa_keygen_bits", "RSA_generate_key_ex", "RSA_generate_key_fips",
"EVP_PKEY_CTX_set_dh_paramgen_prime_len", "DH_generate_parameters_ex"
] and
paramIndex = 1 and
result = 2048
}
class KeyStrengthFlow extends DataFlow::Configuration {
KeyStrengthFlow() { this = "KeyStrengthFlow" }
override predicate isSource(DataFlow::Node node) {
exists(int bits |
node.asInstruction().(IntegerConstantInstruction).getValue().toInt() = bits and
bits < getMinimumKeyStrength(_, _) and
bits > 0 // exclude sentinel values
)
}
override predicate isSink(DataFlow::Node node) {
exists(FunctionCall fc, string name, int param |
node.asExpr() = fc.getArgument(param) and
fc.getTarget().hasGlobalName(name) and
exists(getMinimumKeyStrength(name, param))
)
}
}
from
DataFlow::PathNode source, DataFlow::PathNode sink, KeyStrengthFlow conf, FunctionCall fc,
int param, string name, int minimumBits, int bits
where
conf.hasFlowPath(source, sink) and
sink.getNode().asExpr() = fc.getArgument(param) and
fc.getTarget().hasGlobalName(name) and
minimumBits = getMinimumKeyStrength(name, param) and
bits = source.getNode().asInstruction().(ConstantValueInstruction).getValue().toInt() and
bits < minimumBits and
bits != 0
select fc, source, sink,
"The key size $@ is less than the recommended key size of " + minimumBits.toString() + " bits.",
source, bits.toString()

View File

@@ -3,7 +3,7 @@
* @description Exposing system data or debugging information helps
* an adversary learn about the system and form an
* attack plan.
* @kind path-problem
* @kind problem
* @problem.severity warning
* @security-severity 6.5
* @precision medium
@@ -14,9 +14,7 @@
import cpp
import semmle.code.cpp.commons.Environment
import semmle.code.cpp.ir.dataflow.TaintTracking
import semmle.code.cpp.models.interfaces.FlowSource
import DataFlow::PathGraph
import semmle.code.cpp.security.OutputWrite
/**
* An element that should not be exposed to an adversary.
@@ -26,19 +24,42 @@ abstract class SystemData extends Element {
* Gets an expression that is part of this `SystemData`.
*/
abstract Expr getAnExpr();
/**
* Gets an expression whose value originates from, or is used by,
* this `SystemData`.
*/
Expr getAnExprIndirect() {
// direct SystemData
result = this.getAnExpr() or
// flow via global or member variable (conservative approximation)
result = this.getAnAffectedVar().getAnAccess() or
// flow via stack variable
definitionUsePair(_, this.getAnExprIndirect(), result) or
useUsePair(_, this.getAnExprIndirect(), result) or
useUsePair(_, result, this.getAnExprIndirect()) or
// flow from assigned value to assignment expression
result.(AssignExpr).getRValue() = this.getAnExprIndirect()
}
/**
* Gets a global or member variable that may be affected by this system
* data (conservative approximation).
*/
private Variable getAnAffectedVar() {
(
result.getAnAssignedValue() = this.getAnExprIndirect() or
result.getAnAccess() = this.getAnExprIndirect()
) and
not result instanceof LocalScopeVariable
}
}
/**
* Data originating from the environment.
*/
class EnvData extends SystemData {
EnvData() {
// identify risky looking environment variables only
this.(EnvironmentRead)
.getEnvironmentVariable()
.toLowerCase()
.regexpMatch(".*(user|host|admin|root|home|path|http|ssl|snmp|sock|port|proxy|pass|token|crypt|key).*")
}
EnvData() { this instanceof EnvironmentRead }
override Expr getAnExpr() { result = this }
}
@@ -70,6 +91,11 @@ class SQLConnectInfo extends SystemData {
}
private predicate posixSystemInfo(FunctionCall source, Element use) {
// long sysconf(int name)
// - various OS / system values and limits
source.getTarget().hasName("sysconf") and
use = source
or
// size_t confstr(int name, char *buf, size_t len)
// - various OS / system strings, such as the libc version
// int statvfs(const char *__path, struct statvfs *__buf)
@@ -285,31 +311,70 @@ class RegQuery extends SystemData {
override Expr getAnExpr() { regQuery(this, result) }
}
class ExposedSystemDataConfiguration extends TaintTracking::Configuration {
ExposedSystemDataConfiguration() { this = "ExposedSystemDataConfiguration" }
override predicate isSource(DataFlow::Node source) {
source.asConvertedExpr() = any(SystemData sd).getAnExpr()
}
override predicate isSink(DataFlow::Node sink) {
exists(FunctionCall fc, FunctionInput input, int arg |
fc.getTarget().(RemoteFlowSinkFunction).hasRemoteFlowSink(input, _) and
input.isParameterDeref(arg) and
fc.getArgument(arg).getAChild*() = sink.asExpr()
)
}
/**
* Somewhere data is output.
*/
abstract class DataOutput extends Element {
/**
* Get an expression containing data that is output.
*/
abstract Expr getASource();
}
from ExposedSystemDataConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
where
config.hasFlowPath(source, sink) and
not exists(
DataFlow::Node alt // remove duplicate results on conversions
|
config.hasFlow(source.getNode(), alt) and
alt.asConvertedExpr() = sink.getNode().asExpr() and
alt != sink.getNode()
/**
* Data that is output via standard output or standard error.
*/
class StandardOutput extends DataOutput instanceof OutputWrite {
override Expr getASource() { result = OutputWrite.super.getASource() }
}
private predicate socketCallOrIndirect(FunctionCall call) {
// direct socket call
// int socket(int domain, int type, int protocol);
call.getTarget().getName() = "socket"
or
exists(ReturnStmt rtn |
// indirect socket call
call.getTarget() = rtn.getEnclosingFunction() and
(
socketCallOrIndirect(rtn.getExpr()) or
socketCallOrIndirect(rtn.getExpr().(VariableAccess).getTarget().getAnAssignedValue())
)
)
select sink, source, sink, "This operation exposes system data from $@.", source,
source.getNode().toString()
}
private predicate socketFileDescriptor(Expr e) {
exists(Variable var, FunctionCall socket |
socketCallOrIndirect(socket) and
var.getAnAssignedValue() = socket and
e = var.getAnAccess()
)
}
private predicate socketOutput(FunctionCall call, Expr data) {
(
// ssize_t send(int sockfd, const void *buf, size_t len, int flags);
// ssize_t sendto(int sockfd, const void *buf, size_t len, int flags,
// const struct sockaddr *dest_addr, socklen_t addrlen);
// ssize_t sendmsg(int sockfd, const struct msghdr *msg, int flags);
// int write(int handle, void *buffer, int nbyte);
call.getTarget().hasGlobalName(["send", "sendto", "sendmsg", "write"]) and
data = call.getArgument(1) and
socketFileDescriptor(call.getArgument(0))
)
}
/**
* Data that is output via a socket.
*/
class SocketOutput extends DataOutput {
SocketOutput() { socketOutput(this, _) }
override Expr getASource() { socketOutput(this, result) }
}
from SystemData sd, DataOutput ow
where
sd.getAnExprIndirect() = ow.getASource() or
sd.getAnExprIndirect() = ow.getASource().getAChild*()
select ow, "This operation exposes system data from $@.", sd, sd.toString()

View File

@@ -12,16 +12,17 @@
import cpp
import FilePermissions
import semmle.code.cpp.commons.unix.Constants
predicate worldWritableCreation(FileCreationExpr fc, int mode) {
mode = localUmask(fc).mask(fc.getMode()) and
setsAnyBits(mode, UnixConstants::s_iwoth())
sets(mode, s_iwoth())
}
predicate setWorldWritable(FunctionCall fc, int mode) {
fc.getTarget().getName() = ["chmod", "fchmod", "_chmod", "_wchmod"] and
mode = fc.getArgument(1).getValue().toInt() and
setsAnyBits(mode, UnixConstants::s_iwoth())
sets(mode, s_iwoth())
}
from Expr fc, int mode, string message

View File

@@ -1,49 +1,5 @@
import cpp
import semmle.code.cpp.commons.unix.Constants as UnixConstants
/**
* Gets the number corresponding to the contents of `input` in base-16.
* Note: the first two characters of `input` must be `0x`. For example:
* `parseHex("0x123abc") = 1194684`.
*/
bindingset[input]
int parseHex(string input) {
exists(string lowerCaseInput | lowerCaseInput = input.toLowerCase() |
lowerCaseInput.regexpMatch("0x[0-9a-f]+") and
result =
strictsum(int ix |
ix in [2 .. input.length()]
|
16.pow(input.length() - (ix + 1)) * "0123456789abcdef".indexOf(lowerCaseInput.charAt(ix))
)
)
}
/**
* Gets the value defined by the `O_CREAT` macro if the macro
* exists and if every definition defines the same value.
*/
int o_creat() {
result =
unique(int v |
exists(Macro m | m.getName() = "O_CREAT" |
v = parseHex(m.getBody()) or v = UnixConstants::parseOctal(m.getBody())
)
)
}
/**
* Gets the value defined by the `O_TMPFILE` macro if the macro
* exists and if every definition defines the same value.
*/
int o_tmpfile() {
result =
unique(int v |
exists(Macro m | m.getName() = "O_TMPFILE" |
v = parseHex(m.getBody()) or v = UnixConstants::parseOctal(m.getBody())
)
)
}
import semmle.code.cpp.commons.unix.Constants
bindingset[n, digit]
private string octalDigit(int n, int digit) {
@@ -64,17 +20,11 @@ string octalFileMode(int mode) {
else result = "[non-standard mode: decimal " + mode + "]"
}
/**
* Holds if the bitmask `value` sets the bits in `flag`.
*/
bindingset[value, flag]
predicate setsFlag(int value, int flag) { value.bitAnd(flag) = flag }
/**
* Holds if the bitmask `mask` sets any of the bit fields in `fields`.
*/
bindingset[mask, fields]
predicate setsAnyBits(int mask, int fields) { mask.bitAnd(fields) != 0 }
predicate sets(int mask, int fields) { mask.bitAnd(fields) != 0 }
/**
* Gets the value that `fc` sets the umask to, if `fc` is a call to
@@ -133,24 +83,16 @@ abstract class FileCreationExpr extends FunctionCall {
abstract int getMode();
}
abstract class FileCreationWithOptionalModeExpr extends FileCreationExpr {
abstract predicate hasModeArgument();
}
class OpenCreationExpr extends FileCreationWithOptionalModeExpr {
class OpenCreationExpr extends FileCreationExpr {
OpenCreationExpr() {
this.getTarget().hasGlobalOrStdName(["open", "_open", "_wopen"]) and
exists(int flag | flag = this.getArgument(1).getValue().toInt() |
setsFlag(flag, o_creat()) or setsFlag(flag, o_tmpfile())
)
this.getTarget().getName() = ["open", "_open", "_wopen"] and
sets(this.getArgument(1).getValue().toInt(), o_creat())
}
override Expr getPath() { result = this.getArgument(0) }
override predicate hasModeArgument() { exists(this.getArgument(2)) }
override int getMode() {
if this.hasModeArgument()
if exists(this.getArgument(2))
then result = this.getArgument(2).getValue().toInt()
else
// assume anything is permitted
@@ -166,35 +108,20 @@ class CreatCreationExpr extends FileCreationExpr {
override int getMode() { result = this.getArgument(1).getValue().toInt() }
}
class OpenatCreationExpr extends FileCreationWithOptionalModeExpr {
class OpenatCreationExpr extends FileCreationExpr {
OpenatCreationExpr() {
this.getTarget().hasGlobalOrStdName("openat") and
exists(int flag | flag = this.getArgument(2).getValue().toInt() |
setsFlag(flag, o_creat()) or setsFlag(flag, o_tmpfile())
)
this.getTarget().getName() = "openat" and
this.getNumberOfArguments() = 4
}
override Expr getPath() { result = this.getArgument(1) }
override predicate hasModeArgument() { exists(this.getArgument(3)) }
override int getMode() {
if this.hasModeArgument()
then result = this.getArgument(3).getValue().toInt()
else
// assume anything is permitted
result = 0.bitNot()
}
override int getMode() { result = this.getArgument(3).getValue().toInt() }
}
private int fopenMode() {
result =
UnixConstants::s_irusr()
.bitOr(UnixConstants::s_irgrp())
.bitOr(UnixConstants::s_iroth())
.bitOr(UnixConstants::s_iwusr())
.bitOr(UnixConstants::s_iwgrp())
.bitOr(UnixConstants::s_iwoth())
s_irusr().bitOr(s_irgrp()).bitOr(s_iroth()).bitOr(s_iwusr()).bitOr(s_iwgrp()).bitOr(s_iwoth())
}
class FopenCreationExpr extends FileCreationExpr {
@@ -226,6 +153,6 @@ class FopensCreationExpr extends FileCreationExpr {
// fopen_s has restrictive permissions unless you have "u" in the mode
if this.getArgument(2).getValue().charAt(_) = "u"
then result = fopenMode()
else result = UnixConstants::s_irusr().bitOr(UnixConstants::s_iwusr())
else result = s_irusr().bitOr(s_iwusr())
}
}

View File

@@ -1,9 +0,0 @@
int open_file_bad() {
// BAD - this uses arbitrary bytes from the stack as mode argument
return open(FILE, O_CREAT)
}
int open_file_good() {
// GOOD - the mode argument is supplied
return open(FILE, O_CREAT, S_IRUSR | S_IWUSR)
}

View File

@@ -1,31 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
When opening a file with the <code>O_CREAT</code> or <code>O_TMPFILE</code> flag, the <code>mode</code> must
be supplied. If the <code>mode</code> argument is omitted, some arbitrary bytes from the stack will be used
as the file mode. This leaks some bits from the stack into the permissions of the file.
</p>
</overview>
<recommendation>
<p>
The <code>mode</code> must be supplied when <code>O_CREAT</code> or <code>O_TMPFILE</code> is specified.
</p>
</recommendation>
<example>
<p>
The first example opens a file with the <code>O_CREAT</code> flag without supplying the <code>mode</code>
argument. In this case arbitrary bytes from the stack will be used as <code>mode</code> argument. The
second example correctly supplies the <code>mode</code> argument and creates a file that is user readable
and writable.
</p>
<sample src="OpenCallMissingModeArgument.c" />
</example>
</qhelp>

View File

@@ -1,19 +0,0 @@
/**
* @name File opened with O_CREAT flag but without mode argument
* @description Opening a file with the O_CREAT flag but without mode argument reads arbitrary bytes from the stack.
* @kind problem
* @problem.severity error
* @security-severity 7.8
* @precision high
* @id cpp/open-call-with-mode-argument
* @tags security
* external/cwe/cwe-732
*/
import cpp
import FilePermissions
from FileCreationWithOptionalModeExpr fc
where not fc.hasModeArgument()
select fc,
"A file is created here without providing a mode argument, which may leak bits from the stack."

View File

@@ -0,0 +1,4 @@
---
category: newQuery
---
* A new `cpp/very-likely-overruning-write` query has been added to the default query suite for C/C++. The query reports some results that were formerly flagged by `cpp/overruning-write`.

View File

@@ -0,0 +1,5 @@
---
category: minorAnalysis
---
* Added exception for GLib's gboolean to cpp/ambiguously-signed-bit-field.
This change reduces the number of false positives in the query.

View File

@@ -0,0 +1,4 @@
---
category: newQuery
---
* The "Uncontrolled data in arithmetic expression" (cpp/uncontrolled-arithmetic) query has been enhanced to reduce false positive results and its @precision increased to high.

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The "Potential improper null termination" (`cpp/improper-null-termination`) query now produces fewer false positive results around control flow branches and loops.

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The "Cleartext transmission of sensitive information" (`cpp/cleartext-transmission`) query has been improved in several ways to reduce false positive results.

View File

@@ -0,0 +1,5 @@
---
category: minorAnalysis
---
* The `cpp/return-stack-allocated-memory` query has been improved to produce fewer false positives. The
query has also been converted to a `path-problem` query.

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The "Cleartext transmission of sensitive information" (`cpp/cleartext-transmission`) query now finds more results, where a password is stored in a struct field or class member variable.

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `cpp/cleartext-storage-file` query has been upgraded with non-local taint flow and has been converted to a `path-problem` query.

View File

@@ -0,0 +1,4 @@
---
category: newQuery
---
* The `security` tag has been added to the `cpp/return-stack-allocated-memory` query. As a result, its results will now appear by default.

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `cpp/cleartext-storage-buffer` query has been updated to use the `semmle.code.cpp.dataflow.TaintTracking` library.

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Fix an issue with the `cpp/declaration-hides-variable` query where it would report variables that are unnamed in a database.

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `cpp/cleartext-storage-file` query has been improved, removing false positives where data is written to a standard output stream.

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The "Cleartext transmission of sensitive information" (`cpp/cleartext-transmission`) query has been further improved to reduce false positive results, and upgraded from `medium` to `high` precision.

View File

@@ -1,5 +0,0 @@
## 0.0.10
### Deprecated Classes
* The `CodeDuplication.Copy`, `CodeDuplication.DuplicateBlock`, and `CodeDuplication.SimilarBlock` classes have been deprecated.

View File

@@ -1,20 +0,0 @@
## 0.0.11
### Breaking Changes
* The deprecated queries `cpp/duplicate-block`, `cpp/duplicate-function`, `cpp/duplicate-class`, `cpp/duplicate-file`, `cpp/mostly-duplicate-function`,`cpp/similar-file`, `cpp/duplicated-lines-in-files` have been removed.
### Deprecated Predicates and Classes
* The predicates and classes in the `CodeDuplication` library have been deprecated.
### New Queries
* A new query titled "Use of expired stack-address" (`cpp/using-expired-stack-address`) has been added.
This query finds accesses to expired stack-allocated memory that escaped via a global variable.
* A new `cpp/insufficient-key-size` query has been added to the default query suite for C/C++. The query finds uses of certain cryptographic algorithms where the key size is too small to provide adequate encryption strength.
### Minor Analysis Improvements
* The "Failure to use HTTPS URLs" (`cpp/non-https-url`) has been improved reducing false positive results, and its precision has been increased to 'high'.
* The `cpp/system-data-exposure` query has been modernized and has converted to a `path-problem` query. There are now fewer false positive results.

View File

@@ -1,18 +0,0 @@
## 0.0.8
### New Queries
* The `security` tag has been added to the `cpp/return-stack-allocated-memory` query. As a result, its results will now appear by default.
* The "Uncontrolled data in arithmetic expression" (cpp/uncontrolled-arithmetic) query has been enhanced to reduce false positive results and its @precision increased to high.
* A new `cpp/very-likely-overruning-write` query has been added to the default query suite for C/C++. The query reports some results that were formerly flagged by `cpp/overruning-write`.
### Minor Analysis Improvements
* Fix an issue with the `cpp/declaration-hides-variable` query where it would report variables that are unnamed in a database.
* The `cpp/cleartext-storage-file` query has been upgraded with non-local taint flow and has been converted to a `path-problem` query.
* The `cpp/return-stack-allocated-memory` query has been improved to produce fewer false positives. The
query has also been converted to a `path-problem` query.
* The "Cleartext transmission of sensitive information" (`cpp/cleartext-transmission`) query has been improved in several ways to reduce false positive results.
* The "Potential improper null termination" (`cpp/improper-null-termination`) query now produces fewer false positive results around control flow branches and loops.
* Added exception for GLib's gboolean to cpp/ambiguously-signed-bit-field.
This change reduces the number of false positives in the query.

View File

@@ -1,13 +0,0 @@
## 0.0.9
### New Queries
* Added a new query, `cpp/open-call-with-mode-argument`, to detect when `open` or `openat` is called with the `O_CREAT` or `O_TMPFILE` flag but when the `mode` argument is omitted.
### Minor Analysis Improvements
* The "Cleartext transmission of sensitive information" (`cpp/cleartext-transmission`) query has been further improved to reduce false positive results, and upgraded from `medium` to `high` precision.
* The "Cleartext transmission of sensitive information" (`cpp/cleartext-transmission`) query now finds more results, where a password is stored in a struct field or class member variable.
* The `cpp/cleartext-storage-file` query has been improved, removing false positives where data is written to a standard output stream.
* The `cpp/cleartext-storage-buffer` query has been updated to use the `semmle.code.cpp.dataflow.TaintTracking` library.
* The `cpp/world-writable-file-creation` query now only detects `open` and `openat` calls with the `O_CREAT` or `O_TMPFILE` flag.

View File

@@ -1,2 +1,2 @@
---
lastReleaseVersion: 0.0.11
lastReleaseVersion: 0.0.7

View File

@@ -2,6 +2,7 @@
# These queries are infeasible to compute on large projects:
- exclude:
query path:
- Security/CWE/CWE-497/ExposedSystemData.ql
- Critical/DescriptorMayNotBeClosed.ql
- Critical/DescriptorNeverClosed.ql
- Critical/FileMayNotBeClosed.ql

View File

@@ -38,8 +38,6 @@ where
) and
eq.getRightOperand() instanceof Literal and
ne.getRightOperand() instanceof Literal and
eq.getLeftOperand().getFullyConverted().getUnspecifiedType() =
ne.getLeftOperand().getFullyConverted().getUnspecifiedType() and
nearestParent(eq) = nearestParent(ne) and
sameExpr(eq.getLeftOperand(), ne.getLeftOperand())
select ne, "Useless Test"

View File

@@ -1,5 +0,0 @@
...
vUnsignedLong = (unsigned long)(vUnsignedInt*vUnsignedInt); // BAD
...
vUnsignedLong = ((unsigned long)vUnsignedInt*vUnsignedInt); // GOOD
...

View File

@@ -1,24 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Search for places where the result of the multiplication is subjected to explicit conversion, not the arguments. Therefore, during the multiplication period, you can lose meaningful data.</p>
</overview>
<example>
<p>The following example demonstrates erroneous and fixed methods for working with type conversion.</p>
<sample src="DangerousUseOfTransformationAfterOperation.cpp" />
</example>
<references>
<li>
CERT C Coding Standard:
<a href="https://wiki.sei.cmu.edu/confluence/display/c/INT30-C.+Ensure+that+unsigned+integer+operations+do+not+wrap">INT30-C. Ensure that unsigned integer operations do not wrap</a>.
</li>
</references>
</qhelp>

View File

@@ -1,102 +0,0 @@
/**
* @name Dangerous use of transformation after operation.
* @description By using the transformation after the operation, you are doing a pointless and dangerous action.
* @kind problem
* @id cpp/dangerous-use-of-transformation-after-operation
* @problem.severity warning
* @precision medium
* @tags correctness
* security
* external/cwe/cwe-190
*/
import cpp
/** Returns the number of the expression in the function call arguments. */
int argumentPosition(FunctionCall fc, Expr exp, int n) {
n in [0 .. fc.getNumberOfArguments() - 1] and
fc.getArgument(n) = exp and
result = n
}
/** Holds if a nonsensical type conversion situation is found. */
predicate conversionDoneLate(MulExpr mexp) {
exists(Expr e1, Expr e2 |
mexp.hasOperands(e1, e2) and
not e1.isConstant() and
not e1.hasConversion() and
not e1.hasConversion() and
(
e2.isConstant() or
not e2.hasConversion()
) and
mexp.getConversion().hasExplicitConversion() and
mexp.getConversion() instanceof ParenthesisExpr and
mexp.getConversion().getConversion() instanceof CStyleCast and
mexp.getConversion().getConversion().getType().getSize() > mexp.getType().getSize() and
mexp.getConversion().getConversion().getType().getSize() > e2.getType().getSize() and
mexp.getConversion().getConversion().getType().getSize() > e1.getType().getSize() and
exists(Expr e0 |
e0.(AssignExpr).getRValue() = mexp.getParent*() and
e0.(AssignExpr).getLValue().getType().getSize() =
mexp.getConversion().getConversion().getType().getSize()
or
mexp.getEnclosingElement().(ComparisonOperation).hasOperands(mexp, e0) and
e0.getType().getSize() = mexp.getConversion().getConversion().getType().getSize()
or
e0.(FunctionCall)
.getTarget()
.getParameter(argumentPosition(e0.(FunctionCall), mexp, _))
.getType()
.getSize() = mexp.getConversion().getConversion().getType().getSize()
)
)
}
/** Holds if the situation of a possible signed overflow used in pointer arithmetic is found. */
predicate signSmallerWithEqualSizes(MulExpr mexp) {
exists(Expr e1, Expr e2 |
mexp.hasOperands(e1, e2) and
not e1.isConstant() and
not e1.hasConversion() and
not e1.hasConversion() and
(
e2.isConstant() or
not e2.hasConversion()
) and
mexp.getConversion+().getUnderlyingType().getSize() = e1.getUnderlyingType().getSize() and
(
e2.isConstant() or
mexp.getConversion+().getUnderlyingType().getSize() = e2.getUnderlyingType().getSize()
) and
mexp.getConversion+().getUnderlyingType().getSize() = e1.getUnderlyingType().getSize() and
exists(AssignExpr ae |
ae.getRValue() = mexp.getParent*() and
ae.getRValue().getUnderlyingType().(IntegralType).isUnsigned() and
ae.getLValue().getUnderlyingType().(IntegralType).isSigned() and
(
not exists(DivExpr de | mexp.getParent*() = de)
or
exists(DivExpr de, Expr ec |
e2.isConstant() and
de.hasOperands(mexp.getParent*(), ec) and
ec.isConstant() and
e2.getValue().toInt() > ec.getValue().toInt()
)
) and
exists(PointerAddExpr pa |
ae.getASuccessor+() = pa and
pa.getAnOperand().(VariableAccess).getTarget() = ae.getLValue().(VariableAccess).getTarget()
)
)
)
}
from MulExpr mexp, string msg
where
conversionDoneLate(mexp) and
msg = "This transformation is applied after multiplication."
or
signSmallerWithEqualSizes(mexp) and
msg = "Possible signed overflow followed by offset of the pointer out of bounds."
select mexp, msg

View File

@@ -1,10 +0,0 @@
...
FILE *fp = fopen(filename,"w"); // BAD
...
umask(S_IXUSR|S_IRWXG|S_IRWXO);
FILE *fp;
fp = fopen(filename,"w"); // GOOD
chmod(filename,S_IRUSR|S_IWUSR);
fprintf(fp,"%s\n","data to file");
fclose(fp);
...

View File

@@ -1,24 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>When creating a file using a library function such as <code>fopen</code>, the access rights for the newly created file are not specified as part of the call. Instead these rights are determined by the system unless the programmer takes specific measures, such as calling the Posix <code>umask</code> function at some point before the call to <code>fopen</code>. For some applications, the default access rights assigned by the system are not sufficient to protect a file against access by an attacker.</p>
</overview>
<example>
<p>The following example demonstrates erroneous and fixed methods for working with files.</p>
<sample src="ExposureSensitiveInformationUnauthorizedActor.cpp" />
</example>
<references>
<li>
CERT C Coding Standard:
<a href="https://wiki.sei.cmu.edu/confluence/display/c/FIO06-C.+Create+files+with+appropriate+access+permissions">FIO06-C. Create files with appropriate access permissions</a>.
</li>
</references>
</qhelp>

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