Skip to content

Commit 2622de9

Browse files
geoffw0geoffw0
committed
Swift: Improve Core Data coverage.
1 parent 82f9903 commit 2622de9

File tree

4 files changed

+79
-13
lines changed

4 files changed

+79
-13
lines changed

‎swift/ql/src/queries/Security/CWE-311/CleartextStorageDatabase.ql

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,23 @@ abstract class Stored extends DataFlow::Node { }
2727
*/
2828
class CoreDataStore extends Stored {
2929
CoreDataStore() {
30-
// values written into Core Data objects are a sink
30+
// values written into Core Data objects through `set*Value` methods are a sink.
3131
exists(CallExpr call |
3232
call.getStaticTarget()
3333
.(MethodDecl)
3434
.hasQualifiedName("NSManagedObject",
3535
["setValue(_:forKey:)", "setPrimitiveValue(_:forKey:)"]) and
3636
call.getArgument(0).getExpr() = this.asExpr()
37+
) or
38+
// any write into a class derived from `NSManagedObject` is a sink. For
39+
// example in `coreDataObj.data = sensitive` the post-update node corresponding
40+
// with `coreDataObj.data` is a sink.
41+
// (ideally this would be only members with the `@NSManaged` attribute)
42+
exists(ClassOrStructDecl cd, Expr e |
43+
cd.getABaseTypeDecl*().getName() = "NSManagedObject" and
44+
this.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = e and
45+
e.getFullyConverted().getType() = cd.getType() and
46+
not e.(DeclRefExpr).getDecl() instanceof SelfParamDecl
3747
)
3848
}
3949
}
@@ -78,12 +88,12 @@ class CleartextStorageConfig extends TaintTracking::Configuration {
7888
}
7989

8090
override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
81-
// flow out from fields of a `RealmSwiftObject` at the sink, for example in
82-
// `realmObj.data = sensitive`.
91+
// flow out from fields of an `NSManagedObject` or `RealmSwiftObject` at the sink,
92+
// for example in `realmObj.data = sensitive`.
8393
isSink(node) and
8494
exists(ClassOrStructDecl cd |
8595
c.getAReadContent().(DataFlow::Content::FieldContent).getField() = cd.getAMember() and
86-
cd.getABaseTypeDecl*().getName() = "RealmSwiftObject"
96+
cd.getABaseTypeDecl*().getName() = ["NSManagedObject", "RealmSwiftObject"]
8797
)
8898
or
8999
// any default implicit reads

‎swift/ql/test/query-tests/Security/CWE-311/CleartextStorageDatabase.expected

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,29 @@
11
edges
22
| file://:0:0:0:0 | value : | file://:0:0:0:0 | [post] self [data] : |
3+
| file://:0:0:0:0 | value : | file://:0:0:0:0 | [post] self [notStoredBankAccountNumber] : |
4+
| testCoreData2.swift:23:13:23:13 | value : | file://:0:0:0:0 | value : |
5+
| testCoreData2.swift:37:2:37:2 | [post] obj [myValue] : | testCoreData2.swift:37:2:37:2 | [post] obj |
6+
| testCoreData2.swift:37:16:37:16 | bankAccountNo : | testCoreData2.swift:37:2:37:2 | [post] obj [myValue] : |
7+
| testCoreData2.swift:39:2:39:2 | [post] obj [myBankAccountNumber] : | testCoreData2.swift:39:2:39:2 | [post] obj |
8+
| testCoreData2.swift:39:28:39:28 | bankAccountNo : | testCoreData2.swift:39:2:39:2 | [post] obj [myBankAccountNumber] : |
9+
| testCoreData2.swift:43:2:43:2 | [post] obj [notStoredBankAccountNumber] : | testCoreData2.swift:43:2:43:2 | [post] obj |
10+
| testCoreData2.swift:43:35:43:35 | bankAccountNo : | testCoreData2.swift:23:13:23:13 | value : |
11+
| testCoreData2.swift:43:35:43:35 | bankAccountNo : | testCoreData2.swift:43:2:43:2 | [post] obj [notStoredBankAccountNumber] : |
12+
| testCoreData2.swift:46:2:46:10 | [post] ...? [myValue] : | testCoreData2.swift:46:2:46:10 | [post] ...? |
13+
| testCoreData2.swift:46:22:46:22 | bankAccountNo : | testCoreData2.swift:46:2:46:10 | [post] ...? [myValue] : |
14+
| testCoreData2.swift:48:2:48:10 | [post] ...? [myBankAccountNumber] : | testCoreData2.swift:48:2:48:10 | [post] ...? |
15+
| testCoreData2.swift:48:34:48:34 | bankAccountNo : | testCoreData2.swift:48:2:48:10 | [post] ...? [myBankAccountNumber] : |
16+
| testCoreData2.swift:52:2:52:10 | [post] ...? [notStoredBankAccountNumber] : | testCoreData2.swift:52:2:52:10 | [post] ...? |
17+
| testCoreData2.swift:52:41:52:41 | bankAccountNo : | testCoreData2.swift:23:13:23:13 | value : |
18+
| testCoreData2.swift:52:41:52:41 | bankAccountNo : | testCoreData2.swift:52:2:52:10 | [post] ...? [notStoredBankAccountNumber] : |
19+
| testCoreData2.swift:57:3:57:3 | [post] obj [myBankAccountNumber] : | testCoreData2.swift:57:3:57:3 | [post] obj |
20+
| testCoreData2.swift:57:29:57:29 | bankAccountNo : | testCoreData2.swift:57:3:57:3 | [post] obj [myBankAccountNumber] : |
321
| testCoreData.swift:18:19:18:26 | value : | testCoreData.swift:19:12:19:12 | value |
422
| testCoreData.swift:31:3:31:3 | newValue : | testCoreData.swift:32:13:32:13 | newValue |
523
| testCoreData.swift:61:25:61:25 | password : | testCoreData.swift:18:19:18:26 | value : |
24+
| testCoreData.swift:64:2:64:2 | [post] obj [myValue] : | testCoreData.swift:64:2:64:2 | [post] obj |
625
| testCoreData.swift:64:16:64:16 | password : | testCoreData.swift:31:3:31:3 | newValue : |
26+
| testCoreData.swift:64:16:64:16 | password : | testCoreData.swift:64:2:64:2 | [post] obj [myValue] : |
727
| testCoreData.swift:77:24:77:24 | x : | testCoreData.swift:78:15:78:15 | x |
828
| testCoreData.swift:80:10:80:22 | call to getPassword() : | testCoreData.swift:81:15:81:15 | y |
929
| testCoreData.swift:91:10:91:10 | passwd : | testCoreData.swift:95:15:95:15 | x |
@@ -24,7 +44,31 @@ edges
2444
| testRealm.swift:59:11:59:11 | myPassword : | testRealm.swift:59:2:59:2 | [post] g [data] : |
2545
nodes
2646
| file://:0:0:0:0 | [post] self [data] : | semmle.label | [post] self [data] : |
47+
| file://:0:0:0:0 | [post] self [notStoredBankAccountNumber] : | semmle.label | [post] self [notStoredBankAccountNumber] : |
2748
| file://:0:0:0:0 | value : | semmle.label | value : |
49+
| file://:0:0:0:0 | value : | semmle.label | value : |
50+
| testCoreData2.swift:23:13:23:13 | value : | semmle.label | value : |
51+
| testCoreData2.swift:37:2:37:2 | [post] obj | semmle.label | [post] obj |
52+
| testCoreData2.swift:37:2:37:2 | [post] obj [myValue] : | semmle.label | [post] obj [myValue] : |
53+
| testCoreData2.swift:37:16:37:16 | bankAccountNo : | semmle.label | bankAccountNo : |
54+
| testCoreData2.swift:39:2:39:2 | [post] obj | semmle.label | [post] obj |
55+
| testCoreData2.swift:39:2:39:2 | [post] obj [myBankAccountNumber] : | semmle.label | [post] obj [myBankAccountNumber] : |
56+
| testCoreData2.swift:39:28:39:28 | bankAccountNo : | semmle.label | bankAccountNo : |
57+
| testCoreData2.swift:43:2:43:2 | [post] obj | semmle.label | [post] obj |
58+
| testCoreData2.swift:43:2:43:2 | [post] obj [notStoredBankAccountNumber] : | semmle.label | [post] obj [notStoredBankAccountNumber] : |
59+
| testCoreData2.swift:43:35:43:35 | bankAccountNo : | semmle.label | bankAccountNo : |
60+
| testCoreData2.swift:46:2:46:10 | [post] ...? | semmle.label | [post] ...? |
61+
| testCoreData2.swift:46:2:46:10 | [post] ...? [myValue] : | semmle.label | [post] ...? [myValue] : |
62+
| testCoreData2.swift:46:22:46:22 | bankAccountNo : | semmle.label | bankAccountNo : |
63+
| testCoreData2.swift:48:2:48:10 | [post] ...? | semmle.label | [post] ...? |
64+
| testCoreData2.swift:48:2:48:10 | [post] ...? [myBankAccountNumber] : | semmle.label | [post] ...? [myBankAccountNumber] : |
65+
| testCoreData2.swift:48:34:48:34 | bankAccountNo : | semmle.label | bankAccountNo : |
66+
| testCoreData2.swift:52:2:52:10 | [post] ...? | semmle.label | [post] ...? |
67+
| testCoreData2.swift:52:2:52:10 | [post] ...? [notStoredBankAccountNumber] : | semmle.label | [post] ...? [notStoredBankAccountNumber] : |
68+
| testCoreData2.swift:52:41:52:41 | bankAccountNo : | semmle.label | bankAccountNo : |
69+
| testCoreData2.swift:57:3:57:3 | [post] obj | semmle.label | [post] obj |
70+
| testCoreData2.swift:57:3:57:3 | [post] obj [myBankAccountNumber] : | semmle.label | [post] obj [myBankAccountNumber] : |
71+
| testCoreData2.swift:57:29:57:29 | bankAccountNo : | semmle.label | bankAccountNo : |
2872
| testCoreData.swift:18:19:18:26 | value : | semmle.label | value : |
2973
| testCoreData.swift:19:12:19:12 | value | semmle.label | value |
3074
| testCoreData.swift:31:3:31:3 | newValue : | semmle.label | newValue : |
@@ -33,6 +77,8 @@ nodes
3377
| testCoreData.swift:51:24:51:24 | password | semmle.label | password |
3478
| testCoreData.swift:58:15:58:15 | password | semmle.label | password |
3579
| testCoreData.swift:61:25:61:25 | password : | semmle.label | password : |
80+
| testCoreData.swift:64:2:64:2 | [post] obj | semmle.label | [post] obj |
81+
| testCoreData.swift:64:2:64:2 | [post] obj [myValue] : | semmle.label | [post] obj [myValue] : |
3682
| testCoreData.swift:64:16:64:16 | password : | semmle.label | password : |
3783
| testCoreData.swift:77:24:77:24 | x : | semmle.label | x : |
3884
| testCoreData.swift:78:15:78:15 | x | semmle.label | x |
@@ -59,16 +105,26 @@ nodes
59105
| testRealm.swift:59:2:59:2 | [post] g [data] : | semmle.label | [post] g [data] : |
60106
| testRealm.swift:59:11:59:11 | myPassword : | semmle.label | myPassword : |
61107
subpaths
108+
| testCoreData2.swift:43:35:43:35 | bankAccountNo : | testCoreData2.swift:23:13:23:13 | value : | file://:0:0:0:0 | [post] self [notStoredBankAccountNumber] : | testCoreData2.swift:43:2:43:2 | [post] obj [notStoredBankAccountNumber] : |
109+
| testCoreData2.swift:52:41:52:41 | bankAccountNo : | testCoreData2.swift:23:13:23:13 | value : | file://:0:0:0:0 | [post] self [notStoredBankAccountNumber] : | testCoreData2.swift:52:2:52:10 | [post] ...? [notStoredBankAccountNumber] : |
62110
| testRealm.swift:34:11:34:11 | myPassword : | testRealm.swift:16:6:16:6 | value : | file://:0:0:0:0 | [post] self [data] : | testRealm.swift:34:2:34:2 | [post] a [data] : |
63111
| testRealm.swift:42:11:42:11 | myPassword : | testRealm.swift:16:6:16:6 | value : | file://:0:0:0:0 | [post] self [data] : | testRealm.swift:42:2:42:2 | [post] c [data] : |
64112
| testRealm.swift:52:12:52:12 | myPassword : | testRealm.swift:16:6:16:6 | value : | file://:0:0:0:0 | [post] self [data] : | testRealm.swift:52:2:52:3 | [post] ...! [data] : |
65113
| testRealm.swift:59:11:59:11 | myPassword : | testRealm.swift:16:6:16:6 | value : | file://:0:0:0:0 | [post] self [data] : | testRealm.swift:59:2:59:2 | [post] g [data] : |
66114
#select
115+
| testCoreData2.swift:37:2:37:2 | obj | testCoreData2.swift:37:16:37:16 | bankAccountNo : | testCoreData2.swift:37:2:37:2 | [post] obj | This operation stores '[post] obj' in a database. It may contain unencrypted sensitive data from $@. | testCoreData2.swift:37:16:37:16 | bankAccountNo : | bankAccountNo |
116+
| testCoreData2.swift:39:2:39:2 | obj | testCoreData2.swift:39:28:39:28 | bankAccountNo : | testCoreData2.swift:39:2:39:2 | [post] obj | This operation stores '[post] obj' in a database. It may contain unencrypted sensitive data from $@. | testCoreData2.swift:39:28:39:28 | bankAccountNo : | bankAccountNo |
117+
| testCoreData2.swift:43:2:43:2 | obj | testCoreData2.swift:43:35:43:35 | bankAccountNo : | testCoreData2.swift:43:2:43:2 | [post] obj | This operation stores '[post] obj' in a database. It may contain unencrypted sensitive data from $@. | testCoreData2.swift:43:35:43:35 | bankAccountNo : | bankAccountNo |
118+
| testCoreData2.swift:46:2:46:10 | ...? | testCoreData2.swift:46:22:46:22 | bankAccountNo : | testCoreData2.swift:46:2:46:10 | [post] ...? | This operation stores '[post] ...?' in a database. It may contain unencrypted sensitive data from $@. | testCoreData2.swift:46:22:46:22 | bankAccountNo : | bankAccountNo |
119+
| testCoreData2.swift:48:2:48:10 | ...? | testCoreData2.swift:48:34:48:34 | bankAccountNo : | testCoreData2.swift:48:2:48:10 | [post] ...? | This operation stores '[post] ...?' in a database. It may contain unencrypted sensitive data from $@. | testCoreData2.swift:48:34:48:34 | bankAccountNo : | bankAccountNo |
120+
| testCoreData2.swift:52:2:52:10 | ...? | testCoreData2.swift:52:41:52:41 | bankAccountNo : | testCoreData2.swift:52:2:52:10 | [post] ...? | This operation stores '[post] ...?' in a database. It may contain unencrypted sensitive data from $@. | testCoreData2.swift:52:41:52:41 | bankAccountNo : | bankAccountNo |
121+
| testCoreData2.swift:57:3:57:3 | obj | testCoreData2.swift:57:29:57:29 | bankAccountNo : | testCoreData2.swift:57:3:57:3 | [post] obj | This operation stores '[post] obj' in a database. It may contain unencrypted sensitive data from $@. | testCoreData2.swift:57:29:57:29 | bankAccountNo : | bankAccountNo |
67122
| testCoreData.swift:19:12:19:12 | value | testCoreData.swift:61:25:61:25 | password : | testCoreData.swift:19:12:19:12 | value | This operation stores 'value' in a database. It may contain unencrypted sensitive data from $@. | testCoreData.swift:61:25:61:25 | password : | password |
68123
| testCoreData.swift:32:13:32:13 | newValue | testCoreData.swift:64:16:64:16 | password : | testCoreData.swift:32:13:32:13 | newValue | This operation stores 'newValue' in a database. It may contain unencrypted sensitive data from $@. | testCoreData.swift:64:16:64:16 | password : | password |
69124
| testCoreData.swift:48:15:48:15 | password | testCoreData.swift:48:15:48:15 | password | testCoreData.swift:48:15:48:15 | password | This operation stores 'password' in a database. It may contain unencrypted sensitive data from $@. | testCoreData.swift:48:15:48:15 | password | password |
70125
| testCoreData.swift:51:24:51:24 | password | testCoreData.swift:51:24:51:24 | password | testCoreData.swift:51:24:51:24 | password | This operation stores 'password' in a database. It may contain unencrypted sensitive data from $@. | testCoreData.swift:51:24:51:24 | password | password |
71126
| testCoreData.swift:58:15:58:15 | password | testCoreData.swift:58:15:58:15 | password | testCoreData.swift:58:15:58:15 | password | This operation stores 'password' in a database. It may contain unencrypted sensitive data from $@. | testCoreData.swift:58:15:58:15 | password | password |
127+
| testCoreData.swift:64:2:64:2 | obj | testCoreData.swift:64:16:64:16 | password : | testCoreData.swift:64:2:64:2 | [post] obj | This operation stores '[post] obj' in a database. It may contain unencrypted sensitive data from $@. | testCoreData.swift:64:16:64:16 | password : | password |
72128
| testCoreData.swift:78:15:78:15 | x | testCoreData.swift:77:24:77:24 | x : | testCoreData.swift:78:15:78:15 | x | This operation stores 'x' in a database. It may contain unencrypted sensitive data from $@. | testCoreData.swift:77:24:77:24 | x : | x |
73129
| testCoreData.swift:81:15:81:15 | y | testCoreData.swift:80:10:80:22 | call to getPassword() : | testCoreData.swift:81:15:81:15 | y | This operation stores 'y' in a database. It may contain unencrypted sensitive data from $@. | testCoreData.swift:80:10:80:22 | call to getPassword() : | call to getPassword() |
74130
| testCoreData.swift:85:15:85:17 | .password | testCoreData.swift:85:15:85:17 | .password | testCoreData.swift:85:15:85:17 | .password | This operation stores '.password' in a database. It may contain unencrypted sensitive data from $@. | testCoreData.swift:85:15:85:17 | .password | .password |

‎swift/ql/test/query-tests/Security/CWE-311/testCoreData.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class MyManagedObject : NSManagedObject
2929
}
3030
}
3131
set {
32-
setValue(newValue, forKey: "myKey")
32+
setValue(newValue, forKey: "myKey") // [additional result reported here]
3333
}
3434
}
3535
}
@@ -61,7 +61,7 @@ func test2(obj : MyManagedObject, password : String, password_file : String) {
6161
obj.setIndirect(value: password) // BAD [reported on line 19]
6262
obj.setIndirect(value: password_file) // GOOD (not sensitive)
6363

64-
obj.myValue = password // BAD [reported on line 32]
64+
obj.myValue = password // BAD [also reported on line 32]
6565
obj.myValue = password_file // GOOD (not sensitive)
6666
}
6767

‎swift/ql/test/query-tests/Security/CWE-311/testCoreData2.swift

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,27 +34,27 @@ func testCoreData2_1(obj: MyManagedObject2, maybeObj: MyManagedObject2?, value:
3434
{
3535
// @NSManaged fields of an NSManagedObject...
3636
obj.myValue = value // GOOD (not sensitive)
37-
obj.myValue = bankAccountNo // BAD [NOT DETECTED]
37+
obj.myValue = bankAccountNo // BAD
3838
obj.myBankAccountNumber = value // BAD [NOT DETECTED]
39-
obj.myBankAccountNumber = bankAccountNo // BAD [NOT DETECTED]
39+
obj.myBankAccountNumber = bankAccountNo // BAD
4040
obj.myBankAccountNumber2 = value // BAD [NOT DETECTED]
4141
obj.myBankAccountNumber2 = bankAccountNo // BAD [NOT DETECTED]
4242
obj.notStoredBankAccountNumber = value // GOOD (not stored in the database)
43-
obj.notStoredBankAccountNumber = bankAccountNo // GOOD (not stored in the datbase)
43+
obj.notStoredBankAccountNumber = bankAccountNo // GOOD (not stored in the datbase) [FALSE POSITIVE]
4444

4545
maybeObj?.myValue = value // GOOD (not sensitive)
46-
maybeObj?.myValue = bankAccountNo // BAD [NOT DETECTED]
46+
maybeObj?.myValue = bankAccountNo // BAD
4747
maybeObj?.myBankAccountNumber = value // BAD [NOT DETECTED]
48-
maybeObj?.myBankAccountNumber = bankAccountNo // BAD [NOT DETECTED]
48+
maybeObj?.myBankAccountNumber = bankAccountNo // BAD
4949
maybeObj?.myBankAccountNumber2 = value // BAD [NOT DETECTED]
5050
maybeObj?.myBankAccountNumber2 = bankAccountNo // BAD [NOT DETECTED]
5151
maybeObj?.notStoredBankAccountNumber = value // GOOD (not stored in the database)
52-
maybeObj?.notStoredBankAccountNumber = bankAccountNo // GOOD (not stored in the datbase)
52+
maybeObj?.notStoredBankAccountNumber = bankAccountNo // GOOD (not stored in the datbase) [FALSE POSITIVE]
5353
}
5454

5555
class testCoreData2_2 {
5656
func myFunc(obj: MyManagedObject2, bankAccountNo: Int) {
57-
obj.myBankAccountNumber = bankAccountNo // BAD [NOT DETECTED]
57+
obj.myBankAccountNumber = bankAccountNo // BAD
5858

5959
if #available(iOS 10.0, *) {
6060
obj.myBankAccountNumber = bankAccountNo // BAD [NOT DETECTED]

0 commit comments

Comments
 (0)