Skip to content

Commit 4323bee

Browse files
geoffw0geoffw0
authored
Merge pull request #13980 from geoffw0/logfix
Swift: Improvements related to the swift/cleartext-logging query.
2 parents 734a91d + 86b0fae commit 4323bee

File tree

15 files changed

+173
-22
lines changed

15 files changed

+173
-22
lines changed

‎swift/ql/.generated.list

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎swift/ql/.gitattributes

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎swift/ql/lib/change-notes/2023-08-16-varargs.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
5+
* Added flow through variadic arguments, and the `getVaList` function.

‎swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,9 @@ private module Cached {
221221
// flow through unary `+` (which does nothing)
222222
nodeFrom.asExpr() = nodeTo.asExpr().(UnaryPlusExpr).getOperand()
223223
or
224+
// flow through varargs expansions (that wrap an `ArrayExpr` where varargs enter a call)
225+
nodeFrom.asExpr() = nodeTo.asExpr().(VarargExpansionExpr).getSubExpr()
226+
or
224227
// flow through nil-coalescing operator `??`
225228
exists(BinaryExpr nco |
226229
nco.getOperator().(FreeFunction).getName() = "??(_:_:)" and
@@ -966,7 +969,7 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
966969
node2.(KeyPathReturnNodeImpl).getKeyPathExpr() = component.getKeyPathExpr()
967970
)
968971
or
969-
// read of an array member via subscript operator
972+
// read of array or collection content via subscript operator
970973
exists(SubscriptExpr subscript |
971974
subscript.getBase() = node1.asExpr() and
972975
subscript = node2.asExpr() and

‎swift/ql/lib/codeql/swift/elements/type/VariadicSequenceType.qll

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
// generated by codegen/codegen.py, remove this comment if you wish to edit this file
21
private import codeql.swift.generated.type.VariadicSequenceType
32

3+
/**
4+
* A variadic sequence type, that is, an array-like type that holds
5+
* variadic arguments. For example the type `Int...` of `args` in:
6+
* ```
7+
* func myVarargsFunction(args: Int...) {
8+
* ...
9+
* }
10+
* ```
11+
*/
412
class VariadicSequenceType extends Generated::VariadicSequenceType { }

‎swift/ql/lib/codeql/swift/frameworks/StandardLibrary/CInterop.qll

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* Provides models for Swift "C Interoperability" functions.
3+
*/
4+
5+
import swift
6+
private import codeql.swift.dataflow.ExternalFlow
7+
8+
private class CInteropSummaries extends SummaryModelCsv {
9+
override predicate row(string row) {
10+
row = ";;false;getVaList(_:);;;Argument[0].ArrayElement;ReturnValue;value"
11+
}
12+
}

‎swift/ql/lib/codeql/swift/frameworks/StandardLibrary/StandardLibrary.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44

55
private import Array
6+
private import CInterop
67
private import Collection
78
private import CustomUrlSchemes
89
private import Data

‎swift/ql/lib/codeql/swift/security/CleartextLoggingExtensions.qll

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,17 @@ private class LoggingSinks extends SinkModelCsv {
9797
override predicate row(string row) {
9898
row =
9999
[
100-
";;false;print(_:separator:terminator:);;;Argument[0].ArrayElement;log-injection",
101-
";;false;print(_:separator:terminator:);;;Argument[1..2];log-injection",
102-
";;false;print(_:separator:terminator:toStream:);;;Argument[0].ArrayElement;log-injection",
103-
";;false;print(_:separator:terminator:toStream:);;;Argument[1..2];log-injection",
104-
";;false;NSLog(_:_:);;;Argument[0];log-injection",
105-
";;false;NSLog(_:_:);;;Argument[1].ArrayElement;log-injection",
106-
";;false;NSLogv(_:_:);;;Argument[0];log-injection",
107-
";;false;NSLogv(_:_:);;;Argument[1].ArrayElement;log-injection",
100+
";;false;print(_:separator:terminator:);;;Argument[0..2];log-injection",
101+
";;false;print(_:separator:terminator:toStream:);;;Argument[0..2];log-injection",
102+
";;false;print(_:separator:terminator:to:);;;Argument[0..2];log-injection",
103+
";;false;debugPrint(_:separator:terminator:);;;Argument[0..2];log-injection",
104+
";;false;debugPrint(_:separator:terminator:to:);;;Argument[0..2];log-injection",
105+
";;false;dump(_:name:indent:maxDepth:maxItems:);;;Argument[0..1];log-injection",
106+
";;false;dump(_:to:name:indent:maxDepth:maxItems:);;;Argument[0];log-injection",
107+
";;false;dump(_:to:name:indent:maxDepth:maxItems:);;;Argument[2];log-injection",
108+
";;false;fatalError(_:file:line:);;;Argument[0];log-injection",
109+
";;false;NSLog(_:_:);;;Argument[0..1];log-injection",
110+
";;false;NSLogv(_:_:);;;Argument[0..1];log-injection",
108111
";;false;vfprintf(_:_:_:);;;Agument[1..2];log-injection",
109112
";Logger;true;log(_:);;;Argument[0];log-injection",
110113
";Logger;true;log(level:_:);;;Argument[1];log-injection",

‎swift/ql/lib/codeql/swift/security/CleartextLoggingQuery.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ module CleartextLoggingConfig implements DataFlow::ConfigSig {
2525
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
2626
any(CleartextLoggingAdditionalFlowStep s).step(n1, n2)
2727
}
28+
29+
predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
30+
// flow out from collection content at the sink.
31+
isSink(node) and
32+
c.getAReadContent() instanceof DataFlow::Content::CollectionContent
33+
}
2834
}
2935

3036
/**

‎swift/ql/src/change-notes/2023-08-16-cleartext-logging.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
5+
* Added new logging sinks to the `swift/cleartext-logging` query.

0 commit comments

Comments
 (0)