mirror of
https://github.com/github/codeql.git
synced 2026-04-14 11:34:00 +02:00
Merge branch 'main' into redsun82/swift-open-redirection
This commit is contained in:
62
.github/workflows/ql-for-ql-tests.yml
vendored
62
.github/workflows/ql-for-ql-tests.yml
vendored
@@ -33,19 +33,73 @@ jobs:
|
||||
~/.cargo/registry
|
||||
~/.cargo/git
|
||||
ql/target
|
||||
key: ${{ runner.os }}-${{ steps.os_version.outputs.version }}-qltest-cargo-${{ hashFiles('ql/**/Cargo.lock') }}
|
||||
key: ${{ runner.os }}-${{ steps.os_version.outputs.version }}-qltest-cargo-${{ hashFiles('ql/rust-toolchain.toml', 'ql/**/Cargo.lock') }}
|
||||
- name: Build extractor
|
||||
run: |
|
||||
cd ql;
|
||||
codeqlpath=$(dirname ${{ steps.find-codeql.outputs.codeql-path }});
|
||||
env "PATH=$PATH:$codeqlpath" ./scripts/create-extractor-pack.sh
|
||||
- name: Cache compilation cache
|
||||
id: query-cache
|
||||
uses: ./.github/actions/cache-query-compilation
|
||||
with:
|
||||
key: ql-for-ql-tests
|
||||
- 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
|
||||
"${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 --compilation-cache "${{ steps.query-cache.outputs.cache-dir }}" ql/ql/test
|
||||
env:
|
||||
CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
|
||||
- name: Check QL formatting
|
||||
|
||||
other-os:
|
||||
strategy:
|
||||
matrix:
|
||||
os: [macos-latest, windows-latest]
|
||||
needs: [qltest]
|
||||
runs-on: ${{ matrix.os }}
|
||||
steps:
|
||||
- uses: actions/checkout@v3
|
||||
- name: Install GNU tar
|
||||
if: runner.os == 'macOS'
|
||||
run: |
|
||||
find ql/ql/src "(" -name "*.ql" -or -name "*.qll" ")" -print0 | xargs -0 "${CODEQL}" query format --check-only
|
||||
brew install gnu-tar
|
||||
echo "/usr/local/opt/gnu-tar/libexec/gnubin" >> $GITHUB_PATH
|
||||
- name: Find codeql
|
||||
id: find-codeql
|
||||
uses: github/codeql-action/init@77a8d2d10c0b403a8b4aadbd223dc489ecd22683
|
||||
with:
|
||||
languages: javascript # does not matter
|
||||
- uses: ./.github/actions/os-version
|
||||
id: os_version
|
||||
- uses: actions/cache@v3
|
||||
with:
|
||||
path: |
|
||||
~/.cargo/registry
|
||||
~/.cargo/git
|
||||
ql/target
|
||||
key: ${{ runner.os }}-${{ steps.os_version.outputs.version }}-qltest-cargo-${{ hashFiles('ql/rust-toolchain.toml', 'ql/**/Cargo.lock') }}
|
||||
- name: Build extractor
|
||||
if: runner.os != 'Windows'
|
||||
run: |
|
||||
cd ql;
|
||||
codeqlpath=$(dirname ${{ steps.find-codeql.outputs.codeql-path }});
|
||||
env "PATH=$PATH:$codeqlpath" ./scripts/create-extractor-pack.sh
|
||||
- name: Build extractor (Windows)
|
||||
if: runner.os == 'Windows'
|
||||
shell: pwsh
|
||||
run: |
|
||||
cd ql;
|
||||
$Env:PATH += ";$(dirname ${{ steps.find-codeql.outputs.codeql-path }})"
|
||||
pwsh ./scripts/create-extractor-pack.ps1
|
||||
- name: Run a single QL tests - Unix
|
||||
if: runner.os != 'Windows'
|
||||
run: |
|
||||
"${CODEQL}" test run --check-databases --search-path "${{ github.workspace }}/ql/extractor-pack" ql/ql/test/queries/style/DeadCode/DeadCode.qlref
|
||||
env:
|
||||
CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}
|
||||
- name: Run a single QL tests - Windows
|
||||
if: runner.os == 'Windows'
|
||||
shell: pwsh
|
||||
run: |
|
||||
$Env:PATH += ";$(dirname ${{ steps.find-codeql.outputs.codeql-path }})"
|
||||
codeql test run --check-databases --search-path "${{ github.workspace }}/ql/extractor-pack" ql/ql/test/queries/style/DeadCode/DeadCode.qlref
|
||||
|
||||
22
.github/workflows/ruby-build.yml
vendored
22
.github/workflows/ruby-build.yml
vendored
@@ -205,11 +205,6 @@ jobs:
|
||||
- name: Fetch CodeQL
|
||||
uses: ./.github/actions/fetch-codeql
|
||||
|
||||
- uses: actions/checkout@v3
|
||||
with:
|
||||
repository: Shopify/example-ruby-app
|
||||
ref: 67a0decc5eb550f3a9228eda53925c3afd40dfe9
|
||||
|
||||
- name: Download Ruby bundle
|
||||
uses: actions/download-artifact@v3
|
||||
with:
|
||||
@@ -218,26 +213,15 @@ jobs:
|
||||
- name: Unzip Ruby bundle
|
||||
shell: bash
|
||||
run: unzip -q -d "${{ runner.temp }}/ruby-bundle" "${{ runner.temp }}/codeql-ruby-bundle.zip"
|
||||
- name: Prepare test files
|
||||
shell: bash
|
||||
run: |
|
||||
echo "import codeql.ruby.AST select count(File f)" > "test.ql"
|
||||
echo "| 4 |" > "test.expected"
|
||||
echo 'name: sample-tests
|
||||
version: 0.0.0
|
||||
dependencies:
|
||||
codeql/ruby-all: "*"
|
||||
extractor: ruby
|
||||
tests: .
|
||||
' > qlpack.yml
|
||||
|
||||
- name: Run QL test
|
||||
shell: bash
|
||||
run: |
|
||||
codeql test run --search-path "${{ runner.temp }}/ruby-bundle" --additional-packs "${{ runner.temp }}/ruby-bundle" .
|
||||
codeql test run --search-path "${{ runner.temp }}/ruby-bundle" --additional-packs "${{ runner.temp }}/ruby-bundle" ruby/ql/test/library-tests/ast/constants/
|
||||
- name: Create database
|
||||
shell: bash
|
||||
run: |
|
||||
codeql database create --search-path "${{ runner.temp }}/ruby-bundle" --language ruby --source-root . ../database
|
||||
codeql database create --search-path "${{ runner.temp }}/ruby-bundle" --language ruby --source-root ruby/ql/test/library-tests/ast/constants/ ../database
|
||||
- name: Analyze database
|
||||
shell: bash
|
||||
run: |
|
||||
|
||||
@@ -7,6 +7,6 @@ byte[] encrypted = cipher.doFinal(input.getBytes("UTF-8"));
|
||||
// ...
|
||||
|
||||
// GOOD: AES is a strong algorithm
|
||||
Cipher des = Cipher.getInstance("AES");
|
||||
Cipher aes = Cipher.getInstance("AES");
|
||||
|
||||
// ...
|
||||
// ...
|
||||
|
||||
@@ -166,6 +166,11 @@ module UnsafeShellCommandConstruction {
|
||||
.asExpr()
|
||||
.(BooleanLiteral)
|
||||
.getValue() = "true"
|
||||
or
|
||||
exists(API::Node node |
|
||||
node.asSink() = sys.getOptionsArg() and
|
||||
node.getMember("shell").asSink().mayHaveBooleanValue(true)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -282,6 +282,14 @@ nodes
|
||||
| lib/lib.js:543:23:543:26 | name |
|
||||
| lib/lib.js:545:23:545:26 | name |
|
||||
| lib/lib.js:545:23:545:26 | name |
|
||||
| lib/lib.js:550:39:550:42 | name |
|
||||
| lib/lib.js:550:39:550:42 | name |
|
||||
| lib/lib.js:551:33:551:36 | args |
|
||||
| lib/lib.js:552:23:552:26 | args |
|
||||
| lib/lib.js:552:23:552:26 | args |
|
||||
| lib/lib.js:555:25:555:37 | ["-rf", name] |
|
||||
| lib/lib.js:555:33:555:36 | name |
|
||||
| lib/lib.js:555:33:555:36 | name |
|
||||
| lib/subLib2/compiled-file.ts:3:26:3:29 | name |
|
||||
| lib/subLib2/compiled-file.ts:3:26:3:29 | name |
|
||||
| lib/subLib2/compiled-file.ts:4:25:4:28 | name |
|
||||
@@ -659,6 +667,14 @@ edges
|
||||
| lib/lib.js:509:39:509:42 | name | lib/lib.js:545:23:545:26 | name |
|
||||
| lib/lib.js:509:39:509:42 | name | lib/lib.js:545:23:545:26 | name |
|
||||
| lib/lib.js:509:39:509:42 | name | lib/lib.js:545:23:545:26 | name |
|
||||
| lib/lib.js:550:39:550:42 | name | lib/lib.js:555:33:555:36 | name |
|
||||
| lib/lib.js:550:39:550:42 | name | lib/lib.js:555:33:555:36 | name |
|
||||
| lib/lib.js:550:39:550:42 | name | lib/lib.js:555:33:555:36 | name |
|
||||
| lib/lib.js:550:39:550:42 | name | lib/lib.js:555:33:555:36 | name |
|
||||
| lib/lib.js:551:33:551:36 | args | lib/lib.js:552:23:552:26 | args |
|
||||
| lib/lib.js:551:33:551:36 | args | lib/lib.js:552:23:552:26 | args |
|
||||
| lib/lib.js:555:25:555:37 | ["-rf", name] | lib/lib.js:551:33:551:36 | args |
|
||||
| lib/lib.js:555:33:555:36 | name | lib/lib.js:555:25:555:37 | ["-rf", name] |
|
||||
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
|
||||
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
|
||||
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
|
||||
@@ -775,6 +791,8 @@ edges
|
||||
| lib/lib.js:537:11:537:26 | "rm -rf " + name | lib/lib.js:509:39:509:42 | name | lib/lib.js:537:23:537:26 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:509:39:509:42 | name | library input | lib/lib.js:537:3:537:27 | cp.exec ... + name) | shell command |
|
||||
| lib/lib.js:543:11:543:26 | "rm -rf " + name | lib/lib.js:509:39:509:42 | name | lib/lib.js:543:23:543:26 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:509:39:509:42 | name | library input | lib/lib.js:543:3:543:27 | cp.exec ... + name) | shell command |
|
||||
| lib/lib.js:545:11:545:26 | "rm -rf " + name | lib/lib.js:509:39:509:42 | name | lib/lib.js:545:23:545:26 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:509:39:509:42 | name | library input | lib/lib.js:545:3:545:27 | cp.exec ... + name) | shell command |
|
||||
| lib/lib.js:552:23:552:26 | args | lib/lib.js:550:39:550:42 | name | lib/lib.js:552:23:552:26 | args | This shell argument which depends on $@ is later used in a $@. | lib/lib.js:550:39:550:42 | name | library input | lib/lib.js:552:9:552:38 | cp.spaw ... wnOpts) | shell command |
|
||||
| lib/lib.js:555:33:555:36 | name | lib/lib.js:550:39:550:42 | name | lib/lib.js:555:33:555:36 | name | This shell argument which depends on $@ is later used in a $@. | lib/lib.js:550:39:550:42 | name | library input | lib/lib.js:552:9:552:38 | cp.spaw ... wnOpts) | shell command |
|
||||
| lib/subLib2/compiled-file.ts:4:13:4:28 | "rm -rf " + name | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib2/compiled-file.ts:3:26:3:29 | name | library input | lib/subLib2/compiled-file.ts:4:5:4:29 | cp.exec ... + name) | shell command |
|
||||
| lib/subLib2/special-file.js:4:10:4:25 | "rm -rf " + name | lib/subLib2/special-file.js:3:28:3:31 | name | lib/subLib2/special-file.js:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib2/special-file.js:3:28:3:31 | name | library input | lib/subLib2/special-file.js:4:2:4:26 | cp.exec ... + name) | shell command |
|
||||
| lib/subLib3/my-file.ts:4:10:4:25 | "rm -rf " + name | lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib3/my-file.ts:3:28:3:31 | name | library input | lib/subLib3/my-file.ts:4:2:4:26 | cp.exec ... + name) | shell command |
|
||||
|
||||
@@ -545,3 +545,12 @@ module.exports.sanitizer4 = function (name) {
|
||||
cp.exec("rm -rf " + name); // NOT OK
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
module.exports.shellThing = function (name) {
|
||||
function indirectShell(cmd, args, spawnOpts) {
|
||||
cp.spawn(cmd, args, spawnOpts); // NOT OK
|
||||
}
|
||||
|
||||
indirectShell("rm", ["-rf", name], {shell: true});
|
||||
}
|
||||
7
ql/rust-toolchain.toml
Normal file
7
ql/rust-toolchain.toml
Normal file
@@ -0,0 +1,7 @@
|
||||
# This file specifies the Rust version used to develop and test the QL
|
||||
# extractor. It is set to the lowest version of Rust we want to support.
|
||||
|
||||
[toolchain]
|
||||
channel = "1.54"
|
||||
profile = "minimal"
|
||||
components = [ "rustfmt" ]
|
||||
@@ -8,6 +8,7 @@ type NUL && "%CODEQL_DIST%\codeql.exe" database index-files ^
|
||||
--include-extension=.yml ^
|
||||
--size-limit=5m ^
|
||||
--language=ql ^
|
||||
--working-dir=. ^
|
||||
"%CODEQL_EXTRACTOR_QL_WIP_DATABASE%"
|
||||
|
||||
exit /b %ERRORLEVEL%
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
/**
|
||||
* Provides default sources, sinks and sanitizers for reasoning about
|
||||
* code constructed from library input vulnerabilities, as
|
||||
* well as extension points for adding your own.
|
||||
*/
|
||||
|
||||
private import ruby
|
||||
private import codeql.ruby.ApiGraphs
|
||||
private import codeql.ruby.frameworks.core.Gem::Gem as Gem
|
||||
private import codeql.ruby.Concepts as Concepts
|
||||
|
||||
/**
|
||||
* Module containing sources, sinks, and sanitizers for code constructed from library input.
|
||||
*/
|
||||
module UnsafeCodeConstruction {
|
||||
/** A source for code constructed from library input vulnerabilities. */
|
||||
abstract class Source extends DataFlow::Node { }
|
||||
|
||||
/** An input parameter to a gem seen as a source. */
|
||||
private class LibraryInputAsSource extends Source instanceof DataFlow::ParameterNode {
|
||||
LibraryInputAsSource() {
|
||||
this = Gem::getALibraryInput() and
|
||||
not this.getName() = "code"
|
||||
}
|
||||
}
|
||||
|
||||
/** A sink for code constructed from library input vulnerabilities. */
|
||||
abstract class Sink extends DataFlow::Node {
|
||||
/**
|
||||
* Gets the node where the unsafe code is executed.
|
||||
*/
|
||||
abstract DataFlow::Node getCodeSink();
|
||||
|
||||
/**
|
||||
* Gets the type of sink.
|
||||
*/
|
||||
string getSinkType() { result = "code construction" }
|
||||
}
|
||||
|
||||
/** Gets a node that is eventually executed as code at `codeExec`. */
|
||||
DataFlow::Node getANodeExecutedAsCode(Concepts::CodeExecution codeExec) {
|
||||
result = getANodeExecutedAsCode(TypeTracker::TypeBackTracker::end(), codeExec)
|
||||
}
|
||||
|
||||
import codeql.ruby.typetracking.TypeTracker as TypeTracker
|
||||
|
||||
/** Gets a node that is eventually executed as code at `codeExec`, type-tracked with `t`. */
|
||||
private DataFlow::LocalSourceNode getANodeExecutedAsCode(
|
||||
TypeTracker::TypeBackTracker t, Concepts::CodeExecution codeExec
|
||||
) {
|
||||
t.start() and
|
||||
result = codeExec.getCode().getALocalSource() and
|
||||
codeExec.runsArbitraryCode() // methods like `Object.send` is benign here, because of the string-construction the attacker cannot control the entire method name
|
||||
or
|
||||
exists(TypeTracker::TypeBackTracker t2 |
|
||||
result = getANodeExecutedAsCode(t2, codeExec).backtrack(t2, t)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* A string constructed using a `.join(...)` call, where the resulting string ends up being executed as code.
|
||||
*/
|
||||
class ArrayJoin extends Sink {
|
||||
Concepts::CodeExecution s;
|
||||
|
||||
ArrayJoin() {
|
||||
exists(DataFlow::CallNode call |
|
||||
call.getMethodName() = "join" and
|
||||
call.getNumberOfArguments() = 1 and // any string. E.g. ";" or "\n".
|
||||
call = getANodeExecutedAsCode(s) and
|
||||
this = call.getReceiver()
|
||||
)
|
||||
}
|
||||
|
||||
override DataFlow::Node getCodeSink() { result = s }
|
||||
|
||||
override string getSinkType() { result = "array" }
|
||||
}
|
||||
|
||||
/**
|
||||
* A string constructed from a string-literal (e.g. `"foo #{sink}"`),
|
||||
* where the resulting string ends up being executed as a code.
|
||||
*/
|
||||
class StringInterpolationAsSink extends Sink {
|
||||
Concepts::CodeExecution s;
|
||||
|
||||
StringInterpolationAsSink() {
|
||||
exists(Ast::StringlikeLiteral lit |
|
||||
any(DataFlow::Node n | n.asExpr().getExpr() = lit) = getANodeExecutedAsCode(s) and
|
||||
this.asExpr().getExpr() = lit.getComponent(_)
|
||||
)
|
||||
}
|
||||
|
||||
override DataFlow::Node getCodeSink() { result = s }
|
||||
|
||||
override string getSinkType() { result = "string interpolation" }
|
||||
}
|
||||
|
||||
import codeql.ruby.security.TaintedFormatStringSpecific as TaintedFormat
|
||||
|
||||
/**
|
||||
* A string constructed from a printf-style call,
|
||||
* where the resulting string ends up being executed as a code.
|
||||
*/
|
||||
class TaintedFormatStringAsSink extends Sink {
|
||||
Concepts::CodeExecution s;
|
||||
|
||||
TaintedFormatStringAsSink() {
|
||||
exists(TaintedFormat::PrintfStyleCall call |
|
||||
call = getANodeExecutedAsCode(s) and
|
||||
this = [call.getFormatArgument(_), call.getFormatString()]
|
||||
)
|
||||
}
|
||||
|
||||
override DataFlow::Node getCodeSink() { result = s }
|
||||
|
||||
override string getSinkType() { result = "string format" }
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,49 @@
|
||||
/**
|
||||
* Provides a taint-tracking configuration for reasoning about code
|
||||
* constructed from library input vulnerabilities.
|
||||
*
|
||||
* Note, for performance reasons: only import this file if `Configuration` is needed,
|
||||
* otherwise `UnsafeCodeConstructionCustomizations` should be imported instead.
|
||||
*/
|
||||
|
||||
import codeql.ruby.DataFlow
|
||||
import UnsafeCodeConstructionCustomizations::UnsafeCodeConstruction
|
||||
private import codeql.ruby.TaintTracking
|
||||
private import codeql.ruby.dataflow.BarrierGuards
|
||||
|
||||
/**
|
||||
* A taint-tracking configuration for detecting code constructed from library input vulnerabilities.
|
||||
*/
|
||||
class Configuration extends TaintTracking::Configuration {
|
||||
Configuration() { this = "UnsafeShellCommandConstruction" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) { source instanceof Source }
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
|
||||
|
||||
override predicate isSanitizer(DataFlow::Node node) {
|
||||
node instanceof StringConstCompareBarrier or
|
||||
node instanceof StringConstArrayInclusionCallBarrier
|
||||
}
|
||||
|
||||
// override to require the path doesn't have unmatched return steps
|
||||
override DataFlow::FlowFeature getAFeature() {
|
||||
result instanceof DataFlow::FeatureHasSourceCallContext
|
||||
}
|
||||
|
||||
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
|
||||
// if an array element gets tainted, then we treat the entire array as tainted
|
||||
exists(DataFlow::CallNode call |
|
||||
call.getMethodName() = ["<<", "push", "append"] and
|
||||
call.getReceiver() = succ and
|
||||
pred = call.getArgument(0) and
|
||||
call.getNumberOfArguments() = 1
|
||||
)
|
||||
or
|
||||
exists(DataFlow::CallNode call |
|
||||
call.getMethodName() = "[]" and
|
||||
succ = call and
|
||||
pred = call.getArgument(_)
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -67,7 +67,7 @@ module UnsafeShellCommandConstruction {
|
||||
*/
|
||||
class StringInterpolationAsSink extends Sink {
|
||||
Concepts::SystemCommandExecution s;
|
||||
Ast::StringLiteral lit;
|
||||
Ast::StringlikeLiteral lit;
|
||||
|
||||
StringInterpolationAsSink() {
|
||||
isUsedAsShellCommand(any(DataFlow::Node n | n.asExpr().getExpr() = lit), s) and
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: newQuery
|
||||
---
|
||||
* Added a new query, `rb/unsafe-code-construction`, to detect libraries that unsafely construct code from their inputs.
|
||||
@@ -0,0 +1,111 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
When a library function dynamically constructs code in a potentially unsafe way,
|
||||
it's important to document to clients of the library that the function should only be
|
||||
used with trusted inputs.
|
||||
|
||||
If the function is not documented as being potentially unsafe, then a client may
|
||||
incorrectly use inputs containing unsafe code fragments, and thereby leave the
|
||||
client vulnerable to code-injection attacks.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
Properly document library functions that construct code from unsanitized
|
||||
inputs, or avoid constructing code in the first place.
|
||||
</p>
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>
|
||||
The following example shows two methods implemented using <code>eval</code>: a simple
|
||||
deserialization routine and a getter method.
|
||||
If untrusted inputs are used with these methods,
|
||||
then an attacker might be able to execute arbitrary code on the system.
|
||||
</p>
|
||||
|
||||
<sample src="examples/UnsafeCodeConstruction.rb" />
|
||||
|
||||
<p>
|
||||
To avoid this problem, either properly document that the function is potentially
|
||||
unsafe, or use an alternative solution such as <code>JSON.parse</code> or another library
|
||||
that does not allow arbitrary code to be executed.
|
||||
</p>
|
||||
|
||||
<sample src="examples/UnsafeCodeConstructionSafe.rb" />
|
||||
|
||||
</example>
|
||||
|
||||
<example>
|
||||
<p>
|
||||
As another example, consider the below code which dynamically constructs
|
||||
a class that has a getter method with a custom name.
|
||||
</p>
|
||||
|
||||
<sample src="examples/UnsafeCodeConstruction2.rb" />
|
||||
|
||||
<p>
|
||||
The example dynamically constructs a string which is then executed using <code>module_eval</code>.
|
||||
This code will break if the specified name is not a valid Ruby identifier, and
|
||||
if the value is controlled by an attacker, then this could lead to code-injection.
|
||||
</p>
|
||||
|
||||
<p>
|
||||
A more robust implementation, that is also immune to code-injection,
|
||||
can be made by using <code>module_eval</code> with a block and using <code>define_method</code>
|
||||
to define the getter method.
|
||||
</p>
|
||||
|
||||
<sample src="examples/UnsafeCodeConstruction2Safe.rb" />
|
||||
</example>
|
||||
|
||||
<example>
|
||||
<p>
|
||||
This example dynamically registers a method on another class which
|
||||
forwards its arguments to a target object. This approach uses
|
||||
<code>module_eval</code> and string interpolation to construct class variables
|
||||
and methods.
|
||||
</p>
|
||||
|
||||
<sample src="examples/UnsafeCodeConstruction3.rb" />
|
||||
|
||||
<p>
|
||||
A safer approach is to use <code>class_variable_set</code> and
|
||||
<code>class_variable_get</code> along with <code>define_method</code>. String
|
||||
interpolation is still used to construct the class variable name, but this is
|
||||
safe because <code>class_variable_set</code> is not susceptible to code-injection.
|
||||
</p>
|
||||
|
||||
<p>
|
||||
<code>send</code> is used to dynamically call the method specified by <code>name</code>.
|
||||
This is a more robust alternative than the previous example, because it does not allow
|
||||
arbitrary code to be executed, but it does still allow for any method to be called
|
||||
on the target object.
|
||||
</p>
|
||||
|
||||
<sample src="examples/UnsafeCodeConstruction3Safe.rb" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>
|
||||
OWASP:
|
||||
<a href="https://www.owasp.org/index.php/Code_Injection">Code Injection</a>.
|
||||
</li>
|
||||
<li>
|
||||
Wikipedia: <a href="https://en.wikipedia.org/wiki/Code_injection">Code Injection</a>.
|
||||
</li>
|
||||
<li>
|
||||
Ruby documentation: <a href="https://docs.ruby-lang.org/en/3.2/Module.html#method-i-define_method">define_method</a>.
|
||||
</li>
|
||||
<li>
|
||||
Ruby documentation: <a href="https://docs.ruby-lang.org/en/3.2/Module.html#method-i-class_variable_set">class_variable_set</a>.
|
||||
</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -0,0 +1,23 @@
|
||||
/**
|
||||
* @name Unsafe code constructed from library input
|
||||
* @description Using externally controlled strings to construct code may allow a malicious
|
||||
* user to execute arbitrary code.
|
||||
* @kind path-problem
|
||||
* @problem.severity warning
|
||||
* @security-severity 6.1
|
||||
* @precision medium
|
||||
* @id rb/unsafe-code-construction
|
||||
* @tags security
|
||||
* external/cwe/cwe-094
|
||||
* external/cwe/cwe-079
|
||||
* external/cwe/cwe-116
|
||||
*/
|
||||
|
||||
import codeql.ruby.security.UnsafeCodeConstructionQuery
|
||||
import DataFlow::PathGraph
|
||||
|
||||
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, Sink sinkNode
|
||||
where cfg.hasFlowPath(source, sink) and sinkNode = sink.getNode()
|
||||
select sink.getNode(), source, sink,
|
||||
"This " + sinkNode.getSinkType() + " which depends on $@ is later $@.", source.getNode(),
|
||||
"library input", sinkNode.getCodeSink(), "interpreted as code"
|
||||
@@ -0,0 +1,10 @@
|
||||
module MyLib
|
||||
def unsafeDeserialize(value)
|
||||
eval("foo = #{value}")
|
||||
foo
|
||||
end
|
||||
|
||||
def unsafeGetter(obj, path)
|
||||
eval("obj.#{path}")
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,17 @@
|
||||
require 'json'
|
||||
|
||||
module BadMakeGetter
|
||||
# Makes a class with a method named `getter_name` that returns `val`
|
||||
def self.define_getter_class(getter_name, val)
|
||||
new_class = Class.new
|
||||
new_class.module_eval <<-END
|
||||
def #{getter_name}
|
||||
#{JSON.dump(val)}
|
||||
end
|
||||
END
|
||||
new_class
|
||||
end
|
||||
end
|
||||
|
||||
one = BadMakeGetter.define_getter_class(:one, "foo")
|
||||
puts "One is #{one.new.one}"
|
||||
@@ -0,0 +1,13 @@
|
||||
# Uses `define_method` instead of constructing a string
|
||||
module GoodMakeGetter
|
||||
def self.define_getter_class(getter_name, val)
|
||||
new_class = Class.new
|
||||
new_class.module_eval do
|
||||
define_method(getter_name) { val }
|
||||
end
|
||||
new_class
|
||||
end
|
||||
end
|
||||
|
||||
two = GoodMakeGetter.define_getter_class(:two, "bar")
|
||||
puts "Two is #{two.new.two}"
|
||||
@@ -0,0 +1,11 @@
|
||||
module Invoker
|
||||
def attach(klass, name, target)
|
||||
klass.module_eval <<-CODE
|
||||
@@#{name} = target
|
||||
|
||||
def #{name}(*args)
|
||||
@@#{name}.#{name}(*args)
|
||||
end
|
||||
CODE
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,9 @@
|
||||
module Invoker
|
||||
def attach(klass, name, target)
|
||||
var = :"@@#{name}"
|
||||
klass.class_variable_set(var, target)
|
||||
klass.define_method(name) do |*args|
|
||||
self.class.class_variable_get(var).send(name, *args)
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,11 @@
|
||||
require 'json'
|
||||
|
||||
module MyLib
|
||||
def safeDeserialize(value)
|
||||
JSON.parse(value)
|
||||
end
|
||||
|
||||
def safeGetter(obj, path)
|
||||
obj.dig(*path.split("."))
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,35 @@
|
||||
edges
|
||||
| impl/unsafeCode.rb:2:12:2:17 | target : | impl/unsafeCode.rb:3:17:3:25 | #{...} |
|
||||
| impl/unsafeCode.rb:7:12:7:12 | x : | impl/unsafeCode.rb:8:30:8:30 | x |
|
||||
| impl/unsafeCode.rb:12:12:12:12 | x : | impl/unsafeCode.rb:13:33:13:33 | x |
|
||||
| impl/unsafeCode.rb:28:17:28:22 | my_arr : | impl/unsafeCode.rb:29:10:29:15 | my_arr |
|
||||
| impl/unsafeCode.rb:32:21:32:21 | x : | impl/unsafeCode.rb:34:10:34:12 | arr |
|
||||
| impl/unsafeCode.rb:37:15:37:15 | x : | impl/unsafeCode.rb:40:10:40:12 | arr |
|
||||
| impl/unsafeCode.rb:37:15:37:15 | x : | impl/unsafeCode.rb:44:10:44:12 | arr |
|
||||
| impl/unsafeCode.rb:47:15:47:15 | x : | impl/unsafeCode.rb:49:9:49:12 | #{...} |
|
||||
nodes
|
||||
| impl/unsafeCode.rb:2:12:2:17 | target : | semmle.label | target : |
|
||||
| impl/unsafeCode.rb:3:17:3:25 | #{...} | semmle.label | #{...} |
|
||||
| impl/unsafeCode.rb:7:12:7:12 | x : | semmle.label | x : |
|
||||
| impl/unsafeCode.rb:8:30:8:30 | x | semmle.label | x |
|
||||
| impl/unsafeCode.rb:12:12:12:12 | x : | semmle.label | x : |
|
||||
| impl/unsafeCode.rb:13:33:13:33 | x | semmle.label | x |
|
||||
| impl/unsafeCode.rb:28:17:28:22 | my_arr : | semmle.label | my_arr : |
|
||||
| impl/unsafeCode.rb:29:10:29:15 | my_arr | semmle.label | my_arr |
|
||||
| impl/unsafeCode.rb:32:21:32:21 | x : | semmle.label | x : |
|
||||
| impl/unsafeCode.rb:34:10:34:12 | arr | semmle.label | arr |
|
||||
| impl/unsafeCode.rb:37:15:37:15 | x : | semmle.label | x : |
|
||||
| impl/unsafeCode.rb:40:10:40:12 | arr | semmle.label | arr |
|
||||
| impl/unsafeCode.rb:44:10:44:12 | arr | semmle.label | arr |
|
||||
| impl/unsafeCode.rb:47:15:47:15 | x : | semmle.label | x : |
|
||||
| impl/unsafeCode.rb:49:9:49:12 | #{...} | semmle.label | #{...} |
|
||||
subpaths
|
||||
#select
|
||||
| impl/unsafeCode.rb:3:17:3:25 | #{...} | impl/unsafeCode.rb:2:12:2:17 | target : | impl/unsafeCode.rb:3:17:3:25 | #{...} | This string interpolation which depends on $@ is later $@. | impl/unsafeCode.rb:2:12:2:17 | target | library input | impl/unsafeCode.rb:3:5:3:27 | call to eval | interpreted as code |
|
||||
| impl/unsafeCode.rb:8:30:8:30 | x | impl/unsafeCode.rb:7:12:7:12 | x : | impl/unsafeCode.rb:8:30:8:30 | x | This string format which depends on $@ is later $@. | impl/unsafeCode.rb:7:12:7:12 | x | library input | impl/unsafeCode.rb:8:5:8:32 | call to eval | interpreted as code |
|
||||
| impl/unsafeCode.rb:13:33:13:33 | x | impl/unsafeCode.rb:12:12:12:12 | x : | impl/unsafeCode.rb:13:33:13:33 | x | This string format which depends on $@ is later $@. | impl/unsafeCode.rb:12:12:12:12 | x | library input | impl/unsafeCode.rb:13:5:13:35 | call to eval | interpreted as code |
|
||||
| impl/unsafeCode.rb:29:10:29:15 | my_arr | impl/unsafeCode.rb:28:17:28:22 | my_arr : | impl/unsafeCode.rb:29:10:29:15 | my_arr | This array which depends on $@ is later $@. | impl/unsafeCode.rb:28:17:28:22 | my_arr | library input | impl/unsafeCode.rb:29:5:29:27 | call to eval | interpreted as code |
|
||||
| impl/unsafeCode.rb:34:10:34:12 | arr | impl/unsafeCode.rb:32:21:32:21 | x : | impl/unsafeCode.rb:34:10:34:12 | arr | This array which depends on $@ is later $@. | impl/unsafeCode.rb:32:21:32:21 | x | library input | impl/unsafeCode.rb:34:5:34:24 | call to eval | interpreted as code |
|
||||
| impl/unsafeCode.rb:40:10:40:12 | arr | impl/unsafeCode.rb:37:15:37:15 | x : | impl/unsafeCode.rb:40:10:40:12 | arr | This array which depends on $@ is later $@. | impl/unsafeCode.rb:37:15:37:15 | x | library input | impl/unsafeCode.rb:40:5:40:24 | call to eval | interpreted as code |
|
||||
| impl/unsafeCode.rb:44:10:44:12 | arr | impl/unsafeCode.rb:37:15:37:15 | x : | impl/unsafeCode.rb:44:10:44:12 | arr | This array which depends on $@ is later $@. | impl/unsafeCode.rb:37:15:37:15 | x | library input | impl/unsafeCode.rb:44:5:44:24 | call to eval | interpreted as code |
|
||||
| impl/unsafeCode.rb:49:9:49:12 | #{...} | impl/unsafeCode.rb:47:15:47:15 | x : | impl/unsafeCode.rb:49:9:49:12 | #{...} | This string interpolation which depends on $@ is later $@. | impl/unsafeCode.rb:47:15:47:15 | x | library input | impl/unsafeCode.rb:51:5:51:13 | call to eval | interpreted as code |
|
||||
@@ -0,0 +1 @@
|
||||
queries/security/cwe-094/UnsafeCodeConstruction.ql
|
||||
@@ -0,0 +1,53 @@
|
||||
class Foobar
|
||||
def foo1(target)
|
||||
eval("foo = #{target}") # NOT OK
|
||||
end
|
||||
|
||||
# sprintf
|
||||
def foo2(x)
|
||||
eval(sprintf("foo = %s", x)) # NOT OK
|
||||
end
|
||||
|
||||
# String#%
|
||||
def foo3(x)
|
||||
eval("foo = %{foo}" % {foo: x}) # NOT OK
|
||||
end
|
||||
|
||||
def indirect_eval(x)
|
||||
eval(x) # OK - no construction.
|
||||
end
|
||||
|
||||
def send_stuff(x)
|
||||
foo.send("foo_#{x}") # OK - attacker cannot control entire string.
|
||||
end
|
||||
|
||||
def named_code(code)
|
||||
eval("def \n #{code} \n end") # OK - parameter is named code
|
||||
end
|
||||
|
||||
def joinStuff(my_arr)
|
||||
eval(my_arr.join("\n")) # NOT OK
|
||||
end
|
||||
|
||||
def joinWithElemt(x)
|
||||
arr = [x, "foobar"]
|
||||
eval(arr.join("\n")) # NOT OK
|
||||
end
|
||||
|
||||
def pushArr(x, y)
|
||||
arr = []
|
||||
arr.push(x)
|
||||
eval(arr.join("\n")) # NOT OK
|
||||
|
||||
arr2 = []
|
||||
arr2 << y
|
||||
eval(arr.join("\n")) # NOT OK
|
||||
end
|
||||
|
||||
def hereDoc(x)
|
||||
foo = <<~HERE
|
||||
#{x}
|
||||
HERE
|
||||
eval(foo) # NOT OK
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,5 @@
|
||||
Gem::Specification.new do |s|
|
||||
s.name = 'unsafe-code'
|
||||
s.require_path = "impl"
|
||||
end
|
||||
|
||||
@@ -8,6 +8,7 @@ type NUL && "%CODEQL_DIST%\codeql.exe" database index-files ^
|
||||
--include=**/Gemfile ^
|
||||
--size-limit=5m ^
|
||||
--language=ruby ^
|
||||
--working-dir=. ^
|
||||
"%CODEQL_EXTRACTOR_RUBY_WIP_DATABASE%"
|
||||
|
||||
exit /b %ERRORLEVEL%
|
||||
|
||||
@@ -89,6 +89,7 @@ private module Frameworks {
|
||||
private import codeql.swift.frameworks.StandardLibrary.UrlSession
|
||||
private import codeql.swift.frameworks.StandardLibrary.WebView
|
||||
private import codeql.swift.frameworks.Alamofire.Alamofire
|
||||
private import codeql.swift.security.CleartextLogging
|
||||
private import codeql.swift.security.PathInjection
|
||||
private import codeql.swift.security.PredicateInjection
|
||||
}
|
||||
|
||||
103
swift/ql/lib/codeql/swift/security/CleartextLogging.qll
Normal file
103
swift/ql/lib/codeql/swift/security/CleartextLogging.qll
Normal file
@@ -0,0 +1,103 @@
|
||||
/** Provides classes and predicates to reason about cleartext logging of sensitive data vulnerabilities. */
|
||||
|
||||
import swift
|
||||
private import codeql.swift.dataflow.DataFlow
|
||||
private import codeql.swift.dataflow.ExternalFlow
|
||||
private import codeql.swift.security.SensitiveExprs
|
||||
|
||||
/** A data flow sink for cleartext logging of sensitive data vulnerabilities. */
|
||||
abstract class CleartextLoggingSink extends DataFlow::Node { }
|
||||
|
||||
/** A sanitizer for cleartext logging of sensitive data vulnerabilities. */
|
||||
abstract class CleartextLoggingSanitizer extends DataFlow::Node { }
|
||||
|
||||
/**
|
||||
* A unit class for adding additional taint steps.
|
||||
*
|
||||
* Extend this class to add additional taint steps that should apply to paths related to
|
||||
* cleartext logging of sensitive data vulnerabilities.
|
||||
*/
|
||||
class CleartextLoggingAdditionalTaintStep extends Unit {
|
||||
/**
|
||||
* Holds if the step from `n1` to `n2` should be considered a taint
|
||||
* step for flows related to cleartext logging of sensitive data vulnerabilities.
|
||||
*/
|
||||
abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
|
||||
}
|
||||
|
||||
private class DefaultCleartextLoggingSink extends CleartextLoggingSink {
|
||||
DefaultCleartextLoggingSink() { sinkNode(this, "logging") }
|
||||
}
|
||||
|
||||
/**
|
||||
* A sanitizer for `OSLogMessage`s configured with the appropriate privacy option.
|
||||
* Numeric and boolean arguments aren't redacted unless the `private` or `sensitive` options are used.
|
||||
* Arguments of other types are always redacted unless the `public` option is used.
|
||||
*/
|
||||
private class OsLogPrivacyCleartextLoggingSanitizer extends CleartextLoggingSanitizer {
|
||||
OsLogPrivacyCleartextLoggingSanitizer() {
|
||||
exists(CallExpr c, AutoClosureExpr e |
|
||||
c.getStaticTarget().getName().matches("appendInterpolation(_:%privacy:%)") and
|
||||
c.getArgument(0).getExpr() = e and
|
||||
this.asExpr() = e
|
||||
|
|
||||
e.getExpr().getType() instanceof OsLogNonRedactedType and
|
||||
c.getArgumentWithLabel("privacy").getExpr().(OsLogPrivacyRef).isSafe()
|
||||
or
|
||||
not e.getExpr().getType() instanceof OsLogNonRedactedType and
|
||||
not c.getArgumentWithLabel("privacy").getExpr().(OsLogPrivacyRef).isPublic()
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/** A type that isn't redacted by default in an `OSLogMessage`. */
|
||||
private class OsLogNonRedactedType extends Type {
|
||||
OsLogNonRedactedType() {
|
||||
this.getName() = [["", "U"] + "Int" + ["", "8", "16", "32", "64"], "Double", "Float", "Bool"]
|
||||
}
|
||||
}
|
||||
|
||||
/** A reference to a field of `OsLogPrivacy`. */
|
||||
private class OsLogPrivacyRef extends MemberRefExpr {
|
||||
string optionName;
|
||||
|
||||
OsLogPrivacyRef() {
|
||||
exists(FieldDecl f | this.getMember() = f |
|
||||
f.getEnclosingDecl().(NominalTypeDecl).getName() = "OSLogPrivacy" and
|
||||
optionName = f.getName()
|
||||
)
|
||||
}
|
||||
|
||||
/** Holds if this is a safe privacy option (private or sensitive). */
|
||||
predicate isSafe() { optionName = ["private", "sensitive"] }
|
||||
|
||||
/** Holds if this is a public (that is, unsafe) privacy option. */
|
||||
predicate isPublic() { optionName = "public" }
|
||||
}
|
||||
|
||||
private class LoggingSinks extends SinkModelCsv {
|
||||
override predicate row(string row) {
|
||||
row =
|
||||
[
|
||||
";;false;print(_:separator:terminator:);;;Argument[0].ArrayElement;logging",
|
||||
";;false;print(_:separator:terminator:);;;Argument[1..2];logging",
|
||||
";;false;print(_:separator:terminator:toStream:);;;Argument[0].ArrayElement;logging",
|
||||
";;false;print(_:separator:terminator:toStream:);;;Argument[1..2];logging",
|
||||
";;false;NSLog(_:_:);;;Argument[0];logging",
|
||||
";;false;NSLog(_:_:);;;Argument[1].ArrayElement;logging",
|
||||
";;false;NSLogv(_:_:);;;Argument[0];logging",
|
||||
";;false;NSLogv(_:_:);;;Argument[1].ArrayElement;logging",
|
||||
";;false;vfprintf(_:_:_:);;;Agument[1..2];logging",
|
||||
";Logger;true;log(_:);;;Argument[0];logging",
|
||||
";Logger;true;log(level:_:);;;Argument[1];logging",
|
||||
";Logger;true;trace(_:);;;Argument[1];logging",
|
||||
";Logger;true;debug(_:);;;Argument[1];logging",
|
||||
";Logger;true;info(_:);;;Argument[1];logging",
|
||||
";Logger;true;notice(_:);;;Argument[1];logging",
|
||||
";Logger;true;warning(_:);;;Argument[1];logging",
|
||||
";Logger;true;error(_:);;;Argument[1];logging",
|
||||
";Logger;true;critical(_:);;;Argument[1];logging",
|
||||
";Logger;true;fault(_:);;;Argument[1];logging",
|
||||
]
|
||||
}
|
||||
}
|
||||
32
swift/ql/lib/codeql/swift/security/CleartextLoggingQuery.qll
Normal file
32
swift/ql/lib/codeql/swift/security/CleartextLoggingQuery.qll
Normal file
@@ -0,0 +1,32 @@
|
||||
/**
|
||||
* Provides a taint-tracking configuration for reasoning about cleartext logging of
|
||||
* sensitive data vulnerabilities.
|
||||
*/
|
||||
|
||||
import swift
|
||||
private import codeql.swift.dataflow.DataFlow
|
||||
private import codeql.swift.dataflow.TaintTracking
|
||||
private import codeql.swift.security.CleartextLogging
|
||||
private import codeql.swift.security.SensitiveExprs
|
||||
|
||||
/**
|
||||
* A taint-tracking configuration for cleartext logging of sensitive data vulnerabilities.
|
||||
*/
|
||||
class CleartextLoggingConfiguration extends TaintTracking::Configuration {
|
||||
CleartextLoggingConfiguration() { this = "CleartextLoggingConfiguration" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveExpr }
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) { sink instanceof CleartextLoggingSink }
|
||||
|
||||
override predicate isSanitizer(DataFlow::Node sanitizer) {
|
||||
sanitizer instanceof CleartextLoggingSanitizer
|
||||
}
|
||||
|
||||
// Disregard paths that contain other paths. This helps with performance.
|
||||
override predicate isSanitizerIn(DataFlow::Node node) { this.isSource(node) }
|
||||
|
||||
override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
|
||||
any(CleartextLoggingAdditionalTaintStep s).step(n1, n2)
|
||||
}
|
||||
}
|
||||
46
swift/ql/src/queries/Security/CWE-312/CleartextLogging.qhelp
Normal file
46
swift/ql/src/queries/Security/CWE-312/CleartextLogging.qhelp
Normal file
@@ -0,0 +1,46 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
Attackers could gain access to sensitive information that is logged unencrypted.
|
||||
</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
Always make sure to encrypt or obfuscate sensitive information before you log it.
|
||||
</p>
|
||||
|
||||
<p>
|
||||
Generally, you should decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.
|
||||
</p>
|
||||
|
||||
<p>
|
||||
Be aware that external processes often store the standard output and
|
||||
standard error streams of the application. This will include logged sensitive information.
|
||||
</p>
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>
|
||||
The following example code logs user credentials (in this case, their password)
|
||||
in plaintext:
|
||||
</p>
|
||||
<sample src="CleartextLoggingBad.swift"/>
|
||||
<p>
|
||||
Instead, you should encrypt or obfuscate the credentials, or omit them entirely:
|
||||
</p>
|
||||
<sample src="CleartextLoggingGood.swift"/>
|
||||
</example>
|
||||
|
||||
<references>
|
||||
|
||||
<li>M. Dowd, J. McDonald and J. Schuhm, <i>The Art of Software Security Assessment</i>, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.</li>
|
||||
<li>M. Howard and D. LeBlanc, <i>Writing Secure Code</i>, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.</li>
|
||||
<li>OWASP: <a href="https://www.owasp.org/index.php/Password_Plaintext_Storage">Password Plaintext Storage</a>.</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
24
swift/ql/src/queries/Security/CWE-312/CleartextLogging.ql
Normal file
24
swift/ql/src/queries/Security/CWE-312/CleartextLogging.ql
Normal file
@@ -0,0 +1,24 @@
|
||||
/**
|
||||
* @name Cleartext logging of sensitive information
|
||||
* @description Logging sensitive information in plaintext can
|
||||
* expose it to an attacker.
|
||||
* @kind path-problem
|
||||
* @problem.severity error
|
||||
* @security-severity 7.5
|
||||
* @precision high
|
||||
* @id swift/clear-text-logging
|
||||
* @tags security
|
||||
* external/cwe/cwe-312
|
||||
* external/cwe/cwe-359
|
||||
* external/cwe/cwe-532
|
||||
*/
|
||||
|
||||
import swift
|
||||
import codeql.swift.dataflow.DataFlow
|
||||
import codeql.swift.security.CleartextLoggingQuery
|
||||
import DataFlow::PathGraph
|
||||
|
||||
from DataFlow::PathNode src, DataFlow::PathNode sink
|
||||
where any(CleartextLoggingConfiguration c).hasFlowPath(src, sink)
|
||||
select sink.getNode(), src, sink, "This $@ is written to a log file.", src.getNode(),
|
||||
"potentially sensitive information"
|
||||
@@ -0,0 +1,2 @@
|
||||
let password = "P@ssw0rd"
|
||||
NSLog("User password changed to \(password)")
|
||||
@@ -0,0 +1,2 @@
|
||||
let password = "P@ssw0rd"
|
||||
NSLog("User password changed")
|
||||
@@ -0,0 +1,24 @@
|
||||
import swift
|
||||
import codeql.swift.dataflow.DataFlow
|
||||
import codeql.swift.security.CleartextLoggingQuery
|
||||
import TestUtilities.InlineExpectationsTest
|
||||
|
||||
class CleartextLogging extends InlineExpectationsTest {
|
||||
CleartextLogging() { this = "CleartextLogging" }
|
||||
|
||||
override string getARelevantTag() { result = "hasCleartextLogging" }
|
||||
|
||||
override predicate hasActualResult(Location location, string element, string tag, string value) {
|
||||
exists(
|
||||
CleartextLoggingConfiguration config, DataFlow::Node source, DataFlow::Node sink,
|
||||
Expr sinkExpr
|
||||
|
|
||||
config.hasFlow(source, sink) and
|
||||
sinkExpr = sink.asExpr() and
|
||||
location = sinkExpr.getLocation() and
|
||||
element = sinkExpr.toString() and
|
||||
tag = "hasCleartextLogging" and
|
||||
value = source.asExpr().getLocation().getStartLine().toString()
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,158 @@
|
||||
// --- stubs ---
|
||||
|
||||
func NSLog(_ format: String, _ args: CVarArg...) {}
|
||||
func NSLogv(_ format: String, _ args: CVaListPointer) {}
|
||||
func getVaList(_ args: [CVarArg]) -> CVaListPointer { return CVaListPointer(_fromUnsafeMutablePointer: UnsafeMutablePointer(bitPattern: 0)!) }
|
||||
|
||||
struct OSLogType : RawRepresentable {
|
||||
static let `default` = OSLogType(rawValue: 0)
|
||||
let rawValue: UInt8
|
||||
init(rawValue: UInt8) { self.rawValue = rawValue}
|
||||
}
|
||||
|
||||
struct OSLogStringAlignment {
|
||||
static var none = OSLogStringAlignment()
|
||||
}
|
||||
|
||||
enum OSLogIntegerFormatting { case decimal }
|
||||
enum OSLogInt32ExtendedFormat { case none }
|
||||
enum OSLogFloatFormatting { case fixed }
|
||||
enum OSLogBoolFormat { case truth }
|
||||
|
||||
struct OSLogPrivacy {
|
||||
enum Mask { case none }
|
||||
static var auto = OSLogPrivacy()
|
||||
static var `private` = OSLogPrivacy()
|
||||
static var `public` = OSLogPrivacy()
|
||||
static var sensitive = OSLogPrivacy()
|
||||
|
||||
static func auto(mask: OSLogPrivacy.Mask) -> OSLogPrivacy { return .auto }
|
||||
static func `private`(mask: OSLogPrivacy.Mask) -> OSLogPrivacy { return .private }
|
||||
static func `sensitive`(mask: OSLogPrivacy.Mask) -> OSLogPrivacy { return .sensitive }
|
||||
}
|
||||
|
||||
struct OSLogInterpolation : StringInterpolationProtocol {
|
||||
typealias StringLiteralType = String
|
||||
init(literalCapacity: Int, interpolationCount: Int) {}
|
||||
func appendLiteral(_: Self.StringLiteralType) {}
|
||||
mutating func appendInterpolation(_ argumentString: @autoclosure @escaping () -> String, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ argumentString: @autoclosure @escaping () -> String, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto, attributes: String) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int8, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int16, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int32, format: OSLogInt32ExtendedFormat, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int32, format: OSLogInt32ExtendedFormat, privacy: OSLogPrivacy = .auto, attributes: String) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int32, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int64, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> UInt, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> UInt8, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> UInt16, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> UInt32, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> UInt64, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Double, format: OSLogFloatFormatting = .fixed, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Double, format: OSLogFloatFormatting = .fixed, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto, attributes: String) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Float,format: OSLogFloatFormatting = .fixed, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
|
||||
mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Float, format: OSLogFloatFormatting = .fixed, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto, attributes: String) {}
|
||||
mutating func appendInterpolation(_ boolean: @autoclosure @escaping () -> Bool, format: OSLogBoolFormat = .truth, privacy: OSLogPrivacy = .auto) {}
|
||||
}
|
||||
|
||||
struct OSLogMessage : ExpressibleByStringInterpolation {
|
||||
typealias StringInterpolation = OSLogInterpolation
|
||||
typealias StringLiteralType = String
|
||||
typealias ExtendedGraphemeClusterLiteralType = String
|
||||
typealias UnicodeScalarLiteralType = String
|
||||
init(stringInterpolation: OSLogInterpolation) {}
|
||||
init(stringLiteral: String) {}
|
||||
init(extendedGraphemeClusterLiteral: String) {}
|
||||
init(unicodeScalarLiteral: String) {}
|
||||
}
|
||||
|
||||
struct Logger {
|
||||
func log(_ message: OSLogMessage) {}
|
||||
func log(level: OSLogType, _ message: OSLogMessage) {}
|
||||
func notice(_: OSLogMessage) {}
|
||||
func debug(_: OSLogMessage) {}
|
||||
func trace(_: OSLogMessage) {}
|
||||
func info(_: OSLogMessage) {}
|
||||
func error(_: OSLogMessage) {}
|
||||
func warning(_: OSLogMessage) {}
|
||||
func fault(_: OSLogMessage) {}
|
||||
func critical(_: OSLogMessage) {}
|
||||
|
||||
}
|
||||
|
||||
// --- tests ---
|
||||
|
||||
func test1(password: String, passwordHash : String) {
|
||||
print(password) // $ MISSING: hasCleartextLogging=87
|
||||
print(password, separator: "") // $ MISSING: $ hasCleartextLogging=88
|
||||
print("", separator: password) // $ hasCleartextLogging=89
|
||||
print(password, separator: "", terminator: "") // $ MISSING: hasCleartextLogging=90
|
||||
print("", separator: password, terminator: "") // $ hasCleartextLogging=91
|
||||
print("", separator: "", terminator: password) // $ hasCleartextLogging=92
|
||||
print(passwordHash) // Safe
|
||||
|
||||
NSLog(password) // $ hasCleartextLogging=95
|
||||
NSLog("%@", password as! CVarArg) // $ MISSING: hasCleartextLogging=96
|
||||
NSLog("%@ %@", "" as! CVarArg, password as! CVarArg) // $ MISSING: hasCleartextLogging=97
|
||||
NSLog("\(password)") // $ hasCleartextLogging=98
|
||||
NSLogv("%@", getVaList([password as! CVarArg])) // $ MISSING: hasCleartextLogging=99
|
||||
NSLogv("%@ %@", getVaList(["" as! CVarArg, password as! CVarArg])) // $ MISSING: hasCleartextLogging=100
|
||||
NSLog(passwordHash) // SAfe
|
||||
NSLogv("%@", getVaList([passwordHash as! CVarArg])) // Safe
|
||||
|
||||
let bankAccount: Int = 0
|
||||
let log = Logger()
|
||||
// These MISSING test cases will be fixed when we properly generate the CFG around autoclosures.
|
||||
log.log("\(password)") // Safe
|
||||
log.log("\(password, privacy: .auto)") // Safe
|
||||
log.log("\(password, privacy: .private)") // Safe
|
||||
log.log("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=110
|
||||
log.log("\(passwordHash, privacy: .public)") // Safe
|
||||
log.log("\(password, privacy: .sensitive)") // Safe
|
||||
log.log("\(bankAccount)") // $ MISSING: hasCleartextLogging=113
|
||||
log.log("\(bankAccount, privacy: .auto)") // $ MISSING: hasCleartextLogging=114
|
||||
log.log("\(bankAccount, privacy: .private)") // Safe
|
||||
log.log("\(bankAccount, privacy: .public)") // $ MISSING: hasCleartextLogging=116
|
||||
log.log("\(bankAccount, privacy: .sensitive)") // Safe
|
||||
log.log(level: .default, "\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=118
|
||||
log.trace("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=119
|
||||
log.trace("\(passwordHash, privacy: .public)") // Safe
|
||||
log.debug("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=121
|
||||
log.debug("\(passwordHash, privacy: .public)") // Safe
|
||||
log.info("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=123
|
||||
log.info("\(passwordHash, privacy: .public)") // Safe
|
||||
log.notice("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=125
|
||||
log.notice("\(passwordHash, privacy: .public)") // Safe
|
||||
log.warning("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=127
|
||||
log.warning("\(passwordHash, privacy: .public)") // Safe
|
||||
log.error("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=129
|
||||
log.error("\(passwordHash, privacy: .public)") // Safe
|
||||
log.critical("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=131
|
||||
log.critical("\(passwordHash, privacy: .public)") // Safe
|
||||
log.fault("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=133
|
||||
log.fault("\(passwordHash, privacy: .public)") // Safe
|
||||
}
|
||||
|
||||
class MyClass {
|
||||
var harmless = "abc"
|
||||
var password = "123"
|
||||
}
|
||||
|
||||
func getPassword() -> String { return "" }
|
||||
func doSomething(password: String) { }
|
||||
|
||||
func test3(x: String) {
|
||||
// alternative evidence of sensitivity...
|
||||
|
||||
NSLog(x) // $ MISSING: hasCleartextLogging=148
|
||||
doSomething(password: x);
|
||||
NSLog(x) // $ hasCleartextLogging=149
|
||||
|
||||
let y = getPassword();
|
||||
NSLog(y) // $ hasCleartextLogging=152
|
||||
|
||||
let z = MyClass()
|
||||
NSLog(z.harmless) // Safe
|
||||
NSLog(z.password) // $ hasCleartextLogging=157
|
||||
}
|
||||
Reference in New Issue
Block a user