Skip to content

Commit 36f410b

Browse files
geoffw0geoffw0
committed
Swift: Move taint logic from isAdditionalTaintStep to defaultAdditionalTaintStep.
1 parent 242dc80 commit 36f410b

File tree

6 files changed

+59
-33
lines changed

6 files changed

+59
-33
lines changed

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,32 @@ private module Cached {
3636
nodeTo.asDefinition().(Ssa::WriteDefinition).isInoutDef(e)
3737
)
3838
or
39+
// allow flow through `try!` and similar constructs
40+
// TODO: this should probably be part of DataFlow / TaintTracking?
41+
nodeFrom.asExpr() = nodeTo.asExpr().(AnyTryExpr).getSubExpr()
42+
or
43+
// allow flow through `!`
44+
// TODO: this should probably be part of DataFlow / TaintTracking?
45+
nodeFrom.asExpr() = nodeTo.asExpr().(ForceValueExpr).getSubExpr()
46+
or
3947
// Flow from the computation of the interpolated string literal to the result of the interpolation.
4048
exists(InterpolatedStringLiteralExpr interpolated |
4149
nodeTo.asExpr() = interpolated and
4250
nodeFrom.asExpr() = interpolated.getAppendingExpr()
4351
)
52+
or
53+
// allow flow through string concatenation.
54+
nodeTo.asExpr().(AddExpr).getAnOperand() = nodeFrom.asExpr()
55+
or
56+
// allow flow through `URL.init`.
57+
exists(CallExpr call, ClassDecl c, AbstractFunctionDecl f |
58+
c.getName() = "URL" and
59+
c.getAMember() = f and
60+
f.getName() = ["init(string:)", "init(string:relativeTo:)"] and
61+
call.getFunction().(ApplyExpr).getStaticTarget() = f and
62+
nodeFrom.asExpr() = call.getAnArgument().getExpr() and
63+
nodeTo.asExpr() = call
64+
)
4465
}
4566

4667
/**

‎swift/ql/src/queries/Security/CWE-079/UnsafeWebViewFetch.ql‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -81,30 +81,6 @@ class UnsafeWebViewFetchConfig extends TaintTracking::Configuration {
8181
node instanceof Sink or
8282
node.asExpr() = any(Sink s).getBaseUrl()
8383
}
84-
85-
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
86-
// allow flow through `try!` and similar constructs
87-
// TODO: this should probably be part of DataFlow / TaintTracking.
88-
node1.asExpr() = node2.asExpr().(AnyTryExpr).getSubExpr()
89-
or
90-
// allow flow through `!`
91-
// TODO: this should probably be part of DataFlow / TaintTracking.
92-
node1.asExpr() = node2.asExpr().(ForceValueExpr).getSubExpr()
93-
or
94-
// allow flow through string concatenation.
95-
// TODO: this should probably be part of TaintTracking.
96-
node2.asExpr().(AddExpr).getAnOperand() = node1.asExpr()
97-
or
98-
// allow flow through `URL.init`.
99-
exists(CallExpr call, ClassDecl c, AbstractFunctionDecl f |
100-
c.getName() = "URL" and
101-
c.getAMember() = f and
102-
f.getName() = ["init(string:)", "init(string:relativeTo:)"] and
103-
call.getFunction().(ApplyExpr).getStaticTarget() = f and
104-
node1.asExpr() = call.getAnArgument().getExpr() and
105-
node2.asExpr() = call
106-
)
107-
}
10884
}
10985

11086
from

‎swift/ql/test/library-tests/dataflow/taint/Taint.expected‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@ edges
55
| string.swift:5:11:5:18 | call to source() : | string.swift:16:13:16:13 | "..." |
66
| string.swift:5:11:5:18 | call to source() : | string.swift:18:13:18:13 | "..." |
77
| string.swift:28:17:28:25 | call to source2() : | string.swift:31:13:31:13 | tainted |
8+
| string.swift:28:17:28:25 | call to source2() : | string.swift:34:13:34:21 | ... call to +(_:_:) ... |
9+
| string.swift:28:17:28:25 | call to source2() : | string.swift:35:13:35:23 | ... call to +(_:_:) ... |
10+
| string.swift:28:17:28:25 | call to source2() : | string.swift:36:13:36:23 | ... call to +(_:_:) ... |
11+
| string.swift:28:17:28:25 | call to source2() : | string.swift:39:13:39:29 | ... call to +(_:_:) ... |
12+
| try.swift:9:17:9:24 | call to source() : | try.swift:9:13:9:24 | try ... |
13+
| url.swift:13:16:13:23 | call to source() : | url.swift:18:12:18:12 | urlTainted |
14+
| url.swift:13:16:13:23 | call to source() : | url.swift:21:12:21:49 | ...! |
15+
| url.swift:13:16:13:23 | call to source() : | url.swift:23:12:23:54 | ...! |
16+
| url.swift:13:16:13:23 | call to source() : | url.swift:39:12:39:12 | ...! |
817
nodes
918
| string.swift:5:11:5:18 | call to source() : | semmle.label | call to source() : |
1019
| string.swift:7:13:7:13 | "..." | semmle.label | "..." |
@@ -14,6 +23,17 @@ nodes
1423
| string.swift:18:13:18:13 | "..." | semmle.label | "..." |
1524
| string.swift:28:17:28:25 | call to source2() : | semmle.label | call to source2() : |
1625
| string.swift:31:13:31:13 | tainted | semmle.label | tainted |
26+
| string.swift:34:13:34:21 | ... call to +(_:_:) ... | semmle.label | ... call to +(_:_:) ... |
27+
| string.swift:35:13:35:23 | ... call to +(_:_:) ... | semmle.label | ... call to +(_:_:) ... |
28+
| string.swift:36:13:36:23 | ... call to +(_:_:) ... | semmle.label | ... call to +(_:_:) ... |
29+
| string.swift:39:13:39:29 | ... call to +(_:_:) ... | semmle.label | ... call to +(_:_:) ... |
30+
| try.swift:9:13:9:24 | try ... | semmle.label | try ... |
31+
| try.swift:9:17:9:24 | call to source() : | semmle.label | call to source() : |
32+
| url.swift:13:16:13:23 | call to source() : | semmle.label | call to source() : |
33+
| url.swift:18:12:18:12 | urlTainted | semmle.label | urlTainted |
34+
| url.swift:21:12:21:49 | ...! | semmle.label | ...! |
35+
| url.swift:23:12:23:54 | ...! | semmle.label | ...! |
36+
| url.swift:39:12:39:12 | ...! | semmle.label | ...! |
1737
subpaths
1838
#select
1939
| string.swift:7:13:7:13 | "..." | string.swift:5:11:5:18 | call to source() : | string.swift:7:13:7:13 | "..." | result |
@@ -22,3 +42,12 @@ subpaths
2242
| string.swift:16:13:16:13 | "..." | string.swift:5:11:5:18 | call to source() : | string.swift:16:13:16:13 | "..." | result |
2343
| string.swift:18:13:18:13 | "..." | string.swift:5:11:5:18 | call to source() : | string.swift:18:13:18:13 | "..." | result |
2444
| string.swift:31:13:31:13 | tainted | string.swift:28:17:28:25 | call to source2() : | string.swift:31:13:31:13 | tainted | result |
45+
| string.swift:34:13:34:21 | ... call to +(_:_:) ... | string.swift:28:17:28:25 | call to source2() : | string.swift:34:13:34:21 | ... call to +(_:_:) ... | result |
46+
| string.swift:35:13:35:23 | ... call to +(_:_:) ... | string.swift:28:17:28:25 | call to source2() : | string.swift:35:13:35:23 | ... call to +(_:_:) ... | result |
47+
| string.swift:36:13:36:23 | ... call to +(_:_:) ... | string.swift:28:17:28:25 | call to source2() : | string.swift:36:13:36:23 | ... call to +(_:_:) ... | result |
48+
| string.swift:39:13:39:29 | ... call to +(_:_:) ... | string.swift:28:17:28:25 | call to source2() : | string.swift:39:13:39:29 | ... call to +(_:_:) ... | result |
49+
| try.swift:9:13:9:24 | try ... | try.swift:9:17:9:24 | call to source() : | try.swift:9:13:9:24 | try ... | result |
50+
| url.swift:18:12:18:12 | urlTainted | url.swift:13:16:13:23 | call to source() : | url.swift:18:12:18:12 | urlTainted | result |
51+
| url.swift:21:12:21:49 | ...! | url.swift:13:16:13:23 | call to source() : | url.swift:21:12:21:49 | ...! | result |
52+
| url.swift:23:12:23:54 | ...! | url.swift:13:16:13:23 | call to source() : | url.swift:23:12:23:54 | ...! | result |
53+
| url.swift:39:12:39:12 | ...! | url.swift:13:16:13:23 | call to source() : | url.swift:39:12:39:12 | ...! | result |

‎swift/ql/test/library-tests/dataflow/taint/string.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,12 @@ func taintThroughStringConcatenation() {
3131
sink(arg: tainted) // tainted
3232

3333
sink(arg: clean + clean)
34-
sink(arg: clean + tainted) // tainted [NOT DETECTED]
35-
sink(arg: tainted + clean) // tainted [NOT DETECTED]
36-
sink(arg: tainted + tainted) // tainted [NOT DETECTED]
34+
sink(arg: clean + tainted) // tainted
35+
sink(arg: tainted + clean) // tainted
36+
sink(arg: tainted + tainted) // tainted
3737

3838
sink(arg: ">" + clean + "<")
39-
sink(arg: ">" + tainted + "<") // tainted [NOT DETECTED]
39+
sink(arg: ">" + tainted + "<") // tainted
4040

4141
var str = "abc"
4242

‎swift/ql/test/library-tests/dataflow/taint/try.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ func taintThroughTry() {
66
do
77
{
88
sink(arg: try clean())
9-
sink(arg: try source()) // tainted [NOT DETECTED]
9+
sink(arg: try source()) // tainted
1010
} catch {
1111
// ...
1212
}

‎swift/ql/test/library-tests/dataflow/taint/url.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@ func taintThroughURL() {
1515
let urlTainted = URL(string: tainted)!
1616

1717
sink(arg: urlClean)
18-
sink(arg: urlTainted) // tainted [NOT DETECTED]
18+
sink(arg: urlTainted) // tainted
1919

2020
sink(arg: URL(string: clean, relativeTo: nil)!)
21-
sink(arg: URL(string: tainted, relativeTo: nil)!) // tainted [NOT DETECTED]
21+
sink(arg: URL(string: tainted, relativeTo: nil)!) // tainted
2222
sink(arg: URL(string: clean, relativeTo: urlClean)!)
23-
sink(arg: URL(string: clean, relativeTo: urlTainted)!) // tainted [NOT DETECTED]
23+
sink(arg: URL(string: clean, relativeTo: urlTainted)!) // tainted
2424

2525
if let x = URL(string: clean) {
2626
sink(arg: x)
@@ -36,5 +36,5 @@ func taintThroughURL() {
3636

3737
var urlTainted2 : URL!
3838
urlTainted2 = URL(string: tainted)
39-
sink(arg: urlTainted2) // tainted [NOT DETECTED]
39+
sink(arg: urlTainted2) // tainted
4040
}

0 commit comments

Comments
 (0)