Compare commits

..

1 Commits

Author SHA1 Message Date
github-actions[bot]
56dffea911 Release preparation for version 2.7.4 2021-12-11 13:15:46 +00:00
1163 changed files with 5293 additions and 63899 deletions

View File

@@ -11,9 +11,7 @@
"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"
"ruby/ql/consistency-queries/qlpack.yml"
],
"versionPolicies": {
"default": {

3
.github/labeler.yml vendored
View File

@@ -26,6 +26,3 @@ documentation:
- "**/*.qhelp"
- "**/*.md"
- docs/**/*
"QL-for-QL":
- ql/**/*

View File

@@ -7,7 +7,6 @@ on:
- "*/ql/src/**/*.ql"
- "*/ql/src/**/*.qll"
- "!**/experimental/**"
- "!ql/**"
jobs:
check-change-note:

View File

@@ -1,192 +0,0 @@
name: Run QL for QL
on:
push:
branches: [main]
pull_request:
branches: [main]
env:
CARGO_TERM_COLOR: always
jobs:
queries:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Find codeql
id: find-codeql
uses: github/codeql-action/init@erik-krogh/ql
with:
languages: javascript # does not matter
- name: Get CodeQL version
id: get-codeql-version
run: |
echo "::set-output name=version::$("${CODEQL}" --version | head -n 1 | rev | cut -d " " -f 1 | rev)"
shell: bash
env:
CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
- name: Cache queries
id: cache-queries
uses: actions/cache@v2
with:
path: ${{ runner.temp }}/query-pack.zip
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-all/0.0.0
zip "${PACKZIP}" -r .
env:
CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
PACKZIP: ${{ runner.temp }}/query-pack.zip
- name: Upload query pack
uses: actions/upload-artifact@v2
with:
name: query-pack-zip
path: ${{ runner.temp }}/query-pack.zip
extractors:
strategy:
fail-fast: false
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Cache entire extractor
id: cache-extractor
uses: actions/cache@v2
with:
path: |
ql/target/release/ql-autobuilder
ql/target/release/ql-autobuilder.exe
ql/target/release/ql-extractor
ql/target/release/ql-extractor.exe
key: ${{ runner.os }}-extractor-${{ hashFiles('ql/**/Cargo.lock') }}-${{ hashFiles('ql/**/*.rs') }}
- name: Cache cargo
if: steps.cache-extractor.outputs.cache-hit != 'true'
uses: actions/cache@v2
with:
path: |
~/.cargo/registry
~/.cargo/git
ql/target
key: ${{ runner.os }}-rust-cargo-${{ hashFiles('ql/**/Cargo.lock') }}
- name: Check formatting
if: steps.cache-extractor.outputs.cache-hit != 'true'
run: cd ql; cargo fmt --all -- --check
- name: Build
if: steps.cache-extractor.outputs.cache-hit != 'true'
run: cd ql; cargo build --verbose
- name: Run tests
if: steps.cache-extractor.outputs.cache-hit != 'true'
run: cd ql; cargo test --verbose
- name: Release build
if: steps.cache-extractor.outputs.cache-hit != 'true'
run: cd ql; cargo build --release
- name: Generate dbscheme
if: steps.cache-extractor.outputs.cache-hit != 'true'
run: ql/target/release/ql-generator --dbscheme ql/ql/src/ql.dbscheme --library ql/ql/src/codeql_ql/ast/internal/TreeSitter.qll
- uses: actions/upload-artifact@v2
with:
name: extractor-ubuntu-latest
path: |
ql/target/release/ql-autobuilder
ql/target/release/ql-autobuilder.exe
ql/target/release/ql-extractor
ql/target/release/ql-extractor.exe
retention-days: 1
package:
runs-on: ubuntu-latest
needs:
- extractors
- queries
steps:
- uses: actions/checkout@v2
- uses: actions/download-artifact@v2
with:
name: query-pack-zip
path: query-pack-zip
- uses: actions/download-artifact@v2
with:
name: extractor-ubuntu-latest
path: linux64
- run: |
unzip query-pack-zip/*.zip -d pack
cp -r ql/codeql-extractor.yml ql/tools ql/ql/src/ql.dbscheme.stats pack/
mkdir -p pack/tools/linux64
if [[ -f linux64/ql-autobuilder ]]; then
cp linux64/ql-autobuilder pack/tools/linux64/autobuilder
chmod +x pack/tools/linux64/autobuilder
fi
if [[ -f linux64/ql-extractor ]]; then
cp linux64/ql-extractor pack/tools/linux64/extractor
chmod +x pack/tools/linux64/extractor
fi
cd pack
zip -rq ../codeql-ql.zip .
- uses: actions/upload-artifact@v2
with:
name: codeql-ql-pack
path: codeql-ql.zip
retention-days: 1
analyze:
runs-on: ubuntu-latest
strategy:
matrix:
folder: [cpp, csharp, java, javascript, python, ql, ruby]
needs:
- package
steps:
- name: Download pack
uses: actions/download-artifact@v2
with:
name: codeql-ql-pack
path: ${{ runner.temp }}/codeql-ql-pack-artifact
- name: Prepare pack
run: |
unzip "${PACK_ARTIFACT}/*.zip" -d "${PACK}"
env:
PACK_ARTIFACT: ${{ runner.temp }}/codeql-ql-pack-artifact
PACK: ${{ runner.temp }}/pack
- name: Hack codeql-action options
run: |
JSON=$(jq -nc --arg pack "${PACK}" '.resolve.queries=["--search-path", $pack] | .resolve.extractor=["--search-path", $pack] | .database.init=["--search-path", $pack]')
echo "CODEQL_ACTION_EXTRA_OPTIONS=${JSON}" >> ${GITHUB_ENV}
env:
PACK: ${{ runner.temp }}/pack
- name: Checkout repository
uses: actions/checkout@v2
- name: Create CodeQL config file
run: |
echo "paths:" > ${CONF}
echo " - ${FOLDER}" >> ${CONF}
echo "paths-ignore:" >> ${CONF}
echo " - ql/ql/test" >> ${CONF}
echo "Config file: "
cat ${CONF}
env:
CONF: ./ql-for-ql-config.yml
FOLDER: ${{ matrix.folder }}
- name: Initialize CodeQL
uses: github/codeql-action/init@erik-krogh/ql
with:
languages: ql
db-location: ${{ runner.temp }}/db
config-file: ./ql-for-ql-config.yml
- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@erik-krogh/ql
with:
category: "ql-for-ql-${{ matrix.folder }}"

View File

@@ -1,84 +0,0 @@
name: Collect database stats for QL for QL
on:
push:
branches: [main]
paths:
- ql/ql/src/ql.dbscheme
pull_request:
branches: [main]
paths:
- ql/ql/src/ql.dbscheme
workflow_dispatch:
jobs:
measure:
env:
CODEQL_THREADS: 4 # TODO: remove this once it's set by the CLI
strategy:
matrix:
repo:
- github/codeql
- github/codeql-go
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Find codeql
id: find-codeql
uses: github/codeql-action/init@erik-krogh/ql
with:
languages: javascript # does not matter
- uses: actions/cache@v2
with:
path: |
~/.cargo/registry
~/.cargo/git
ql/target
key: ${{ runner.os }}-qltest-cargo-${{ hashFiles('**/Cargo.lock') }}
- name: Build Extractor
run: cd ql; env "PATH=$PATH:`dirname ${CODEQL}`" ./create-extractor-pack.sh
env:
CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
- name: Checkout ${{ matrix.repo }}
uses: actions/checkout@v2
with:
repository: ${{ matrix.repo }}
path: ${{ github.workspace }}/repo
- name: Create database
run: |
"${CODEQL}" database create \
--search-path "ql/extractor-pack" \
--threads 4 \
--language ql --source-root "${{ github.workspace }}/repo" \
"${{ runner.temp }}/database"
env:
CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
- name: Measure database
run: |
mkdir -p "stats/${{ matrix.repo }}"
"${CODEQL}" dataset measure --threads 4 --output "stats/${{ matrix.repo }}/stats.xml" "${{ runner.temp }}/database/db-ql"
env:
CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
- uses: actions/upload-artifact@v2
with:
name: measurements
path: stats
retention-days: 1
merge:
runs-on: ubuntu-latest
needs: measure
steps:
- uses: actions/checkout@v2
- uses: actions/download-artifact@v2
with:
name: measurements
path: stats
- run: |
python -m pip install --user lxml
find stats -name 'stats.xml' -print0 | sort -z | xargs -0 python ql/scripts/merge_stats.py --output ql/ql/src/ql.dbscheme.stats --normalise ql_tokeninfo
- uses: actions/upload-artifact@v2
with:
name: ql.dbscheme.stats
path: ql/ql/src/ql.dbscheme.stats

View File

@@ -1,52 +0,0 @@
name: Run QL for QL Tests
on:
push:
branches: [main]
paths:
- "ql/**"
pull_request:
branches: [main]
paths:
- "ql/**"
env:
CARGO_TERM_COLOR: always
jobs:
qltest:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Find codeql
id: find-codeql
uses: github/codeql-action/init@erik-krogh/ql
with:
languages: javascript # does not matter
- uses: actions/cache@v2
with:
path: |
~/.cargo/registry
~/.cargo/git
ql/target
key: ${{ runner.os }}-qltest-cargo-${{ hashFiles('**/Cargo.lock') }}
- name: Build extractor
run: |
cd ql;
codeqlpath=$(dirname ${{ steps.find-codeql.outputs.codeql-path }});
env "PATH=$PATH:$codeqlpath" ./create-extractor-pack.sh
- name: Run QL tests
run: |
"${CODEQL}" test run --check-databases --check-unused-labels --check-repeated-labels --check-redefined-labels --check-use-before-definition --search-path "${{ github.workspace }}/ql/extractor-pack" --consistency-queries ql/ql/consistency-queries ql/ql/test
env:
CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
- name: Check QL formatting
run: |
find ql/ql "(" -name "*.ql" -or -name "*.qll" ")" -print0 | xargs -0 "${CODEQL}" query format --check-only
env:
CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
- name: Check QL compilation
run: |
"${CODEQL}" query compile --check-only --threads=4 --warnings=error --search-path "${{ github.workspace }}/ql/extractor-pack" "ql/ql/src" "ql/ql/examples"
env:
CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}

View File

@@ -25,6 +25,3 @@
/docs/codeql-for-visual-studio-code/ @github/codeql-vscode-reviewers
/docs/ql-language-reference/ @github/codeql-frontend-reviewers
/docs/query-*-style-guide.md @github/codeql-analysis-reviewers
# QL for QL reviewers
/ql/ @erik-krogh @tausbn

View File

@@ -452,15 +452,9 @@
"ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImplCommon.qll",
"cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll"
],
"CryptoAlgorithms Python/JS/Ruby": [
"CryptoAlgorithms Python/JS": [
"javascript/ql/lib/semmle/javascript/security/CryptoAlgorithms.qll",
"python/ql/lib/semmle/python/concepts/CryptoAlgorithms.qll",
"ruby/ql/lib/codeql/ruby/security/CryptoAlgorithms.qll"
],
"CryptoAlgorithmNames Python/JS/Ruby": [
"javascript/ql/lib/semmle/javascript/security/internal/CryptoAlgorithmNames.qll",
"python/ql/lib/semmle/python/concepts/internal/CryptoAlgorithmNames.qll",
"ruby/ql/lib/codeql/ruby/security/internal/CryptoAlgorithmNames.qll"
"python/ql/lib/semmle/python/concepts/CryptoAlgorithms.qll"
],
"SensitiveDataHeuristics Python/JS": [
"javascript/ql/lib/semmle/javascript/security/internal/SensitiveDataHeuristics.qll",

View File

@@ -17,7 +17,7 @@
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Build" Version="16.11.0" />
<PackageReference Include="Microsoft.Build" Version="16.9.0" />
</ItemGroup>
<ItemGroup>

View File

@@ -0,0 +1,4 @@
lgtm,codescanning
* The QL library `semmle.code.cpp.commons.Exclusions` now contains a predicate
`isFromSystemMacroDefinition` for identifying code that originates from a
macro outside the project being analyzed.

View File

@@ -0,0 +1,2 @@
lgtm,codescanning
* A new query `cpp/non-https-url` has been added for C/C++. The query flags uses of `http` URLs that might be better replaced with `https`.

View File

@@ -0,0 +1,2 @@
lgtm,codescanning
* A new query `cpp/certificate-not-checked` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.

View File

@@ -0,0 +1,2 @@
lgtm,codescanning
* A new query `cpp/certificate-result-conflation` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.

View File

@@ -1,2 +0,0 @@
lgtm,codescanning
* The "Cleartext transmission of sensitive information" (`cpp/cleartext-transmission`) query has been improved, reducing the number of false positive results when encryption is present.

View File

@@ -1,7 +1,3 @@
## 0.0.7
## 0.0.6
## 0.0.5
## 0.0.4

View File

@@ -73,7 +73,7 @@ class Options extends string {
* __assume(0);
* ```
* (note that in this case if the hint is wrong and the expression is reached at
* runtime, the program's behavior is undefined)
* runtime, the program's behaviour is undefined)
*/
predicate exprExits(Expr e) {
e.(AssumeExpr).getChild(0).(CompileTimeConstantInt).getIntValue() = 0 or

View File

@@ -50,7 +50,7 @@ class CustomOptions extends Options {
* __assume(0);
* ```
* (note that in this case if the hint is wrong and the expression is reached at
* runtime, the program's behavior is undefined)
* runtime, the program's behaviour is undefined)
*/
override predicate exprExits(Expr e) { Options.super.exprExits(e) }

View File

@@ -1 +0,0 @@
## 0.0.6

View File

@@ -1 +0,0 @@
## 0.0.7

View File

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

View File

@@ -37,7 +37,7 @@ abstract class SimpleRangeAnalysisDefinition extends RangeSsaDefinition {
* dependencies. Without this information, range analysis might work for
* simple cases but will go into infinite loops on complex code.
*
* For example, when modeling the definition by reference in a call to an
* For example, when modelling the definition by reference in a call to an
* overloaded `operator=`, written as `v = e`, the definition of `(this, v)`
* depends on `e`.
*/

View File

@@ -5,7 +5,7 @@
* `Instruction` level), and then using the array length analysis and the range
* analysis together to prove that some of these pointer dereferences are safe.
*
* The analysis is soundy, i.e. it is sound if no undefined behavior is present
* The analysis is soundy, i.e. it is sound if no undefined behaviour is present
* in the program.
* Furthermore, it crucially depends on the soundiness of the range analysis and
* the array length analysis.

View File

@@ -1,8 +1,8 @@
name: codeql/cpp-all
version: 0.0.8-dev
version: 0.0.5
groups: cpp
dbscheme: semmlecode.cpp.dbscheme
extractor: cpp
library: true
dependencies:
codeql/cpp-upgrades: ^0.0.3
codeql/cpp-upgrades: 0.0.3

View File

@@ -9,83 +9,6 @@ import semmle.code.cpp.models.interfaces.FormattingFunction
private import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
private import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
private newtype TBufferWriteEstimationReason =
TNoSpecifiedEstimateReason() or
TTypeBoundsAnalysis() or
TValueFlowAnalysis()
/**
* A reason for a specific buffer write size estimate.
*/
abstract class BufferWriteEstimationReason extends TBufferWriteEstimationReason {
/**
* Returns the name of the concrete class.
*/
abstract string toString();
/**
* Returns a human readable representation of this reason.
*/
abstract string getDescription();
/**
* Combine estimate reasons. Used to give a reason for the size of a format string
* conversion given reasons coming from its individual specifiers.
*/
abstract BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other);
}
/**
* No particular reason given. This is currently used for backward compatibility so that
* classes derived from BufferWrite and overriding `getMaxData/0` still work with the
* queries as intended.
*/
class NoSpecifiedEstimateReason extends BufferWriteEstimationReason, TNoSpecifiedEstimateReason {
override string toString() { result = "NoSpecifiedEstimateReason" }
override string getDescription() { result = "no reason specified" }
override BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
// this reason should not be used in format specifiers, so it should not be combined
// with other reasons
none()
}
}
/**
* The estimation comes from rough bounds just based on the type (e.g.
* `0 <= x < 2^32` for an unsigned 32 bit integer).
*/
class TypeBoundsAnalysis extends BufferWriteEstimationReason, TTypeBoundsAnalysis {
override string toString() { result = "TypeBoundsAnalysis" }
override string getDescription() { result = "based on type bounds" }
override BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
other != TNoSpecifiedEstimateReason() and result = TTypeBoundsAnalysis()
}
}
/**
* The estimation comes from non trivial bounds found via actual flow analysis.
* For example
* ```
* unsigned u = x;
* if (u < 1000) {
* //... <- estimation done here based on u
* }
* ```
*/
class ValueFlowAnalysis extends BufferWriteEstimationReason, TValueFlowAnalysis {
override string toString() { result = "ValueFlowAnalysis" }
override string getDescription() { result = "based on flow analysis of value bounds" }
override BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
other != TNoSpecifiedEstimateReason() and result = other
}
}
class PrintfFormatAttribute extends FormatAttribute {
PrintfFormatAttribute() { this.getArchetype() = ["printf", "__printf__"] }
}
@@ -1067,14 +990,7 @@ class FormatLiteral extends Literal {
* conversion specifier of this format string; has no result if this cannot
* be determined.
*/
int getMaxConvertedLength(int n) { result = max(getMaxConvertedLength(n, _)) }
/**
* Gets the maximum length of the string that can be produced by the nth
* conversion specifier of this format string, specifying the estimation reason;
* has no result if this cannot be determined.
*/
int getMaxConvertedLength(int n, BufferWriteEstimationReason reason) {
int getMaxConvertedLength(int n) {
exists(int len |
(
(
@@ -1086,12 +1002,10 @@ class FormatLiteral extends Literal {
) and
(
this.getConversionChar(n) = "%" and
len = 1 and
reason = TValueFlowAnalysis()
len = 1
or
this.getConversionChar(n).toLowerCase() = "c" and
len = 1 and
reason = TValueFlowAnalysis() // e.g. 'a'
len = 1 // e.g. 'a'
or
this.getConversionChar(n).toLowerCase() = "f" and
exists(int dot, int afterdot |
@@ -1105,8 +1019,7 @@ class FormatLiteral extends Literal {
afterdot = 6
) and
len = 1 + 309 + dot + afterdot
) and
reason = TTypeBoundsAnalysis() // e.g. -1e308="-100000"...
) // e.g. -1e308="-100000"...
or
this.getConversionChar(n).toLowerCase() = "e" and
exists(int dot, int afterdot |
@@ -1120,8 +1033,7 @@ class FormatLiteral extends Literal {
afterdot = 6
) and
len = 1 + 1 + dot + afterdot + 1 + 1 + 3
) and
reason = TTypeBoundsAnalysis() // -1e308="-1.000000e+308"
) // -1e308="-1.000000e+308"
or
this.getConversionChar(n).toLowerCase() = "g" and
exists(int dot, int afterdot |
@@ -1144,80 +1056,67 @@ class FormatLiteral extends Literal {
// (e.g. 123456, 0.000123456 are just OK)
// so case %f can be at most P characters + 4 zeroes, sign, dot = P + 6
len = (afterdot.maximum(1) + 6).maximum(1 + 1 + dot + afterdot + 1 + 1 + 3)
) and
reason = TTypeBoundsAnalysis() // (e.g. "-1.59203e-319")
) // (e.g. "-1.59203e-319")
or
this.getConversionChar(n).toLowerCase() = ["d", "i"] and
// e.g. -2^31 = "-2147483648"
exists(float typeBasedBound, float valueBasedBound |
// The first case handles length sub-specifiers
// Subtract one in the exponent because one bit is for the sign.
// Add 1 to account for the possible sign in the output.
typeBasedBound =
1 + lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8 - 1)) and
// The second case uses range analysis to deduce a length that's shorter than the length
// of the number -2^31.
exists(Expr arg, float lower, float upper, float typeLower, float typeUpper |
arg = this.getUse().getConversionArgument(n) and
lower = lowerBound(arg.getFullyConverted()) and
upper = upperBound(arg.getFullyConverted()) and
typeLower = exprMinVal(arg.getFullyConverted()) and
typeUpper = exprMaxVal(arg.getFullyConverted())
|
valueBasedBound =
max(int cand |
// Include the sign bit in the length if it can be negative
(
if lower < 0
then cand = 1 + lengthInBase10(lower.abs())
else cand = lengthInBase10(lower)
len =
min(float cand |
// The first case handles length sub-specifiers
// Subtract one in the exponent because one bit is for the sign.
// Add 1 to account for the possible sign in the output.
cand = 1 + lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8 - 1))
or
// The second case uses range analysis to deduce a length that's shorter than the length
// of the number -2^31.
exists(Expr arg, float lower, float upper |
arg = this.getUse().getConversionArgument(n) and
lower = lowerBound(arg.getFullyConverted()) and
upper = upperBound(arg.getFullyConverted())
|
cand =
max(int cand0 |
// Include the sign bit in the length if it can be negative
(
if lower < 0
then cand0 = 1 + lengthInBase10(lower.abs())
else cand0 = lengthInBase10(lower)
)
or
(
if upper < 0
then cand0 = 1 + lengthInBase10(upper.abs())
else cand0 = lengthInBase10(upper)
)
)
or
(
if upper < 0
then cand = 1 + lengthInBase10(upper.abs())
else cand = lengthInBase10(upper)
)
) and
(
if lower > typeLower or upper < typeUpper
then reason = TValueFlowAnalysis()
else reason = TTypeBoundsAnalysis()
)
) and
len = valueBasedBound.minimum(typeBasedBound)
)
)
or
this.getConversionChar(n).toLowerCase() = "u" and
// e.g. 2^32 - 1 = "4294967295"
exists(float typeBasedBound, float valueBasedBound |
// The first case handles length sub-specifiers
typeBasedBound = lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8) - 1) and
// The second case uses range analysis to deduce a length that's shorter than
// the length of the number 2^31 - 1.
exists(Expr arg, float lower, float upper, float typeLower, float typeUpper |
arg = this.getUse().getConversionArgument(n) and
lower = lowerBound(arg.getFullyConverted()) and
upper = upperBound(arg.getFullyConverted()) and
typeLower = exprMinVal(arg.getFullyConverted()) and
typeUpper = exprMaxVal(arg.getFullyConverted())
|
valueBasedBound =
lengthInBase10(max(float cand |
len =
min(float cand |
// The first case handles length sub-specifiers
cand = 2.pow(this.getIntegralDisplayType(n).getSize() * 8)
or
// The second case uses range analysis to deduce a length that's shorter than
// the length of the number 2^31 - 1.
exists(Expr arg, float lower |
arg = this.getUse().getConversionArgument(n) and
lower = lowerBound(arg.getFullyConverted())
|
cand =
max(float cand0 |
// If lower can be negative we use `(unsigned)-1` as the candidate value.
lower < 0 and
cand = 2.pow(any(IntType t | t.isUnsigned()).getSize() * 8)
cand0 = 2.pow(any(IntType t | t.isUnsigned()).getSize() * 8)
or
cand = upper
)) and
(
if lower > typeLower or upper < typeUpper
then reason = TValueFlowAnalysis()
else reason = TTypeBoundsAnalysis()
cand0 = upperBound(arg.getFullyConverted())
)
)
) and
len = valueBasedBound.minimum(typeBasedBound)
)
|
lengthInBase10(cand)
)
or
this.getConversionChar(n).toLowerCase() = "x" and
// e.g. "12345678"
@@ -1236,8 +1135,7 @@ class FormatLiteral extends Literal {
(
if this.hasAlternateFlag(n) then len = 2 + baseLen else len = baseLen // "0x"
)
) and
reason = TTypeBoundsAnalysis()
)
or
this.getConversionChar(n).toLowerCase() = "p" and
exists(PointerType ptrType, int baseLen |
@@ -1246,8 +1144,7 @@ class FormatLiteral extends Literal {
(
if this.hasAlternateFlag(n) then len = 2 + baseLen else len = baseLen // "0x"
)
) and
reason = TValueFlowAnalysis()
)
or
this.getConversionChar(n).toLowerCase() = "o" and
// e.g. 2^32 - 1 = "37777777777"
@@ -1266,16 +1163,14 @@ class FormatLiteral extends Literal {
(
if this.hasAlternateFlag(n) then len = 1 + baseLen else len = baseLen // "0"
)
) and
reason = TTypeBoundsAnalysis()
)
or
this.getConversionChar(n).toLowerCase() = "s" and
len =
min(int v |
v = this.getPrecision(n) or
v = this.getUse().getFormatArgument(n).(AnalysedString).getMaxLength() - 1 // (don't count null terminator)
) and
reason = TValueFlowAnalysis()
)
)
)
}
@@ -1287,19 +1182,10 @@ class FormatLiteral extends Literal {
* determining whether a buffer overflow is caused by long float to string
* conversions.
*/
int getMaxConvertedLengthLimited(int n) { result = max(getMaxConvertedLengthLimited(n, _)) }
/**
* Gets the maximum length of the string that can be produced by the nth
* conversion specifier of this format string, specifying the reason for the
* estimation, except that float to string conversions are assumed to be 8
* characters. This is helpful for determining whether a buffer overflow is
* caused by long float to string conversions.
*/
int getMaxConvertedLengthLimited(int n, BufferWriteEstimationReason reason) {
int getMaxConvertedLengthLimited(int n) {
if this.getConversionChar(n).toLowerCase() = "f"
then result = this.getMaxConvertedLength(n, reason).minimum(8)
else result = this.getMaxConvertedLength(n, reason)
then result = this.getMaxConvertedLength(n).minimum(8)
else result = this.getMaxConvertedLength(n)
}
/**
@@ -1339,35 +1225,29 @@ class FormatLiteral extends Literal {
)
}
private int getMaxConvertedLengthAfter(int n, BufferWriteEstimationReason reason) {
private int getMaxConvertedLengthAfter(int n) {
if n = this.getNumConvSpec()
then result = this.getConstantSuffix().length() + 1 and reason = TValueFlowAnalysis()
then result = this.getConstantSuffix().length() + 1
else
exists(BufferWriteEstimationReason headReason, BufferWriteEstimationReason tailReason |
result =
this.getConstantPart(n).length() + this.getMaxConvertedLength(n, headReason) +
this.getMaxConvertedLengthAfter(n + 1, tailReason) and
reason = headReason.combineWith(tailReason)
)
result =
this.getConstantPart(n).length() + this.getMaxConvertedLength(n) +
this.getMaxConvertedLengthAfter(n + 1)
}
private int getMaxConvertedLengthAfterLimited(int n, BufferWriteEstimationReason reason) {
private int getMaxConvertedLengthAfterLimited(int n) {
if n = this.getNumConvSpec()
then result = this.getConstantSuffix().length() + 1 and reason = TValueFlowAnalysis()
then result = this.getConstantSuffix().length() + 1
else
exists(BufferWriteEstimationReason headReason, BufferWriteEstimationReason tailReason |
result =
this.getConstantPart(n).length() + this.getMaxConvertedLengthLimited(n, headReason) +
this.getMaxConvertedLengthAfterLimited(n + 1, tailReason) and
reason = headReason.combineWith(tailReason)
)
result =
this.getConstantPart(n).length() + this.getMaxConvertedLengthLimited(n) +
this.getMaxConvertedLengthAfterLimited(n + 1)
}
/**
* Gets the maximum length of the string that can be produced by this format
* string. Has no result if this cannot be determined.
*/
int getMaxConvertedLength() { result = this.getMaxConvertedLengthAfter(0, _) }
int getMaxConvertedLength() { result = this.getMaxConvertedLengthAfter(0) }
/**
* Gets the maximum length of the string that can be produced by this format
@@ -1375,24 +1255,5 @@ class FormatLiteral extends Literal {
* characters. This is helpful for determining whether a buffer overflow
* is caused by long float to string conversions.
*/
int getMaxConvertedLengthLimited() { result = this.getMaxConvertedLengthAfterLimited(0, _) }
/**
* Gets the maximum length of the string that can be produced by this format
* string, specifying the reason for the estimate. Has no result if no estimate
* can be found.
*/
int getMaxConvertedLengthWithReason(BufferWriteEstimationReason reason) {
result = this.getMaxConvertedLengthAfter(0, reason)
}
/**
* Gets the maximum length of the string that can be produced by this format
* string, specifying the reason for the estimate, except that float to string
* conversions are assumed to be 8 characters. This is helpful for determining
* whether a buffer overflow is caused by long float to string conversions.
*/
int getMaxConvertedLengthLimitedWithReason(BufferWriteEstimationReason reason) {
result = this.getMaxConvertedLengthAfterLimited(0, reason)
}
int getMaxConvertedLengthLimited() { result = this.getMaxConvertedLengthAfterLimited(0) }
}

View File

@@ -153,11 +153,9 @@ library class SSAHelper extends int {
* Modern Compiler Implementation by Andrew Appel.
*/
private predicate frontier_phi_node(StackVariable v, BasicBlock b) {
exists(BasicBlock x |
dominanceFrontier(x, b) and ssa_defn_rec(pragma[only_bind_into](v), pragma[only_bind_into](x))
) and
exists(BasicBlock x | dominanceFrontier(x, b) and ssa_defn_rec(v, x)) and
/* We can also eliminate those nodes where the variable is not live on any incoming edge */
live_at_start_of_bb(pragma[only_bind_into](v), b)
live_at_start_of_bb(v, b)
}
private predicate ssa_defn_rec(StackVariable v, BasicBlock b) {

View File

@@ -15,23 +15,11 @@ module Consistency {
class ConsistencyConfiguration extends TConsistencyConfiguration {
string toString() { none() }
/** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
predicate uniqueEnclosingCallableExclude(Node n) { none() }
/** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
predicate uniqueNodeLocationExclude(Node n) { none() }
/** Holds if `n` should be excluded from the consistency test `missingLocation`. */
predicate missingLocationExclude(Node n) { none() }
/** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
predicate postWithInFlowExclude(Node n) { none() }
/** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
/** Holds if `n` should be excluded from the consistency test `reverseRead`. */
predicate reverseReadExclude(Node n) { none() }
}
private class RelevantNode extends Node {
@@ -58,7 +46,6 @@ module Consistency {
n instanceof RelevantNode and
c = count(nodeGetEnclosingCallable(n)) and
c != 1 and
not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and
msg = "Node should have one enclosing callable but has " + c + "."
)
}
@@ -79,7 +66,6 @@ module Consistency {
n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
) and
c != 1 and
not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and
msg = "Node should have one location but has " + c + "."
)
}
@@ -90,8 +76,7 @@ module Consistency {
strictcount(Node n |
not exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
) and
not any(ConsistencyConfiguration conf).missingLocationExclude(n)
)
) and
msg = "Nodes without location: " + c
)
@@ -187,7 +172,6 @@ module Consistency {
query predicate reverseRead(Node n, string msg) {
exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and
not any(ConsistencyConfiguration conf).reverseReadExclude(n) and
msg = "Origin of readStep is missing a PostUpdateNode."
}

View File

@@ -118,7 +118,7 @@ class LambdaCapture extends Locatable, @lambdacapture {
* An identifier is captured by reference if:
* - It is explicitly captured by reference.
* - It is implicitly captured, and the lambda's default capture mode is by-reference.
* - The identifier is "this". [Said behavior is dictated by the C++11 standard, but it
* - The identifier is "this". [Said behaviour is dictated by the C++11 standard, but it
* is actually "*this" being captured rather than "this".]
*/
predicate isCapturedByReference() { lambda_capture(this, _, _, _, true, _, _) }

View File

@@ -15,23 +15,11 @@ module Consistency {
class ConsistencyConfiguration extends TConsistencyConfiguration {
string toString() { none() }
/** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
predicate uniqueEnclosingCallableExclude(Node n) { none() }
/** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
predicate uniqueNodeLocationExclude(Node n) { none() }
/** Holds if `n` should be excluded from the consistency test `missingLocation`. */
predicate missingLocationExclude(Node n) { none() }
/** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
predicate postWithInFlowExclude(Node n) { none() }
/** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
/** Holds if `n` should be excluded from the consistency test `reverseRead`. */
predicate reverseReadExclude(Node n) { none() }
}
private class RelevantNode extends Node {
@@ -58,7 +46,6 @@ module Consistency {
n instanceof RelevantNode and
c = count(nodeGetEnclosingCallable(n)) and
c != 1 and
not any(ConsistencyConfiguration conf).uniqueEnclosingCallableExclude(n) and
msg = "Node should have one enclosing callable but has " + c + "."
)
}
@@ -79,7 +66,6 @@ module Consistency {
n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
) and
c != 1 and
not any(ConsistencyConfiguration conf).uniqueNodeLocationExclude(n) and
msg = "Node should have one location but has " + c + "."
)
}
@@ -90,8 +76,7 @@ module Consistency {
strictcount(Node n |
not exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
) and
not any(ConsistencyConfiguration conf).missingLocationExclude(n)
)
) and
msg = "Nodes without location: " + c
)
@@ -187,7 +172,6 @@ module Consistency {
query predicate reverseRead(Node n, string msg) {
exists(Node n2 | readStep(n, _, n2) and hasPost(n2) and not hasPost(n)) and
not any(ConsistencyConfiguration conf).reverseReadExclude(n) and
msg = "Origin of readStep is missing a PostUpdateNode."
}

View File

@@ -266,20 +266,6 @@ private predicate operandReturned(Operand operand, IntValue bitOffset) {
bitOffset = Ints::unknown()
}
pragma[nomagic]
private predicate initializeParameterInstructionHasVariable(
IRVariable var, InitializeParameterInstruction init
) {
init.getIRVariable() = var
}
private predicate instructionInitializesThisInFunction(
Language::Function f, InitializeParameterInstruction init
) {
initializeParameterInstructionHasVariable(any(IRThisVariable var), pragma[only_bind_into](init)) and
init.getEnclosingFunction() = f
}
private predicate isArgumentForParameter(
CallInstruction ci, Operand operand, InitializeParameterInstruction init
) {
@@ -289,7 +275,8 @@ private predicate isArgumentForParameter(
(
init.getParameter() = f.getParameter(operand.(PositionalArgumentOperand).getIndex())
or
instructionInitializesThisInFunction(f, init) and
init.getIRVariable() instanceof IRThisVariable and
unique( | | init.getEnclosingFunction()) = f and
operand instanceof ThisArgumentOperand
) and
not Language::isFunctionVirtual(f) and

View File

@@ -266,20 +266,6 @@ private predicate operandReturned(Operand operand, IntValue bitOffset) {
bitOffset = Ints::unknown()
}
pragma[nomagic]
private predicate initializeParameterInstructionHasVariable(
IRVariable var, InitializeParameterInstruction init
) {
init.getIRVariable() = var
}
private predicate instructionInitializesThisInFunction(
Language::Function f, InitializeParameterInstruction init
) {
initializeParameterInstructionHasVariable(any(IRThisVariable var), pragma[only_bind_into](init)) and
init.getEnclosingFunction() = f
}
private predicate isArgumentForParameter(
CallInstruction ci, Operand operand, InitializeParameterInstruction init
) {
@@ -289,7 +275,8 @@ private predicate isArgumentForParameter(
(
init.getParameter() = f.getParameter(operand.(PositionalArgumentOperand).getIndex())
or
instructionInitializesThisInFunction(f, init) and
init.getIRVariable() instanceof IRThisVariable and
unique( | | init.getEnclosingFunction()) = f and
operand instanceof ThisArgumentOperand
) and
not Language::isFunctionVirtual(f) and

View File

@@ -71,30 +71,13 @@ abstract class BufferWrite extends Expr {
*/
int getMaxData() { none() }
/**
* Gets an upper bound to the amount of data that's being written (if one
* can be found), specifying the reason for the estimation.
*/
int getMaxData(BufferWriteEstimationReason reason) {
reason instanceof NoSpecifiedEstimateReason and result = getMaxData()
}
/**
* Gets an upper bound to the amount of data that's being written (if one
* can be found), except that float to string conversions are assumed to be
* much smaller (8 bytes) than their true maximum length. This can be
* helpful in determining the cause of a buffer overflow issue.
*/
int getMaxDataLimited() { result = getMaxData() }
/**
* Gets an upper bound to the amount of data that's being written (if one
* can be found), specifying the reason for the estimation, except that
* float to string conversions are assumed to be much smaller (8 bytes)
* than their true maximum length. This can be helpful in determining the
* cause of a buffer overflow issue.
*/
int getMaxDataLimited(BufferWriteEstimationReason reason) { result = getMaxData(reason) }
int getMaxDataLimited() { result = this.getMaxData() }
/**
* Gets the size of a single character of the type this
@@ -152,16 +135,10 @@ class StrCopyBW extends BufferWriteCall {
result = this.getArgument(this.getParamSize()).getValue().toInt() * this.getCharSize()
}
private int getMaxDataImpl(BufferWriteEstimationReason reason) {
// when result exists, it is an exact flow analysis
reason instanceof ValueFlowAnalysis and
override int getMaxData() {
result =
this.getArgument(this.getParamSrc()).(AnalysedString).getMaxLength() * this.getCharSize()
}
override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
override int getMaxData() { result = max(getMaxDataImpl(_)) }
}
/**
@@ -196,16 +173,10 @@ class StrCatBW extends BufferWriteCall {
result = this.getArgument(this.getParamSize()).getValue().toInt() * this.getCharSize()
}
private int getMaxDataImpl(BufferWriteEstimationReason reason) {
// when result exists, it is an exact flow analysis
reason instanceof ValueFlowAnalysis and
override int getMaxData() {
result =
this.getArgument(this.getParamSrc()).(AnalysedString).getMaxLength() * this.getCharSize()
}
override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
override int getMaxData() { result = max(getMaxDataImpl(_)) }
}
/**
@@ -262,29 +233,19 @@ class SprintfBW extends BufferWriteCall {
override Expr getDest() { result = this.getArgument(f.getOutputParameterIndex(false)) }
private int getMaxDataImpl(BufferWriteEstimationReason reason) {
override int getMaxData() {
exists(FormatLiteral fl |
fl = this.(FormattingFunctionCall).getFormat() and
result = fl.getMaxConvertedLengthWithReason(reason) * this.getCharSize()
result = fl.getMaxConvertedLength() * this.getCharSize()
)
}
override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
override int getMaxData() { result = max(getMaxDataImpl(_)) }
private int getMaxDataLimitedImpl(BufferWriteEstimationReason reason) {
override int getMaxDataLimited() {
exists(FormatLiteral fl |
fl = this.(FormattingFunctionCall).getFormat() and
result = fl.getMaxConvertedLengthLimitedWithReason(reason) * this.getCharSize()
result = fl.getMaxConvertedLengthLimited() * this.getCharSize()
)
}
override int getMaxDataLimited(BufferWriteEstimationReason reason) {
result = getMaxDataLimitedImpl(reason)
}
override int getMaxDataLimited() { result = max(getMaxDataLimitedImpl(_)) }
}
/**
@@ -375,29 +336,19 @@ class SnprintfBW extends BufferWriteCall {
result = this.getArgument(this.getParamSize()).getValue().toInt() * this.getCharSize()
}
private int getMaxDataImpl(BufferWriteEstimationReason reason) {
override int getMaxData() {
exists(FormatLiteral fl |
fl = this.(FormattingFunctionCall).getFormat() and
result = fl.getMaxConvertedLengthWithReason(reason) * this.getCharSize()
result = fl.getMaxConvertedLength() * this.getCharSize()
)
}
override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
override int getMaxData() { result = max(getMaxDataImpl(_)) }
private int getMaxDataLimitedImpl(BufferWriteEstimationReason reason) {
override int getMaxDataLimited() {
exists(FormatLiteral fl |
fl = this.(FormattingFunctionCall).getFormat() and
result = fl.getMaxConvertedLengthLimitedWithReason(reason) * this.getCharSize()
result = fl.getMaxConvertedLengthLimited() * this.getCharSize()
)
}
override int getMaxDataLimited(BufferWriteEstimationReason reason) {
result = getMaxDataLimitedImpl(reason)
}
override int getMaxDataLimited() { result = max(getMaxDataLimitedImpl(_)) }
}
/**
@@ -485,9 +436,7 @@ class ScanfBW extends BufferWrite {
override Expr getDest() { result = this }
private int getMaxDataImpl(BufferWriteEstimationReason reason) {
// when this returns, it is based on exact flow analysis
reason instanceof ValueFlowAnalysis and
override int getMaxData() {
exists(ScanfFunctionCall fc, ScanfFormatLiteral fl, int arg |
this = fc.getArgument(arg) and
fl = fc.getFormat() and
@@ -495,10 +444,6 @@ class ScanfBW extends BufferWrite {
)
}
override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
override int getMaxData() { result = max(getMaxDataImpl(_)) }
override string getBWDesc() {
exists(FunctionCall fc |
this = fc.getArgument(_) and
@@ -529,14 +474,8 @@ class RealpathBW extends BufferWriteCall {
override Expr getASource() { result = this.getArgument(0) }
private int getMaxDataImpl(BufferWriteEstimationReason reason) {
// although there may be some unknown invariants guaranteeing that a real path is shorter than PATH_MAX, we can consider providing less than PATH_MAX a problem with high precision
reason instanceof ValueFlowAnalysis and
override int getMaxData() {
result = path_max() and
this = this // Suppress a compiler warning
}
override int getMaxData(BufferWriteEstimationReason reason) { result = getMaxDataImpl(reason) }
override int getMaxData() { result = max(getMaxDataImpl(_)) }
}

View File

@@ -1,14 +1,5 @@
## 0.0.7
## 0.0.6
## 0.0.5
### New Queries
* A new query `cpp/certificate-not-checked` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.
* A new query `cpp/certificate-result-conflation` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.
## 0.0.4
### New Queries

View File

@@ -85,8 +85,7 @@ private predicate cancelingSubExprs(ComparisonOperation cmp, VariableAccess a1,
exists(Variable v |
exists(float m | m < 0 and cmpLinearSubVariable(cmp, v, a1, m)) and
exists(float m | m > 0 and cmpLinearSubVariable(cmp, v, a2, m))
) and
not any(ClassTemplateInstantiation inst).getATemplateArgument() = cmp.getParent*()
)
}
from ComparisonOperation cmp, VariableAccess a1, VariableAccess a2

View File

@@ -29,9 +29,7 @@ predicate pointlessSelfComparison(ComparisonOperation cmp) {
not exists(lhs.getQualifier()) and // Avoid structure fields
not exists(rhs.getQualifier()) and // Avoid structure fields
not convertedExprMightOverflow(lhs) and
not convertedExprMightOverflow(rhs) and
// Don't warn if the comparison is part of a template argument.
not any(ClassTemplateInstantiation inst).getATemplateArgument() = cmp.getParent*()
not convertedExprMightOverflow(rhs)
)
}

View File

@@ -3,7 +3,7 @@
"qhelp.dtd">
<qhelp>
<overview>
<p>The return value of a call to <code>snprintf</code> is the number of characters that <i>would have</i> been written to the buffer assuming there was sufficient space. In the event that the operation reaches the end of the buffer and more than one character is discarded, the return value will be greater than the buffer size. This can cause incorrect behavior, for example:
<p>The return value of a call to <code>snprintf</code> is the number of characters that <i>would have</i> been written to the buffer assuming there was sufficient space. In the event that the operation reaches the end of the buffer and more than one character is discarded, the return value will be greater than the buffer size. This can cause incorrect behaviour, for example:
</p>
</overview>

View File

@@ -21,15 +21,14 @@ import semmle.code.cpp.commons.Alloc
* See CWE-120/UnboundedWrite.ql for a summary of CWE-120 alert cases.
*/
from BufferWrite bw, Expr dest, int destSize, int estimated
from BufferWrite bw, Expr dest, int destSize
where
not bw.hasExplicitLimit() and // has no explicit size limit
dest = bw.getDest() and
destSize = getBufferSize(dest, _) and
estimated = bw.getMaxDataLimited(_) and
// we can deduce that too much data may be copied (even without
// long '%f' conversions)
estimated > destSize
bw.getMaxDataLimited() > destSize
select bw,
"This '" + bw.getBWDesc() + "' operation requires " + estimated +
"This '" + bw.getBWDesc() + "' operation requires " + bw.getMaxData() +
" bytes but the destination is only " + destSize + " bytes."

View File

@@ -21,15 +21,14 @@ import semmle.code.cpp.security.BufferWrite
* See CWE-120/UnboundedWrite.ql for a summary of CWE-120 alert cases.
*/
from BufferWrite bw, int destSize, int estimated, BufferWriteEstimationReason reason
from BufferWrite bw, int destSize
where
not bw.hasExplicitLimit() and
// has no explicit size limit
destSize = getBufferSize(bw.getDest(), _) and
estimated = bw.getMaxData(reason) and
estimated > destSize and
bw.getMaxData() > destSize and
// and we can deduce that too much data may be copied
bw.getMaxDataLimited(reason) <= destSize // but it would fit without long '%f' conversions
bw.getMaxDataLimited() <= destSize // but it would fit without long '%f' conversions
select bw,
"This '" + bw.getBWDesc() + "' operation may require " + estimated +
"This '" + bw.getBWDesc() + "' operation may require " + bw.getMaxData() +
" bytes because of float conversions, but the target is only " + destSize + " bytes."

View File

@@ -44,7 +44,7 @@ import TaintedWithPath
predicate isUnboundedWrite(BufferWrite bw) {
not bw.hasExplicitLimit() and // has no explicit size limit
not exists(bw.getMaxData(_)) // and we can't deduce an upper bound to the amount copied
not exists(bw.getMaxData()) // and we can't deduce an upper bound to the amount copied
}
/*

View File

@@ -22,13 +22,11 @@ import semmle.code.cpp.dataflow.DataFlow
* Holds if `sub` is guarded by a condition which ensures that
* `left >= right`.
*/
pragma[nomagic]
pragma[noinline]
predicate isGuarded(SubExpr sub, Expr left, Expr right) {
exprIsSubLeftOrLess(pragma[only_bind_into](sub), _) and // Manual magic
exists(GuardCondition guard, int k, BasicBlock bb |
pragma[only_bind_into](bb) = sub.getBasicBlock() and
guard.controls(pragma[only_bind_into](bb), _) and
guard.ensuresLt(left, right, k, bb, false) and
exists(GuardCondition guard, int k |
guard.controls(sub.getBasicBlock(), _) and
guard.ensuresLt(left, right, k, sub.getBasicBlock(), false) and
k >= 0
)
}
@@ -38,56 +36,47 @@ predicate isGuarded(SubExpr sub, Expr left, Expr right) {
* `sub.getLeftOperand()`.
*/
predicate exprIsSubLeftOrLess(SubExpr sub, DataFlow::Node n) {
interestingSubExpr(sub, _) and // Manual magic
(
n.asExpr() = sub.getLeftOperand()
or
exists(DataFlow::Node other |
// dataflow
exprIsSubLeftOrLess(sub, other) and
(
DataFlow::localFlowStep(n, other) or
DataFlow::localFlowStep(other, n)
)
)
or
exists(DataFlow::Node other |
// guard constraining `sub`
exprIsSubLeftOrLess(sub, other) and
isGuarded(sub, other.asExpr(), n.asExpr()) // other >= n
)
or
exists(DataFlow::Node other, float p, float q |
// linear access of `other`
exprIsSubLeftOrLess(sub, other) and
linearAccess(n.asExpr(), other.asExpr(), p, q) and // n = p * other + q
p <= 1 and
q <= 0
)
or
exists(DataFlow::Node other, float p, float q |
// linear access of `n`
exprIsSubLeftOrLess(sub, other) and
linearAccess(other.asExpr(), n.asExpr(), p, q) and // other = p * n + q
p >= 1 and
q >= 0
n.asExpr() = sub.getLeftOperand()
or
exists(DataFlow::Node other |
// dataflow
exprIsSubLeftOrLess(sub, other) and
(
DataFlow::localFlowStep(n, other) or
DataFlow::localFlowStep(other, n)
)
)
}
predicate interestingSubExpr(SubExpr sub, RelationalOperation ro) {
not isFromMacroDefinition(sub) and
ro.getLesserOperand().getValue().toInt() = 0 and
ro.getGreaterOperand() = sub and
sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned() and
// generally catches false positives involving constants
exprMightOverflowNegatively(sub.getFullyConverted())
or
exists(DataFlow::Node other |
// guard constraining `sub`
exprIsSubLeftOrLess(sub, other) and
isGuarded(sub, other.asExpr(), n.asExpr()) // other >= n
)
or
exists(DataFlow::Node other, float p, float q |
// linear access of `other`
exprIsSubLeftOrLess(sub, other) and
linearAccess(n.asExpr(), other.asExpr(), p, q) and // n = p * other + q
p <= 1 and
q <= 0
)
or
exists(DataFlow::Node other, float p, float q |
// linear access of `n`
exprIsSubLeftOrLess(sub, other) and
linearAccess(other.asExpr(), n.asExpr(), p, q) and // other = p * n + q
p >= 1 and
q >= 0
)
}
from RelationalOperation ro, SubExpr sub
where
interestingSubExpr(sub, ro) and
not isFromMacroDefinition(ro) and
// generally catches false positives where there's a relation between the left and right operands
not exprIsSubLeftOrLess(sub, DataFlow::exprNode(sub.getRightOperand()))
not isFromMacroDefinition(sub) and
ro.getLesserOperand().getValue().toInt() = 0 and
ro.getGreaterOperand() = sub and
sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned() and
exprMightOverflowNegatively(sub.getFullyConverted()) and // generally catches false positives involving constants
not exprIsSubLeftOrLess(sub, DataFlow::exprNode(sub.getRightOperand())) // generally catches false positives where there's a relation between the left and right operands
select ro, "Unsigned subtraction can never be negative."

View File

@@ -14,188 +14,105 @@
import cpp
import semmle.code.cpp.security.SensitiveExprs
import semmle.code.cpp.dataflow.TaintTracking
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
import semmle.code.cpp.models.interfaces.FlowSource
import DataFlow::PathGraph
/**
* A DataFlow node corresponding to a variable or function call that
* might contain or return a password or other sensitive information.
*/
class SensitiveNode extends DataFlow::Node {
SensitiveNode() {
this.asExpr() = any(SensitiveVariable sv).getInitializer().getExpr() or
this.asExpr().(VariableAccess).getTarget() =
any(SensitiveVariable sv).(GlobalOrNamespaceVariable) or
this.asUninitialized() instanceof SensitiveVariable or
this.asParameter() instanceof SensitiveVariable or
this.asExpr().(FunctionCall).getTarget() instanceof SensitiveFunction
}
}
/**
* A function that sends or receives data over a network.
*/
abstract class SendRecv extends Function {
/**
* Gets the expression for the socket or similar object used for sending or
* receiving data through the function call `call` (if any).
*/
abstract Expr getSocketExpr(Call call);
/**
* Gets the expression for the buffer to be sent from / received into through
* the function call `call`.
*/
abstract Expr getDataExpr(Call call);
}
/**
* A function that sends data over a network.
*/
class Send extends SendRecv instanceof RemoteFlowSinkFunction {
override Expr getSocketExpr(Call call) {
call.getTarget() = this and
exists(FunctionInput input, int arg |
super.hasSocketInput(input) and
input.isParameter(arg) and
result = call.getArgument(arg)
)
}
override Expr getDataExpr(Call call) {
call.getTarget() = this and
exists(FunctionInput input, int arg |
super.hasRemoteFlowSink(input, _) and
input.isParameterDeref(arg) and
result = call.getArgument(arg)
)
}
}
/**
* A function that receives data over a network.
*/
class Recv extends SendRecv instanceof RemoteFlowSourceFunction {
override Expr getSocketExpr(Call call) {
call.getTarget() = this and
exists(FunctionInput input, int arg |
super.hasSocketInput(input) and
input.isParameter(arg) and
result = call.getArgument(arg)
)
}
override Expr getDataExpr(Call call) {
call.getTarget() = this and
exists(FunctionOutput output, int arg |
super.hasRemoteFlowSource(output, _) and
output.isParameterDeref(arg) and
result = call.getArgument(arg)
)
}
}
/**
* A function call that sends or receives data over a network.
*
* note: function calls such as `write` may be writing to a network source
* or a file. We could attempt to determine which, and sort results into
* `cpp/cleartext-transmission` and perhaps `cpp/cleartext-storage-file`. In
* practice it usually isn't very important which query reports a result as
* long as its reported exactly once.
*
* We do exclude function calls that specify a constant socket, which is
* likely to mean standard input, standard output or a similar channel.
*/
abstract class NetworkSendRecv extends FunctionCall {
SendRecv target;
/**
* Gets the expression for the socket or similar object used for sending or
* receiving data (if any).
*/
abstract Expr getSocketExpr();
NetworkSendRecv() {
this.getTarget() = target and
// exclude calls based on the socket...
not exists(GVN g |
g = globalValueNumber(target.getSocketExpr(this)) and
(
// literal constant
globalValueNumber(any(Literal l)) = g
or
// variable (such as a global) initialized to a literal constant
exists(Variable v |
v.getInitializer().getExpr() instanceof Literal and
g = globalValueNumber(v.getAnAccess())
)
)
)
}
final Expr getDataExpr() { result = target.getDataExpr(this) }
/**
* Gets the expression for the buffer to be sent from / received into.
*/
abstract Expr getDataExpr();
}
/**
* A function call that sends data over a network.
*
* note: functions such as `write` may be writing to a network source or a file. We could attempt to determine which, and sort results into `cpp/cleartext-transmission` and perhaps `cpp/cleartext-storage-file`. In practice it usually isn't very important which query reports a result as long as its reported exactly once.
*/
class NetworkSend extends NetworkSendRecv {
override Send target;
RemoteFlowSinkFunction target;
NetworkSend() { target = this.getTarget() }
override Expr getSocketExpr() {
exists(FunctionInput input, int arg |
target.hasSocketInput(input) and
input.isParameter(arg) and
result = this.getArgument(arg)
)
}
override Expr getDataExpr() {
exists(FunctionInput input, int arg |
target.hasRemoteFlowSink(input, _) and
input.isParameterDeref(arg) and
result = this.getArgument(arg)
)
}
}
/**
* A function call that receives data over a network.
*/
class NetworkRecv extends NetworkSendRecv {
override Recv target;
}
RemoteFlowSourceFunction target;
/**
* An expression that is an argument or return value from an encryption or
* decryption call.
*/
class Encrypted extends Expr {
Encrypted() {
exists(FunctionCall fc |
fc.getTarget().getName().toLowerCase().regexpMatch(".*(crypt|encode|decode).*") and
(
this = fc or
this = fc.getAnArgument()
)
NetworkRecv() { target = this.getTarget() }
override Expr getSocketExpr() {
exists(FunctionInput input, int arg |
target.hasSocketInput(input) and
input.isParameter(arg) and
result = this.getArgument(arg)
)
}
override Expr getDataExpr() {
exists(FunctionOutput output, int arg |
target.hasRemoteFlowSource(output, _) and
output.isParameterDeref(arg) and
result = this.getArgument(arg)
)
}
}
/**
* Taint flow from a sensitive expression.
* Taint flow from a sensitive expression to a network operation with data
* tainted by that expression.
*/
class FromSensitiveConfiguration extends TaintTracking::Configuration {
FromSensitiveConfiguration() { this = "FromSensitiveConfiguration" }
class SensitiveSendRecvConfiguration extends TaintTracking::Configuration {
SensitiveSendRecvConfiguration() { this = "SensitiveSendRecvConfiguration" }
override predicate isSource(DataFlow::Node source) { source instanceof SensitiveNode }
override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveExpr }
override predicate isSink(DataFlow::Node sink) {
sink.asExpr() = any(NetworkSendRecv nsr).getDataExpr()
or
sink.asExpr() instanceof Encrypted
}
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
// flow through encryption functions to the return value (in case we can reach other sinks)
node2.asExpr().(Encrypted).(FunctionCall).getAnArgument() = node1.asExpr()
exists(NetworkSendRecv transmission |
sink.asExpr() = transmission.getDataExpr() and
// a zero socket descriptor is standard input, which is not interesting for this query.
not exists(Zero zero |
DataFlow::localFlow(DataFlow::exprNode(zero),
DataFlow::exprNode(transmission.getSocketExpr()))
)
)
}
}
from
FromSensitiveConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink,
NetworkSendRecv networkSendRecv, string msg
SensitiveSendRecvConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink,
NetworkSendRecv transmission, string msg
where
// flow from sensitive -> network data
config.hasFlowPath(source, sink) and
sink.getNode().asExpr() = networkSendRecv.getDataExpr() and
// no flow from sensitive -> evidence of encryption
not exists(DataFlow::Node encrypted |
config.hasFlow(source.getNode(), encrypted) and
encrypted.asExpr() instanceof Encrypted
) and
// construct result
if networkSendRecv instanceof NetworkSend
sink.getNode().asExpr() = transmission.getDataExpr() and
if transmission instanceof NetworkSend
then
msg =
"This operation transmits '" + sink.toString() +
@@ -204,4 +121,4 @@ where
msg =
"This operation receives into '" + sink.toString() +
"', which may put unencrypted sensitive data into $@"
select networkSendRecv, source, sink, msg, source, source.getNode().toString()
select transmission, source, sink, msg, source, source.getNode().asExpr().toString()

View File

@@ -28,11 +28,6 @@ class PrivateHostName extends string {
}
}
pragma[nomagic]
predicate privateHostNameFlowsToExpr(Expr e) {
TaintTracking::localExprTaint(any(StringLiteral p | p.getValue() instanceof PrivateHostName), e)
}
/**
* A string containing an HTTP URL not in a private domain.
*/
@@ -43,9 +38,11 @@ class HttpStringLiteral extends StringLiteral {
or
exists(string tail |
tail = s.regexpCapture("http://(.*)", 1) and not tail instanceof PrivateHostName
)
) and
not privateHostNameFlowsToExpr(this.getParent*())
) and
not TaintTracking::localExprTaint(any(StringLiteral p |
p.getValue() instanceof PrivateHostName
), this.getParent*())
)
}
}

View File

@@ -1,6 +1 @@
## 0.0.5
### New Queries
* A new query `cpp/certificate-not-checked` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.
* A new query `cpp/certificate-result-conflation` has been added for C/C++. The query flags unsafe use of OpenSSL and similar libraries.

View File

@@ -1 +0,0 @@
## 0.0.6

View File

@@ -1 +0,0 @@
## 0.0.7

View File

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

View File

@@ -1,16 +0,0 @@
...
umask(0); // BAD
...
maskOut = S_IRWXG | S_IRWXO;
umask(maskOut); // GOOD
...
fchmod(fileno(fp), 0555 - maskOut); // BAD
...
fchmod(fileno(fp), 0555 & ~maskOut); // GOOD
...
umask(0666);
chmod(pathname, 0666); // BAD
...
umask(0022);
chmod(pathname, 0666); // GOOD
...

View File

@@ -1,23 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Finding for function calls that set file permissions that may have errors in use. Incorrect arithmetic for calculating the resolution mask, using the same mask in opposite functions, using a mask that is too wide.</p>
</overview>
<example>
<p>The following example demonstrates erroneous and fixed ways to use functions.</p>
<sample src="IncorrectPrivilegeAssignment.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>

View File

@@ -1,87 +0,0 @@
/**
* @name Find the wrong use of the umask function.
* @description Incorrectly evaluated argument to the umask function may have security implications.
* @kind problem
* @id cpp/wrong-use-of-the-umask
* @problem.severity warning
* @precision medium
* @tags correctness
* maintainability
* security
* external/cwe/cwe-266
* external/cwe/cwe-264
* external/cwe/cwe-200
* external/cwe/cwe-560
* external/cwe/cwe-687
*/
import cpp
import semmle.code.cpp.exprs.BitwiseOperation
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
/**
* An expression that is either a `BinaryArithmeticOperation` or the result of one or more `BinaryBitwiseOperation`s on a `BinaryArithmeticOperation`. For example `1 | (2 + 3)`.
*/
class ContainsArithmetic extends Expr {
ContainsArithmetic() {
this instanceof BinaryArithmeticOperation
or
// recursive search into `Operation`s
this.(BinaryBitwiseOperation).getAnOperand() instanceof ContainsArithmetic
}
}
/** Holds for a function `f` that has an argument at index `apos` used to set file permissions. */
predicate numberArgumentModFunctions(Function f, int apos) {
f.hasGlobalOrStdName("umask") and apos = 0
or
f.hasGlobalOrStdName("fchmod") and apos = 1
or
f.hasGlobalOrStdName("chmod") and apos = 1
}
from FunctionCall fc, string msg, FunctionCall fcsnd
where
fc.getTarget().hasGlobalOrStdName("umask") and
fc.getArgument(0).getValue() = "0" and
not exists(FunctionCall fctmp |
fctmp.getTarget().hasGlobalOrStdName("umask") and
not fctmp.getArgument(0).getValue() = "0"
) and
exists(FunctionCall fctmp |
(
fctmp.getTarget().hasGlobalOrStdName("fopen") or
fctmp.getTarget().hasGlobalOrStdName("open")
) and
not fctmp.getArgument(1).getValue().matches("r%") and
fctmp.getNumberOfArguments() = 2 and
not fctmp.getArgument(0).getValue() = "/dev/null" and
fcsnd = fctmp
) and
not exists(FunctionCall fctmp |
fctmp.getTarget().hasGlobalOrStdName("chmod") or
fctmp.getTarget().hasGlobalOrStdName("fchmod")
) and
msg = "Using umask(0) may not be safe with call $@."
or
fc.getTarget().hasGlobalOrStdName("umask") and
exists(FunctionCall fctmp |
(
fctmp.getTarget().hasGlobalOrStdName("chmod") or
fctmp.getTarget().hasGlobalOrStdName("fchmod")
) and
(
globalValueNumber(fc.getArgument(0)) = globalValueNumber(fctmp.getArgument(1)) and
fc.getArgument(0).getValue() != "0"
) and
msg = "Not use equal argument in umask and $@ functions." and
fcsnd = fctmp
)
or
exists(ContainsArithmetic exptmp, int i |
numberArgumentModFunctions(fc.getTarget(), i) and
globalValueNumber(exptmp) = globalValueNumber(fc.getArgument(i)) and
msg = "Using arithmetic to compute the mask in $@ may not be safe." and
fcsnd = fc
)
select fc, msg, fcsnd, fcsnd.getTarget().getName()

View File

@@ -1,5 +1,5 @@
name: codeql/cpp-queries
version: 0.0.8-dev
version: 0.0.5
groups: cpp
dependencies:
codeql/cpp-all: "*"

View File

@@ -1,2 +0,0 @@
| test.cpp:9:3:9:7 | call to umask | Not use equal argument in umask and $@ functions. | test.cpp:13:3:13:7 | call to chmod | chmod |
| test.cpp:30:3:30:7 | call to chmod | Using arithmetic to compute the mask in $@ may not be safe. | test.cpp:30:3:30:7 | call to chmod | chmod |

View File

@@ -1 +0,0 @@
experimental/Security/CWE/CWE-266/IncorrectPrivilegeAssignment.ql

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