Skip to content

Commit 96b4a12

Browse files
geoffw0geoffw0
committed
Swift: Add heuristic sinks.
1 parent 697c3df commit 96b4a12

File tree

3 files changed

+58
-4
lines changed

3 files changed

+58
-4
lines changed

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ private import codeql.swift.StringFormat
88
private import codeql.swift.dataflow.DataFlow
99
private import codeql.swift.dataflow.TaintTracking
1010
private import codeql.swift.dataflow.ExternalFlow
11+
private import codeql.swift.frameworks.StandardLibrary.PointerTypes
1112

1213
/**
1314
* A dataflow sink for uncontrolled format string vulnerabilities.
@@ -43,6 +44,47 @@ private class DefaultUncontrolledFormatStringSink extends UncontrolledFormatStri
4344
}
4445
}
4546

47+
/**
48+
* Holds if `f`, `ix` describe `pd` and `pd` is a parameter that might be a format
49+
* string.
50+
*/
51+
pragma[noinline]
52+
predicate formatLikeHeuristic(Callable f, int ix, ParamDecl pd) {
53+
pd.getName() = ["format", "formatString", "fmt"] and
54+
pd = f.getParam(ix)
55+
}
56+
57+
/**
58+
* An uncontrolled format string sink that is determined by imprecise methods.
59+
*/
60+
class HeuristicUncontrolledFormatStringSink extends UncontrolledFormatStringSink {
61+
HeuristicUncontrolledFormatStringSink() {
62+
exists(Callable f, Type argsType |
63+
(
64+
// by parameter name
65+
exists(CallExpr ce, int ix |
66+
formatLikeHeuristic(f, ix, _) and
67+
f = ce.getStaticTarget() and
68+
this.asExpr() = ce.getArgument(ix).getExpr()
69+
)
70+
or
71+
// by argument name
72+
exists(Argument a |
73+
a.getLabel() = ["format", "formatString", "fmt"] and
74+
a.getApplyExpr().getStaticTarget() = f and
75+
this.asExpr() = a.getExpr()
76+
)
77+
) and
78+
// last parameter is vararg
79+
argsType = f.getParam(f.getNumberOfParams() - 1).getType().getUnderlyingType() and
80+
(
81+
argsType instanceof CVaListPointerType or
82+
argsType instanceof VariadicSequenceType
83+
)
84+
)
85+
}
86+
}
87+
4688
/**
4789
* A barrier for uncontrolled format string vulnerabilities.
4890
*/

‎swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,14 @@ edges
1616
| UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:112:64:112:64 | tainted |
1717
| UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:115:11:115:11 | tainted |
1818
| UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:116:11:116:11 | tainted |
19+
| UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:116:11:116:11 | tainted |
1920
| UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:118:61:118:61 | tainted |
2021
| UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:130:39:130:39 | tainted |
2122
| UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:135:37:135:37 | tainted |
23+
| UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:137:29:137:29 | tainted |
2224
| UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:139:5:139:5 | tainted |
25+
| UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:154:26:154:26 | tainted |
26+
| UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:156:32:156:32 | tainted |
2327
| UncontrolledFormatString.swift:108:43:108:43 | tainted | UncontrolledFormatString.swift:108:26:108:50 | call to NSString.init(string:) |
2428
| UncontrolledFormatString.swift:109:57:109:57 | tainted | UncontrolledFormatString.swift:109:40:109:64 | call to NSString.init(string:) |
2529
| UncontrolledFormatString.swift:111:50:111:50 | tainted | UncontrolledFormatString.swift:111:33:111:57 | call to NSString.init(string:) |
@@ -55,16 +59,20 @@ nodes
5559
| UncontrolledFormatString.swift:112:64:112:64 | tainted | semmle.label | tainted |
5660
| UncontrolledFormatString.swift:115:11:115:11 | tainted | semmle.label | tainted |
5761
| UncontrolledFormatString.swift:116:11:116:11 | tainted | semmle.label | tainted |
62+
| UncontrolledFormatString.swift:116:11:116:11 | tainted | semmle.label | tainted |
5863
| UncontrolledFormatString.swift:118:61:118:61 | tainted | semmle.label | tainted |
5964
| UncontrolledFormatString.swift:130:39:130:39 | tainted | semmle.label | tainted |
6065
| UncontrolledFormatString.swift:135:20:135:44 | call to NSString.init(string:) | semmle.label | call to NSString.init(string:) |
6166
| UncontrolledFormatString.swift:135:37:135:37 | tainted | semmle.label | tainted |
67+
| UncontrolledFormatString.swift:137:29:137:29 | tainted | semmle.label | tainted |
6268
| UncontrolledFormatString.swift:139:5:139:5 | tainted | semmle.label | tainted |
6369
| UncontrolledFormatString.swift:140:9:140:9 | cstr [Collection element] | semmle.label | cstr [Collection element] |
6470
| UncontrolledFormatString.swift:141:24:141:24 | cstr | semmle.label | cstr |
6571
| UncontrolledFormatString.swift:143:21:143:21 | cstr | semmle.label | cstr |
6672
| UncontrolledFormatString.swift:145:27:145:27 | cstr | semmle.label | cstr |
6773
| UncontrolledFormatString.swift:147:35:147:35 | cstr | semmle.label | cstr |
74+
| UncontrolledFormatString.swift:154:26:154:26 | tainted | semmle.label | tainted |
75+
| UncontrolledFormatString.swift:156:32:156:32 | tainted | semmle.label | tainted |
6876
subpaths
6977
#select
7078
| UncontrolledFormatString.swift:79:16:79:16 | format | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:79:16:79:16 | format | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
@@ -80,10 +88,14 @@ subpaths
8088
| UncontrolledFormatString.swift:111:33:111:57 | call to NSString.init(string:) | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:111:33:111:57 | call to NSString.init(string:) | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
8189
| UncontrolledFormatString.swift:112:47:112:71 | call to NSString.init(string:) | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:112:47:112:71 | call to NSString.init(string:) | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
8290
| UncontrolledFormatString.swift:115:11:115:11 | tainted | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:115:11:115:11 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
91+
| UncontrolledFormatString.swift:116:11:116:11 | tainted | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:116:11:116:11 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
8392
| UncontrolledFormatString.swift:118:61:118:61 | tainted | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:118:61:118:61 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
8493
| UncontrolledFormatString.swift:130:39:130:39 | tainted | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:130:39:130:39 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
8594
| UncontrolledFormatString.swift:135:20:135:44 | call to NSString.init(string:) | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:135:20:135:44 | call to NSString.init(string:) | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
95+
| UncontrolledFormatString.swift:137:29:137:29 | tainted | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:137:29:137:29 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
8696
| UncontrolledFormatString.swift:141:24:141:24 | cstr | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:141:24:141:24 | cstr | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
8797
| UncontrolledFormatString.swift:143:21:143:21 | cstr | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:143:21:143:21 | cstr | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
8898
| UncontrolledFormatString.swift:145:27:145:27 | cstr | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:145:27:145:27 | cstr | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
8999
| UncontrolledFormatString.swift:147:35:147:35 | cstr | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:147:35:147:35 | cstr | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
100+
| UncontrolledFormatString.swift:154:26:154:26 | tainted | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:154:26:154:26 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |
101+
| UncontrolledFormatString.swift:156:32:156:32 | tainted | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | UncontrolledFormatString.swift:156:32:156:32 | tainted | This format string depends on $@. | UncontrolledFormatString.swift:91:24:91:77 | call to String.init(contentsOf:) | this user-provided value |

‎swift/ql/test/query-tests/Security/CWE-134/UncontrolledFormatString.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func tests() throws {
113113

114114
NSLog("abc") // GOOD: not tainted
115115
NSLog(tainted) // BAD
116-
MyLog(tainted) // BAD [NOT DETECTED]
116+
MyLog(tainted) // BAD
117117

118118
NSException.raise(NSExceptionName("exception"), format: tainted, arguments: getVaList([])) // BAD
119119

@@ -134,7 +134,7 @@ func tests() throws {
134134
s.appendFormat(NSString(string: "%s"), "abc") // GOOD: not tainted
135135
s.appendFormat(NSString(string: tainted), "abc") // BAD
136136

137-
_ = NSPredicate(format: tainted) // GOOD: this should be flagged by `swift/predicate-injection`, not `swift/uncontrolled-format-string`
137+
_ = NSPredicate(format: tainted) // GOOD: this should be flagged by `swift/predicate-injection`, not `swift/uncontrolled-format-string` [FALSE POSITIVE]
138138

139139
tainted.withCString({
140140
cstr in
@@ -151,8 +151,8 @@ func tests() throws {
151151
myFormatMessage(string: tainted, "abc") // BAD [NOT DETECTED]
152152
myFormatMessage(string: "%s", tainted) // GOOD: format not tainted
153153

154-
_ = MyString(format: tainted, "abc") // BAD [NOT DETECTED]
154+
_ = MyString(format: tainted, "abc") // BAD
155155
_ = MyString(format: "%s", tainted) // GOOD: format not tainted
156-
_ = MyString(formatString: tainted, "abc") // BAD [NOT DETECTED]
156+
_ = MyString(formatString: tainted, "abc") // BAD
157157
_ = MyString(formatString: "%s", tainted) // GOOD: format not tainted
158158
}

0 commit comments

Comments
 (0)