Skip to content

Commit d5952a1

Browse files
geoffw0geoffw0
authored
Merge pull request github#12329 from geoffw0/network
Swift: Modernize the cleartext-* queries
2 parents 687f3c6 + 5110cf1 commit d5952a1

11 files changed

+449
-301
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ private module Frameworks {
9393
private import codeql.swift.frameworks.StandardLibrary.WebView
9494
private import codeql.swift.frameworks.Alamofire.Alamofire
9595
private import codeql.swift.security.CleartextLoggingExtensions
96+
private import codeql.swift.security.CleartextStorageDatabaseExtensions
9697
private import codeql.swift.security.PathInjectionExtensions
9798
private import codeql.swift.security.PredicateInjectionExtensions
9899
}

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

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/**
2+
* Provides classes and predicates for reasoning about cleartext database
3+
* storage vulnerabilities.
4+
*/
5+
6+
import swift
7+
import codeql.swift.security.SensitiveExprs
8+
import codeql.swift.dataflow.DataFlow
9+
import codeql.swift.dataflow.ExternalFlow
10+
11+
/**
12+
* A dataflow sink for cleartext database storage vulnerabilities. That is,
13+
* a `DataFlow::Node` that is something stored in a local database.
14+
*/
15+
abstract class CleartextStorageDatabaseSink extends DataFlow::Node { }
16+
17+
/**
18+
* A sanitizer for cleartext database storage vulnerabilities.
19+
*/
20+
abstract class CleartextStorageDatabaseSanitizer extends DataFlow::Node { }
21+
22+
/**
23+
* A unit class for adding additional taint steps.
24+
*/
25+
class CleartextStorageDatabaseAdditionalTaintStep extends Unit {
26+
/**
27+
* Holds if the step from `node1` to `node2` should be considered a taint
28+
* step for paths related to cleartext database storage vulnerabilities.
29+
*/
30+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
31+
}
32+
33+
/**
34+
* A `DataFlow::Node` that is an expression stored with the Core Data library.
35+
*/
36+
private class CoreDataStore extends CleartextStorageDatabaseSink {
37+
CoreDataStore() {
38+
// values written into Core Data objects through `set*Value` methods are a sink.
39+
exists(CallExpr call |
40+
call.getStaticTarget()
41+
.(MethodDecl)
42+
.hasQualifiedName("NSManagedObject",
43+
["setValue(_:forKey:)", "setPrimitiveValue(_:forKey:)"]) and
44+
call.getArgument(0).getExpr() = this.asExpr()
45+
)
46+
or
47+
// any write into a class derived from `NSManagedObject` is a sink. For
48+
// example in `coreDataObj.data = sensitive` the post-update node corresponding
49+
// with `coreDataObj.data` is a sink.
50+
// (ideally this would be only members with the `@NSManaged` attribute)
51+
exists(ClassOrStructDecl cd, Expr e |
52+
cd.getABaseTypeDecl*().getName() = "NSManagedObject" and
53+
this.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = e and
54+
e.getFullyConverted().getType() = cd.getType() and
55+
not e.(DeclRefExpr).getDecl() instanceof SelfParamDecl
56+
)
57+
}
58+
}
59+
60+
/**
61+
* A `DataFlow::Node` that is an expression stored with the Realm database
62+
* library.
63+
*/
64+
private class RealmStore extends CleartextStorageDatabaseSink instanceof DataFlow::PostUpdateNode {
65+
RealmStore() {
66+
// any write into a class derived from `RealmSwiftObject` is a sink. For
67+
// example in `realmObj.data = sensitive` the post-update node corresponding
68+
// with `realmObj.data` is a sink.
69+
exists(ClassOrStructDecl cd, Expr e |
70+
cd.getABaseTypeDecl*().getName() = "RealmSwiftObject" and
71+
this.getPreUpdateNode().asExpr() = e and
72+
e.getFullyConverted().getType() = cd.getType() and
73+
not e.(DeclRefExpr).getDecl() instanceof SelfParamDecl
74+
)
75+
}
76+
}
77+
78+
private class CleartextStorageDatabaseSinks extends SinkModelCsv {
79+
override predicate row(string row) {
80+
row =
81+
[
82+
// GRDB sinks
83+
";Database;true;allStatements(sql:arguments:);;;Argument[1];database-store",
84+
";Database;true;execute(sql:arguments:);;;Argument[1];database-store",
85+
";SQLRequest;true;init(sql:arguments:adapter:cached:);;;Argument[1];database-store",
86+
";SQL;true;init(sql:arguments:);;;Argument[1];database-store",
87+
";SQL;true;append(sql:arguments:);;;Argument[1];database-store",
88+
";SQLStatementCursor;true;init(database:sql:arguments:prepFlags:);;;Argument[2];database-store",
89+
";TableRecord;true;select(sql:arguments:);;;Argument[1];database-store",
90+
";TableRecord;true;select(sql:arguments:as:);;;Argument[1];database-store",
91+
";TableRecord;true;filter(sql:arguments:);;;Argument[1];database-store",
92+
";TableRecord;true;order(sql:arguments:);;;Argument[1];database-store",
93+
";Row;true;fetchCursor(_:sql:arguments:adapter:);;;Argument[2];database-store",
94+
";Row;true;fetchAll(_:sql:arguments:adapter:);;;Argument[2];database-store",
95+
";Row;true;fetchSet(_:sql:arguments:adapter:);;;Argument[2];database-store",
96+
";Row;true;fetchOne(_:sql:arguments:adapter:);;;Argument[2];database-store",
97+
";DatabaseValueConvertible;true;fetchCursor(_:sql:arguments:adapter:);;;Argument[2];database-store",
98+
";DatabaseValueConvertible;true;fetchAll(_:sql:arguments:adapter:);;;Argument[2];database-store",
99+
";DatabaseValueConvertible;true;fetchSet(_:sql:arguments:adapter:);;;Argument[2];database-store",
100+
";DatabaseValueConvertible;true;fetchOne(_:sql:arguments:adapter:);;;Argument[2];database-store",
101+
";FetchableRecord;true;fetchCursor(_:sql:arguments:adapter:);;;Argument[2];database-store",
102+
";FetchableRecord;true;fetchAll(_:sql:arguments:adapter:);;;Argument[2];database-store",
103+
";FetchableRecord;true;fetchSet(_:sql:arguments:adapter:);;;Argument[2];database-store",
104+
";FetchableRecord;true;fetchOne(_:sql:arguments:adapter:);;;Argument[2];database-store",
105+
";FetchableRecord;true;fetchCursor(_:arguments:adapter:);;;Argument[1];database-store",
106+
";FetchableRecord;true;fetchAll(_:arguments:adapter:);;;Argument[1];database-store",
107+
";FetchableRecord;true;fetchSet(_:arguments:adapter:);;;Argument[1];database-store",
108+
";FetchableRecord;true;fetchOne(_:arguments:adapter:);;;Argument[1];database-store",
109+
";Statement;true;execute(arguments:);;;Argument[0];database-store",
110+
";CommonTableExpression;true;init(recursive:named:columns:sql:arguments:);;;Argument[4];database-store",
111+
";Statement;true;setArguments(_:);;;Argument[0];database-store"
112+
]
113+
}
114+
}
115+
116+
/**
117+
* An encryption sanitizer for cleartext database storage vulnerabilities.
118+
*/
119+
private class CleartextStorageDatabaseEncryptionSanitizer extends CleartextStorageDatabaseSanitizer {
120+
CleartextStorageDatabaseEncryptionSanitizer() { this.asExpr() instanceof EncryptedExpr }
121+
}
122+
123+
/**
124+
* An additional taint step for cleartext database storage vulnerabilities.
125+
* Needed until we have proper content flow through arrays.
126+
*/
127+
private class CleartextStorageDatabaseArrayAdditionalTaintStep extends CleartextStorageDatabaseAdditionalTaintStep {
128+
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
129+
exists(ArrayExpr arr |
130+
nodeFrom.asExpr() = arr.getAnElement() and
131+
nodeTo.asExpr() = arr
132+
)
133+
}
134+
}
135+
136+
/**
137+
* A sink defined in a CSV model.
138+
*/
139+
private class DefaultCleartextStorageDatabaseSink extends CleartextStorageDatabaseSink {
140+
DefaultCleartextStorageDatabaseSink() { sinkNode(this, "database-store") }
141+
}

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about cleartext
3+
* database storage vulnerabilities.
4+
*/
5+
6+
import swift
7+
import codeql.swift.security.SensitiveExprs
8+
import codeql.swift.dataflow.DataFlow
9+
import codeql.swift.dataflow.TaintTracking
10+
import codeql.swift.security.CleartextStorageDatabaseExtensions
11+
12+
/**
13+
* A taint configuration from sensitive information to expressions that are
14+
* transmitted over a network.
15+
*/
16+
class CleartextStorageConfig extends TaintTracking::Configuration {
17+
CleartextStorageConfig() { this = "CleartextStorageConfig" }
18+
19+
override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr }
20+
21+
override predicate isSink(DataFlow::Node node) { node instanceof CleartextStorageDatabaseSink }
22+
23+
override predicate isSanitizer(DataFlow::Node sanitizer) {
24+
sanitizer instanceof CleartextStorageDatabaseSanitizer
25+
}
26+
27+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
28+
any(CleartextStorageDatabaseAdditionalTaintStep s).step(nodeFrom, nodeTo)
29+
}
30+
31+
override predicate isSanitizerIn(DataFlow::Node node) {
32+
// make sources barriers so that we only report the closest instance
33+
isSource(node)
34+
}
35+
36+
override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
37+
// flow out from fields of an `NSManagedObject` or `RealmSwiftObject` at the sink,
38+
// for example in `realmObj.data = sensitive`.
39+
isSink(node) and
40+
exists(ClassOrStructDecl cd, Decl cx |
41+
cd.getABaseTypeDecl*().getName() = ["NSManagedObject", "RealmSwiftObject"] and
42+
cx.asNominalTypeDecl() = cd and
43+
c.getAReadContent().(DataFlow::Content::FieldContent).getField() = cx.getAMember()
44+
)
45+
or
46+
// any default implicit reads
47+
super.allowImplicitRead(node, c)
48+
}
49+
}

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

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/**
2+
* Provides classes and predicates for reasoning about cleartext preferences
3+
* storage vulnerabilities.
4+
*/
5+
6+
import swift
7+
import codeql.swift.security.SensitiveExprs
8+
import codeql.swift.dataflow.DataFlow
9+
import codeql.swift.dataflow.ExternalFlow
10+
11+
/**
12+
* A dataflow sink for cleartext preferences storage vulnerabilities. That is,
13+
* a `DataFlow::Node` of something that gets stored in an application
14+
* preference store.
15+
*/
16+
abstract class CleartextStoragePreferencesSink extends DataFlow::Node {
17+
abstract string getStoreName();
18+
}
19+
20+
/**
21+
* A sanitizer for cleartext preferences storage vulnerabilities.
22+
*/
23+
abstract class CleartextStoragePreferencesSanitizer extends DataFlow::Node { }
24+
25+
/**
26+
* A unit class for adding additional taint steps.
27+
*/
28+
class CleartextStoragePreferencesAdditionalTaintStep extends Unit {
29+
/**
30+
* Holds if the step from `node1` to `node2` should be considered a taint
31+
* step for paths related to cleartext preferences storage vulnerabilities.
32+
*/
33+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
34+
}
35+
36+
/** The `DataFlow::Node` of an expression that gets written to the user defaults database */
37+
private class UserDefaultsStore extends CleartextStoragePreferencesSink {
38+
UserDefaultsStore() {
39+
exists(CallExpr call |
40+
call.getStaticTarget().(MethodDecl).hasQualifiedName("UserDefaults", "set(_:forKey:)") and
41+
call.getArgument(0).getExpr() = this.asExpr()
42+
)
43+
}
44+
45+
override string getStoreName() { result = "the user defaults database" }
46+
}
47+
48+
/** The `DataFlow::Node` of an expression that gets written to the iCloud-backed NSUbiquitousKeyValueStore */
49+
private class NSUbiquitousKeyValueStore extends CleartextStoragePreferencesSink {
50+
NSUbiquitousKeyValueStore() {
51+
exists(CallExpr call |
52+
call.getStaticTarget()
53+
.(MethodDecl)
54+
.hasQualifiedName("NSUbiquitousKeyValueStore", "set(_:forKey:)") and
55+
call.getArgument(0).getExpr() = this.asExpr()
56+
)
57+
}
58+
59+
override string getStoreName() { result = "iCloud" }
60+
}
61+
62+
/**
63+
* A more complicated case, this is a macOS-only way of writing to
64+
* NSUserDefaults by modifying the `NSUserDefaultsController.values: Any`
65+
* object via reflection (`perform(Selector)`) or the `NSKeyValueCoding`,
66+
* `NSKeyValueBindingCreation` APIs. (TODO)
67+
*/
68+
private class NSUserDefaultsControllerStore extends CleartextStoragePreferencesSink {
69+
NSUserDefaultsControllerStore() { none() }
70+
71+
override string getStoreName() { result = "the user defaults database" }
72+
}
73+
74+
/**
75+
* An encryption sanitizer for cleartext preferences storage vulnerabilities.
76+
*/
77+
private class CleartextStoragePreferencesEncryptionSanitizer extends CleartextStoragePreferencesSanitizer {
78+
CleartextStoragePreferencesEncryptionSanitizer() { this.asExpr() instanceof EncryptedExpr }
79+
}
80+
81+
/**
82+
* A sink defined in a CSV model.
83+
*/
84+
private class DefaultCleartextStoragePreferencesSink extends CleartextStoragePreferencesSink {
85+
DefaultCleartextStoragePreferencesSink() { sinkNode(this, "preferences-store") }
86+
87+
override string getStoreName() { result = "a preferences store" }
88+
}

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about cleartext
3+
* preferences storage vulnerabilities.
4+
*/
5+
6+
import swift
7+
import codeql.swift.security.SensitiveExprs
8+
import codeql.swift.dataflow.DataFlow
9+
import codeql.swift.dataflow.TaintTracking
10+
import codeql.swift.security.CleartextStoragePreferencesExtensions
11+
12+
/**
13+
* A taint configuration from sensitive information to expressions that are
14+
* stored as preferences.
15+
*/
16+
class CleartextStorageConfig extends TaintTracking::Configuration {
17+
CleartextStorageConfig() { this = "CleartextStorageConfig" }
18+
19+
override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr }
20+
21+
override predicate isSink(DataFlow::Node node) { node instanceof CleartextStoragePreferencesSink }
22+
23+
override predicate isSanitizer(DataFlow::Node sanitizer) {
24+
sanitizer instanceof CleartextStoragePreferencesSanitizer
25+
}
26+
27+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
28+
any(CleartextStoragePreferencesAdditionalTaintStep s).step(nodeFrom, nodeTo)
29+
}
30+
31+
override predicate isSanitizerIn(DataFlow::Node node) {
32+
// make sources barriers so that we only report the closest instance
33+
this.isSource(node)
34+
}
35+
}

0 commit comments

Comments
 (0)