Skip to content

Commit 0eeaf71

Browse files
atorralbaatorralba
committed
Simplify models by introducing TaintInheritingContent
1 parent 81d3813 commit 0eeaf71

File tree

6 files changed

+239
-460
lines changed

6 files changed

+239
-460
lines changed

‎swift/ql/lib/codeql/swift/dataflow/FlowSteps.qll‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import swift
2+
private import codeql.swift.dataflow.DataFlow
3+
4+
/**
5+
* A `Content` that should be implicitly regarded as tainted whenever an object with such `Content`
6+
* is itself tainted.
7+
*
8+
* For example, if we had a type `class Container { var field: Contained }`, then by default a tainted
9+
* `Container` and a `Container` with a tainted `Contained` stored in its `field` are distinct.
10+
*
11+
* If `any(DataFlow::FieldContent fc | fc.getField().hasQualifiedName("Container", "field"))` was
12+
* included in this type however, then a tainted `Container` would imply that its `field` is also
13+
* tainted (but not vice versa).
14+
*/
15+
abstract class TaintInheritingContent extends DataFlow::Content { }

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ private import swift
22
private import DataFlowPrivate
33
private import TaintTrackingPublic
44
private import codeql.swift.dataflow.DataFlow
5+
private import codeql.swift.dataflow.FlowSteps
56
private import codeql.swift.dataflow.Ssa
67
private import codeql.swift.controlflow.CfgNodes
78
private import FlowSummaryImpl as FlowSummaryImpl
@@ -55,6 +56,12 @@ private module Cached {
5556
se = nodeTo.asExpr()
5657
)
5758
or
59+
// flow through the read of a content that inherits taint
60+
exists(DataFlow::ContentSet f |
61+
readStep(nodeFrom, f, nodeTo) and
62+
f.getAReadContent() instanceof TaintInheritingContent
63+
)
64+
or
5865
// flow through a flow summary (extension of `SummaryModelCsv`)
5966
FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom, nodeTo, false)
6067
}

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

Lines changed: 15 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,19 @@
11
import swift
2+
private import codeql.swift.dataflow.DataFlow
23
private import codeql.swift.dataflow.ExternalFlow
4+
private import codeql.swift.dataflow.FlowSteps
5+
6+
/** The struct `URL`. */
7+
class UrlDecl extends StructDecl {
8+
UrlDecl() { this.getFullName() = "URL" }
9+
}
10+
11+
/**
12+
* A content implying that, if a `URL` is tainted, then all its fields are tainted.
13+
*/
14+
private class UriFieldsInheritTaint extends TaintInheritingContent, DataFlow::Content::FieldContent {
15+
UriFieldsInheritTaint() { this.getField().getEnclosingDecl() instanceof UrlDecl }
16+
}
317

418
/**
519
* A model for `URL` members that are sources of remote flow.
@@ -22,53 +36,7 @@ private class UrlSummaries extends SummaryModelCsv {
2236
row =
2337
[
2438
";URL;true;init(string:);(String);;Argument[0];ReturnValue;taint",
25-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue;taint",
26-
// The base string taints all the URL fields (except baseURL)
27-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[absoluteURL];taint",
28-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[fragment];taint",
29-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[host];taint",
30-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[lastPathComponent];taint",
31-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[path];taint",
32-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[pathComponents];taint",
33-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[pathExtension];taint",
34-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[port];taint",
35-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[query];taint",
36-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[relativePath];taint",
37-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[relativeString];taint",
38-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[scheme];taint",
39-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[standardized];taint",
40-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[standardizedFileURL];taint",
41-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[user];taint",
42-
";URL;true;init(string:);(String);;Argument[0];ReturnValue.Field[password];taint",
43-
// The base string taints all the URL fields (except baseURL) if it's an absolute URL when relativeTo is used
44-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[absoluteURL];taint",
45-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[fragment];taint",
46-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[host];taint",
47-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[lastPathComponent];taint",
48-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[path];taint",
49-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[pathComponents];taint",
50-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[pathExtension];taint",
51-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[port];taint",
52-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[query];taint",
53-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[relativePath];taint",
54-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[relativeString];taint",
55-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[scheme];taint",
56-
// Not mapping precise field taint to standardized/standardizedFileURL even if the return values are URLs too
57-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[standardized];taint",
58-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[standardizedFileURL];taint",
59-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[user];taint",
60-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0];ReturnValue.Field[password];taint",
61-
// The relativeTo URL taints fields not related to the path, query or fragment if the base string is a relative path
62-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[absoluteURL];taint",
63-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[baseURL];taint",
64-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[host];taint",
65-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[port];taint",
66-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[scheme];taint",
67-
// Not mapping precise field taint to standardized/standardizedFileURL even if the return values are URLs too
68-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[standardized];taint",
69-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[standardizedFileURL];taint",
70-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[user];taint",
71-
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[1];ReturnValue.Field[password];taint",
39+
";URL;true;init(string:relativeTo:);(String,URL?);;Argument[0,1];ReturnValue;taint"
7240
]
7341
}
7442
}

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,5 +125,39 @@
125125
| string.swift:39:29:39:29 | < | string.swift:39:13:39:29 | ... .+(_:_:) ... |
126126
| subscript.swift:13:10:13:17 | call to source() | subscript.swift:13:10:13:20 | ...[...] |
127127
| subscript.swift:14:10:14:18 | call to source2() | subscript.swift:14:10:14:21 | ...[...] |
128+
| url.swift:64:12:64:12 | urlTainted | url.swift:64:12:64:23 | .absoluteURL |
129+
| url.swift:65:12:65:12 | urlTainted | url.swift:65:12:65:23 | .baseURL |
130+
| url.swift:66:15:66:15 | urlTainted | url.swift:66:15:66:26 | .fragment |
131+
| url.swift:67:15:67:15 | urlTainted | url.swift:67:15:67:26 | .host |
132+
| url.swift:68:15:68:15 | urlTainted | url.swift:68:15:68:26 | .lastPathComponent |
133+
| url.swift:69:15:69:15 | urlTainted | url.swift:69:15:69:26 | .path |
134+
| url.swift:70:15:70:15 | urlTainted | url.swift:70:15:70:26 | .pathComponents |
128135
| url.swift:70:15:70:26 | .pathComponents | url.swift:70:15:70:42 | ...[...] |
136+
| url.swift:71:15:71:15 | urlTainted | url.swift:71:15:71:26 | .pathExtension |
137+
| url.swift:72:12:72:12 | urlTainted | url.swift:72:12:72:23 | .port |
138+
| url.swift:73:15:73:15 | urlTainted | url.swift:73:15:73:26 | .query |
139+
| url.swift:74:15:74:15 | urlTainted | url.swift:74:15:74:26 | .relativePath |
140+
| url.swift:75:15:75:15 | urlTainted | url.swift:75:15:75:26 | .relativeString |
141+
| url.swift:76:15:76:15 | urlTainted | url.swift:76:15:76:26 | .scheme |
142+
| url.swift:77:12:77:12 | urlTainted | url.swift:77:12:77:23 | .standardized |
143+
| url.swift:78:12:78:12 | urlTainted | url.swift:78:12:78:23 | .standardizedFileURL |
144+
| url.swift:79:15:79:15 | urlTainted | url.swift:79:15:79:26 | .user |
145+
| url.swift:80:15:80:15 | urlTainted | url.swift:80:15:80:26 | .password |
146+
| url.swift:86:12:86:54 | ...! | url.swift:86:12:86:56 | .absoluteURL |
147+
| url.swift:87:12:87:54 | ...! | url.swift:87:12:87:56 | .baseURL |
148+
| url.swift:88:15:88:57 | ...! | url.swift:88:15:88:59 | .fragment |
149+
| url.swift:89:15:89:57 | ...! | url.swift:89:15:89:59 | .host |
150+
| url.swift:90:15:90:57 | ...! | url.swift:90:15:90:59 | .lastPathComponent |
151+
| url.swift:91:15:91:57 | ...! | url.swift:91:15:91:59 | .path |
152+
| url.swift:92:15:92:57 | ...! | url.swift:92:15:92:59 | .pathComponents |
129153
| url.swift:92:15:92:59 | .pathComponents | url.swift:92:15:92:75 | ...[...] |
154+
| url.swift:93:15:93:57 | ...! | url.swift:93:15:93:59 | .pathExtension |
155+
| url.swift:94:12:94:54 | ...! | url.swift:94:12:94:56 | .port |
156+
| url.swift:95:15:95:57 | ...! | url.swift:95:15:95:59 | .query |
157+
| url.swift:96:15:96:57 | ...! | url.swift:96:15:96:59 | .relativePath |
158+
| url.swift:97:15:97:57 | ...! | url.swift:97:15:97:59 | .relativeString |
159+
| url.swift:98:15:98:57 | ...! | url.swift:98:15:98:59 | .scheme |
160+
| url.swift:99:12:99:54 | ...! | url.swift:99:12:99:56 | .standardized |
161+
| url.swift:100:12:100:54 | ...! | url.swift:100:12:100:56 | .standardizedFileURL |
162+
| url.swift:101:15:101:57 | ...! | url.swift:101:15:101:59 | .user |
163+
| url.swift:102:15:102:57 | ...! | url.swift:102:15:102:59 | .password |

0 commit comments

Comments
 (0)