Skip to content

Commit 4ce8d95

Browse files
geoffw0geoffw0
authored
Merge pull request #14698 from geoffw0/realmswift
Swift: Fix an issue with Realm sinks for swift/cleartext-storage-database
2 parents 4e70e67 + e4f6b1a commit 4ce8d95

File tree

6 files changed

+66
-5
lines changed

6 files changed

+66
-5
lines changed

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

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,26 @@ private class CoreDataStore extends CleartextStorageDatabaseSink {
5757
}
5858
}
5959

60+
/**
61+
* The Realm database `RealmSwiftObject` type. Also matches the Realm `Object`
62+
* type, which may or may not be a type alias for `RealmSwiftObject`.
63+
*/
64+
class RealmSwiftObject extends Type {
65+
RealmSwiftObject() {
66+
this.getName() = "RealmSwiftObject"
67+
or
68+
this.getName() = "Object" and
69+
this.(NominalType).getDeclaration().getModule().getName() = "RealmSwift"
70+
}
71+
}
72+
73+
/**
74+
* A class that inherits from `RealmSwiftObject`.
75+
*/
76+
class RealmSwiftObjectType extends Type {
77+
RealmSwiftObjectType() { this.getUnderlyingType().getABaseType*() instanceof RealmSwiftObject }
78+
}
79+
6080
/**
6181
* A `DataFlow::Node` that is an expression stored with the Realm database
6282
* library.
@@ -66,10 +86,9 @@ private class RealmStore extends CleartextStorageDatabaseSink instanceof DataFlo
6686
// any write into a class derived from `RealmSwiftObject` is a sink. For
6787
// example in `realmObj.data = sensitive` the post-update node corresponding
6888
// with `realmObj.data` is a sink.
69-
exists(NominalType t, Expr e |
70-
t.getUnderlyingType().getABaseType*().getName() = "RealmSwiftObject" and
89+
exists(Expr e |
7190
this.getPreUpdateNode().asExpr() = e and
72-
e.getFullyConverted().getType() = t and
91+
e.getFullyConverted().getType() instanceof RealmSwiftObjectType and
7392
not e.(DeclRefExpr).getDecl() instanceof SelfParamDecl
7493
)
7594
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,10 @@ module CleartextStorageDatabaseConfig implements DataFlow::ConfigSig {
3434
// for example in `realmObj.data = sensitive`.
3535
isSink(node) and
3636
exists(NominalTypeDecl d, Decl cx |
37-
d.getType().getUnderlyingType().getABaseType*().getName() =
38-
["NSManagedObject", "RealmSwiftObject"] and
37+
(
38+
d.getType().getUnderlyingType().getABaseType*().getName() = "NSManagedObject" or
39+
d.getType() instanceof RealmSwiftObjectType
40+
) and
3941
cx.asNominalTypeDecl() = d and
4042
c.getAReadContent().(DataFlow::Content::FieldContent).getField() = cx.getAMember()
4143
)

‎swift/ql/src/change-notes/2023-11-06-realm.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
5+
* Fixed an issue where some Realm database sinks were not being recognized for the `swift/cleartext-storage-database` query.

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ edges
7373
| file://:0:0:0:0 | self | file://:0:0:0:0 | .value2 |
7474
| file://:0:0:0:0 | self [value] | file://:0:0:0:0 | .value |
7575
| file://:0:0:0:0 | value | file://:0:0:0:0 | [post] self [data] |
76+
| file://:0:0:0:0 | value | file://:0:0:0:0 | [post] self [data] |
7677
| file://:0:0:0:0 | value | file://:0:0:0:0 | [post] self [notStoredBankAccountNumber] |
7778
| file://:0:0:0:0 | value | file://:0:0:0:0 | [post] self [password] |
7879
| file://:0:0:0:0 | value | file://:0:0:0:0 | [post] self [value] |
@@ -288,6 +289,10 @@ edges
288289
| testGRDB.swift:210:85:210:85 | password | testGRDB.swift:210:84:210:93 | [...] [Collection element] |
289290
| testGRDB.swift:212:98:212:107 | [...] [Collection element] | testGRDB.swift:212:98:212:107 | [...] |
290291
| testGRDB.swift:212:99:212:99 | password | testGRDB.swift:212:98:212:107 | [...] [Collection element] |
292+
| testRealm2.swift:13:6:13:6 | value | file://:0:0:0:0 | value |
293+
| testRealm2.swift:18:2:18:2 | [post] o [data] | testRealm2.swift:18:2:18:2 | [post] o |
294+
| testRealm2.swift:18:11:18:11 | myPassword | testRealm2.swift:13:6:13:6 | value |
295+
| testRealm2.swift:18:11:18:11 | myPassword | testRealm2.swift:18:2:18:2 | [post] o [data] |
291296
| testRealm.swift:27:6:27:6 | value | file://:0:0:0:0 | value |
292297
| testRealm.swift:34:6:34:6 | value | file://:0:0:0:0 | value |
293298
| testRealm.swift:41:2:41:2 | [post] a [data] | testRealm.swift:41:2:41:2 | [post] a |
@@ -405,6 +410,7 @@ nodes
405410
| file://:0:0:0:0 | .value | semmle.label | .value |
406411
| file://:0:0:0:0 | .value2 | semmle.label | .value2 |
407412
| file://:0:0:0:0 | [post] self [data] | semmle.label | [post] self [data] |
413+
| file://:0:0:0:0 | [post] self [data] | semmle.label | [post] self [data] |
408414
| file://:0:0:0:0 | [post] self [notStoredBankAccountNumber] | semmle.label | [post] self [notStoredBankAccountNumber] |
409415
| file://:0:0:0:0 | [post] self [password] | semmle.label | [post] self [password] |
410416
| file://:0:0:0:0 | [post] self [value] | semmle.label | [post] self [value] |
@@ -415,6 +421,7 @@ nodes
415421
| file://:0:0:0:0 | value | semmle.label | value |
416422
| file://:0:0:0:0 | value | semmle.label | value |
417423
| file://:0:0:0:0 | value | semmle.label | value |
424+
| file://:0:0:0:0 | value | semmle.label | value |
418425
| sqlite3_c_api.swift:42:69:42:69 | medicalNotes | semmle.label | medicalNotes |
419426
| sqlite3_c_api.swift:43:49:43:49 | medicalNotes | semmle.label | medicalNotes |
420427
| sqlite3_c_api.swift:46:27:46:27 | insertQuery | semmle.label | insertQuery |
@@ -701,6 +708,10 @@ nodes
701708
| testGRDB.swift:212:98:212:107 | [...] | semmle.label | [...] |
702709
| testGRDB.swift:212:98:212:107 | [...] [Collection element] | semmle.label | [...] [Collection element] |
703710
| testGRDB.swift:212:99:212:99 | password | semmle.label | password |
711+
| testRealm2.swift:13:6:13:6 | value | semmle.label | value |
712+
| testRealm2.swift:18:2:18:2 | [post] o | semmle.label | [post] o |
713+
| testRealm2.swift:18:2:18:2 | [post] o [data] | semmle.label | [post] o [data] |
714+
| testRealm2.swift:18:11:18:11 | myPassword | semmle.label | myPassword |
704715
| testRealm.swift:27:6:27:6 | value | semmle.label | value |
705716
| testRealm.swift:34:6:34:6 | value | semmle.label | value |
706717
| testRealm.swift:41:2:41:2 | [post] a | semmle.label | [post] a |
@@ -733,6 +744,7 @@ subpaths
733744
| testCoreData2.swift:98:18:98:18 | d [value] | testCoreData2.swift:70:9:70:9 | self [value] | file://:0:0:0:0 | .value | testCoreData2.swift:98:18:98:20 | .value |
734745
| testCoreData2.swift:104:18:104:18 | e | testCoreData2.swift:70:9:70:9 | self | file://:0:0:0:0 | .value | testCoreData2.swift:104:18:104:20 | .value |
735746
| testCoreData2.swift:105:18:105:18 | e | testCoreData2.swift:71:9:71:9 | self | file://:0:0:0:0 | .value2 | testCoreData2.swift:105:18:105:20 | .value2 |
747+
| testRealm2.swift:18:11:18:11 | myPassword | testRealm2.swift:13:6:13:6 | value | file://:0:0:0:0 | [post] self [data] | testRealm2.swift:18:2:18:2 | [post] o [data] |
736748
| testRealm.swift:41:11:41:11 | myPassword | testRealm.swift:27:6:27:6 | value | file://:0:0:0:0 | [post] self [data] | testRealm.swift:41:2:41:2 | [post] a [data] |
737749
| testRealm.swift:49:11:49:11 | myPassword | testRealm.swift:27:6:27:6 | value | file://:0:0:0:0 | [post] self [data] | testRealm.swift:49:2:49:2 | [post] c [data] |
738750
| testRealm.swift:59:12:59:12 | myPassword | testRealm.swift:27:6:27:6 | value | file://:0:0:0:0 | [post] self [data] | testRealm.swift:59:2:59:3 | [post] ...! [data] |
@@ -864,6 +876,7 @@ subpaths
864876
| testGRDB.swift:208:80:208:89 | [...] | testGRDB.swift:208:81:208:81 | password | testGRDB.swift:208:80:208:89 | [...] | This operation stores '[...]' in a database. It may contain unencrypted sensitive data from $@. | testGRDB.swift:208:81:208:81 | password | password |
865877
| testGRDB.swift:210:84:210:93 | [...] | testGRDB.swift:210:85:210:85 | password | testGRDB.swift:210:84:210:93 | [...] | This operation stores '[...]' in a database. It may contain unencrypted sensitive data from $@. | testGRDB.swift:210:85:210:85 | password | password |
866878
| testGRDB.swift:212:98:212:107 | [...] | testGRDB.swift:212:99:212:99 | password | testGRDB.swift:212:98:212:107 | [...] | This operation stores '[...]' in a database. It may contain unencrypted sensitive data from $@. | testGRDB.swift:212:99:212:99 | password | password |
879+
| testRealm2.swift:18:2:18:2 | o | testRealm2.swift:18:11:18:11 | myPassword | testRealm2.swift:18:2:18:2 | [post] o | This operation stores 'o' in a database. It may contain unencrypted sensitive data from $@. | testRealm2.swift:18:11:18:11 | myPassword | myPassword |
867880
| testRealm.swift:41:2:41:2 | a | testRealm.swift:41:11:41:11 | myPassword | testRealm.swift:41:2:41:2 | [post] a | This operation stores 'a' in a database. It may contain unencrypted sensitive data from $@. | testRealm.swift:41:11:41:11 | myPassword | myPassword |
868881
| testRealm.swift:49:2:49:2 | c | testRealm.swift:49:11:49:11 | myPassword | testRealm.swift:49:2:49:2 | [post] c | This operation stores 'c' in a database. It may contain unencrypted sensitive data from $@. | testRealm.swift:49:11:49:11 | myPassword | myPassword |
869882
| testRealm.swift:59:2:59:3 | ...! | testRealm.swift:59:12:59:12 | myPassword | testRealm.swift:59:2:59:3 | [post] ...! | This operation stores '...!' in a database. It may contain unencrypted sensitive data from $@. | testRealm.swift:59:12:59:12 | myPassword | myPassword |

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@
138138
| testGRDB.swift:208:81:208:81 | password | label:password, type:credential |
139139
| testGRDB.swift:210:85:210:85 | password | label:password, type:credential |
140140
| testGRDB.swift:212:99:212:99 | password | label:password, type:credential |
141+
| testRealm2.swift:18:11:18:11 | myPassword | label:myPassword, type:credential |
141142
| testRealm.swift:31:20:31:20 | .password | label:password, type:credential |
142143
| testRealm.swift:41:11:41:11 | myPassword | label:myPassword, type:credential |
143144
| testRealm.swift:49:11:49:11 | myPassword | label:myPassword, type:credential |

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
//codeql-extractor-options: -module-name RealmSwift
2+
3+
// --- stubs ---
4+
5+
class Object {
6+
}
7+
8+
// --- tests ---
9+
10+
class MyRealmSwiftObject3 : Object {
11+
override init() { data = "" }
12+
13+
var data: String
14+
}
15+
16+
func test1(o: MyRealmSwiftObject3, myHarmless: String, myPassword : String) {
17+
// ...
18+
o.data = myPassword // BAD
19+
o.data = myHarmless
20+
// ...
21+
}

0 commit comments

Comments
 (0)