Skip to content

Commit 2543f3e

Browse files
geoffw0geoffw0
committed
Swift: Make + a barrier for swift/constant-salt.
1 parent c8438c3 commit 2543f3e

File tree

3 files changed

+19
-38
lines changed

3 files changed

+19
-38
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,17 @@ private class RnCryptorSaltSink extends ConstantSaltSink {
6969
private class DefaultSaltSink extends ConstantSaltSink {
7070
DefaultSaltSink() { sinkNode(this, "encryption-salt") }
7171
}
72+
73+
/**
74+
* A barrier for appending, since appending strings to a constant string
75+
* may (and often does) result in a non-constant string.
76+
*
77+
* Ideally, these would not be a barrier when there is flow to *all*
78+
* inputs.
79+
*/
80+
private class AppendConstantSaltBarrier extends ConstantSaltBarrier {
81+
AppendConstantSaltBarrier() {
82+
this.asExpr() = any(AddExpr ae).getAnOperand() or
83+
this.asExpr() = any(AssignAddExpr aae).getAnOperand()
84+
}
85+
}

‎swift/ql/test/query-tests/Security/CWE-760/ConstantSalt.expected

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,6 @@ edges
1111
| rncryptor.swift:60:24:60:30 | call to Data.init(_:) | rncryptor.swift:76:152:76:152 | myConstantSalt2 | provenance | |
1212
| rncryptor.swift:60:24:60:30 | call to Data.init(_:) | rncryptor.swift:79:160:79:160 | myConstantSalt2 | provenance | |
1313
| rncryptor.swift:60:29:60:29 | 0 | rncryptor.swift:60:24:60:30 | call to Data.init(_:) | provenance | |
14-
| rncryptor.swift:83:62:83:62 | 123 | rncryptor.swift:83:62:83:87 | ... .+(_:_:) ... | provenance | |
15-
| rncryptor.swift:83:62:83:87 | ... .+(_:_:) ... | rncryptor.swift:83:57:83:88 | call to Data.init(_:) | provenance | |
16-
| rncryptor.swift:84:62:84:83 | ... .+(_:_:) ... | rncryptor.swift:84:57:84:88 | call to Data.init(_:) | provenance | |
17-
| rncryptor.swift:84:83:84:83 | abc | rncryptor.swift:84:62:84:83 | ... .+(_:_:) ... | provenance | |
18-
| rncryptor.swift:85:62:85:62 | 123 | rncryptor.swift:85:62:85:70 | ... .+(_:_:) ... | provenance | |
19-
| rncryptor.swift:85:62:85:70 | ... .+(_:_:) ... | rncryptor.swift:85:57:85:75 | call to Data.init(_:) | provenance | |
20-
| rncryptor.swift:85:70:85:70 | abc | rncryptor.swift:85:62:85:70 | ... .+(_:_:) ... | provenance | |
2114
| rncryptor.swift:86:62:86:62 | 123 | rncryptor.swift:86:62:86:62 | "..." | provenance | |
2215
| rncryptor.swift:86:62:86:62 | "..." | rncryptor.swift:86:57:86:91 | call to Data.init(_:) | provenance | |
2316
| rncryptor.swift:86:87:86:87 | abc | rncryptor.swift:86:62:86:62 | "..." | provenance | |
@@ -31,10 +24,6 @@ edges
3124
| rncryptor.swift:94:26:94:26 | abc | rncryptor.swift:94:2:94:2 | [post] myMutableString2 | provenance | |
3225
| rncryptor.swift:94:26:94:26 | abc | rncryptor.swift:95:62:95:62 | myMutableString2 | provenance | AdditionalTaintStep |
3326
| rncryptor.swift:95:62:95:62 | myMutableString2 | rncryptor.swift:95:57:95:78 | call to Data.init(_:) | provenance | |
34-
| rncryptor.swift:97:25:97:25 | 123 | rncryptor.swift:99:62:99:62 | myMutableString3 | provenance | |
35-
| rncryptor.swift:99:62:99:62 | myMutableString3 | rncryptor.swift:99:57:99:78 | call to Data.init(_:) | provenance | |
36-
| rncryptor.swift:102:22:102:22 | abc | rncryptor.swift:103:62:103:62 | myMutableString4 | provenance | |
37-
| rncryptor.swift:103:62:103:62 | myMutableString4 | rncryptor.swift:103:57:103:78 | call to Data.init(_:) | provenance | |
3827
| test.swift:29:3:29:3 | this string is constant | test.swift:33:10:33:28 | call to getConstantString() | provenance | |
3928
| test.swift:33:2:33:34 | call to Array<Element>.init(_:) [Collection element] | test.swift:44:27:44:44 | call to getConstantArray() [Collection element] | provenance | |
4029
| test.swift:33:10:33:28 | call to getConstantString() | test.swift:33:10:33:30 | .utf8 | provenance | |
@@ -62,16 +51,6 @@ nodes
6251
| rncryptor.swift:76:152:76:152 | myConstantSalt2 | semmle.label | myConstantSalt2 |
6352
| rncryptor.swift:78:135:78:135 | myConstantSalt1 | semmle.label | myConstantSalt1 |
6453
| rncryptor.swift:79:160:79:160 | myConstantSalt2 | semmle.label | myConstantSalt2 |
65-
| rncryptor.swift:83:57:83:88 | call to Data.init(_:) | semmle.label | call to Data.init(_:) |
66-
| rncryptor.swift:83:62:83:62 | 123 | semmle.label | 123 |
67-
| rncryptor.swift:83:62:83:87 | ... .+(_:_:) ... | semmle.label | ... .+(_:_:) ... |
68-
| rncryptor.swift:84:57:84:88 | call to Data.init(_:) | semmle.label | call to Data.init(_:) |
69-
| rncryptor.swift:84:62:84:83 | ... .+(_:_:) ... | semmle.label | ... .+(_:_:) ... |
70-
| rncryptor.swift:84:83:84:83 | abc | semmle.label | abc |
71-
| rncryptor.swift:85:57:85:75 | call to Data.init(_:) | semmle.label | call to Data.init(_:) |
72-
| rncryptor.swift:85:62:85:62 | 123 | semmle.label | 123 |
73-
| rncryptor.swift:85:62:85:70 | ... .+(_:_:) ... | semmle.label | ... .+(_:_:) ... |
74-
| rncryptor.swift:85:70:85:70 | abc | semmle.label | abc |
7554
| rncryptor.swift:86:57:86:91 | call to Data.init(_:) | semmle.label | call to Data.init(_:) |
7655
| rncryptor.swift:86:62:86:62 | 123 | semmle.label | 123 |
7756
| rncryptor.swift:86:62:86:62 | "..." | semmle.label | "..." |
@@ -88,12 +67,6 @@ nodes
8867
| rncryptor.swift:94:26:94:26 | abc | semmle.label | abc |
8968
| rncryptor.swift:95:57:95:78 | call to Data.init(_:) | semmle.label | call to Data.init(_:) |
9069
| rncryptor.swift:95:62:95:62 | myMutableString2 | semmle.label | myMutableString2 |
91-
| rncryptor.swift:97:25:97:25 | 123 | semmle.label | 123 |
92-
| rncryptor.swift:99:57:99:78 | call to Data.init(_:) | semmle.label | call to Data.init(_:) |
93-
| rncryptor.swift:99:62:99:62 | myMutableString3 | semmle.label | myMutableString3 |
94-
| rncryptor.swift:102:22:102:22 | abc | semmle.label | abc |
95-
| rncryptor.swift:103:57:103:78 | call to Data.init(_:) | semmle.label | call to Data.init(_:) |
96-
| rncryptor.swift:103:62:103:62 | myMutableString4 | semmle.label | myMutableString4 |
9770
| test.swift:29:3:29:3 | this string is constant | semmle.label | this string is constant |
9871
| test.swift:33:2:33:34 | call to Array<Element>.init(_:) [Collection element] | semmle.label | call to Array<Element>.init(_:) [Collection element] |
9972
| test.swift:33:10:33:28 | call to getConstantString() | semmle.label | call to getConstantString() |
@@ -120,19 +93,13 @@ subpaths
12093
| rncryptor.swift:76:152:76:152 | myConstantSalt2 | rncryptor.swift:60:29:60:29 | 0 | rncryptor.swift:76:152:76:152 | myConstantSalt2 | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:60:29:60:29 | 0 | 0 |
12194
| rncryptor.swift:78:135:78:135 | myConstantSalt1 | rncryptor.swift:59:29:59:29 | abcdef123456 | rncryptor.swift:78:135:78:135 | myConstantSalt1 | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:59:29:59:29 | abcdef123456 | abcdef123456 |
12295
| rncryptor.swift:79:160:79:160 | myConstantSalt2 | rncryptor.swift:60:29:60:29 | 0 | rncryptor.swift:79:160:79:160 | myConstantSalt2 | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:60:29:60:29 | 0 | 0 |
123-
| rncryptor.swift:83:57:83:88 | call to Data.init(_:) | rncryptor.swift:83:62:83:62 | 123 | rncryptor.swift:83:57:83:88 | call to Data.init(_:) | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:83:62:83:62 | 123 | 123 |
124-
| rncryptor.swift:84:57:84:88 | call to Data.init(_:) | rncryptor.swift:84:83:84:83 | abc | rncryptor.swift:84:57:84:88 | call to Data.init(_:) | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:84:83:84:83 | abc | abc |
125-
| rncryptor.swift:85:57:85:75 | call to Data.init(_:) | rncryptor.swift:85:62:85:62 | 123 | rncryptor.swift:85:57:85:75 | call to Data.init(_:) | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:85:62:85:62 | 123 | 123 |
126-
| rncryptor.swift:85:57:85:75 | call to Data.init(_:) | rncryptor.swift:85:70:85:70 | abc | rncryptor.swift:85:57:85:75 | call to Data.init(_:) | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:85:70:85:70 | abc | abc |
12796
| rncryptor.swift:86:57:86:91 | call to Data.init(_:) | rncryptor.swift:86:62:86:62 | 123 | rncryptor.swift:86:57:86:91 | call to Data.init(_:) | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:86:62:86:62 | 123 | 123 |
12897
| rncryptor.swift:86:57:86:91 | call to Data.init(_:) | rncryptor.swift:86:87:86:87 | abc | rncryptor.swift:86:57:86:91 | call to Data.init(_:) | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:86:87:86:87 | abc | abc |
12998
| rncryptor.swift:87:57:87:81 | call to Data.init(_:) | rncryptor.swift:87:62:87:62 | 123 | rncryptor.swift:87:57:87:81 | call to Data.init(_:) | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:87:62:87:62 | 123 | 123 |
13099
| rncryptor.swift:87:57:87:81 | call to Data.init(_:) | rncryptor.swift:87:68:87:68 | const | rncryptor.swift:87:57:87:81 | call to Data.init(_:) | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:87:68:87:68 | const | const |
131100
| rncryptor.swift:87:57:87:81 | call to Data.init(_:) | rncryptor.swift:87:76:87:76 | )abc | rncryptor.swift:87:57:87:81 | call to Data.init(_:) | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:87:76:87:76 | )abc | )abc |
132101
| rncryptor.swift:91:57:91:78 | call to Data.init(_:) | rncryptor.swift:89:25:89:25 | 123 | rncryptor.swift:91:57:91:78 | call to Data.init(_:) | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:89:25:89:25 | 123 | 123 |
133102
| rncryptor.swift:95:57:95:78 | call to Data.init(_:) | rncryptor.swift:94:26:94:26 | abc | rncryptor.swift:95:57:95:78 | call to Data.init(_:) | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:94:26:94:26 | abc | abc |
134-
| rncryptor.swift:99:57:99:78 | call to Data.init(_:) | rncryptor.swift:97:25:97:25 | 123 | rncryptor.swift:99:57:99:78 | call to Data.init(_:) | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:97:25:97:25 | 123 | 123 |
135-
| rncryptor.swift:103:57:103:78 | call to Data.init(_:) | rncryptor.swift:102:22:102:22 | abc | rncryptor.swift:103:57:103:78 | call to Data.init(_:) | The value '$@' is used as a constant, which is insecure for hashing passwords. | rncryptor.swift:102:22:102:22 | abc | abc |
136103
| test.swift:51:49:51:49 | constantSalt | test.swift:43:35:43:130 | [...] | test.swift:51:49:51:49 | constantSalt | The value '$@' is used as a constant, which is insecure for hashing passwords. | test.swift:43:35:43:130 | [...] | [...] |
137104
| test.swift:52:49:52:49 | constantStringSalt | test.swift:29:3:29:3 | this string is constant | test.swift:52:49:52:49 | constantStringSalt | The value '$@' is used as a constant, which is insecure for hashing passwords. | test.swift:29:3:29:3 | this string is constant | this string is constant |
138105
| test.swift:56:59:56:59 | constantSalt | test.swift:43:35:43:130 | [...] | test.swift:56:59:56:59 | constantSalt | The value '$@' is used as a constant, which is insecure for hashing passwords. | test.swift:43:35:43:130 | [...] | [...] |

‎swift/ql/test/query-tests/Security/CWE-760/rncryptor.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,9 @@ func test(myPassword: String) {
8080

8181
// appending constants
8282
let _ = myEncryptor.key(forPassword: myPassword, salt: Data(getARandomString() + getARandomString()), settings: myKeyDerivationSettings) // GOOD
83-
let _ = myEncryptor.key(forPassword: myPassword, salt: Data("123" + getARandomString()), settings: myKeyDerivationSettings) // GOOD [FALSE POSITIVE]
84-
let _ = myEncryptor.key(forPassword: myPassword, salt: Data(getARandomString() + "abc"), settings: myKeyDerivationSettings) // GOOD [FALSE POSITIVE]
85-
let _ = myEncryptor.key(forPassword: myPassword, salt: Data("123" + "abc"), settings: myKeyDerivationSettings) // BAD (constant salt)
83+
let _ = myEncryptor.key(forPassword: myPassword, salt: Data("123" + getARandomString()), settings: myKeyDerivationSettings) // GOOD
84+
let _ = myEncryptor.key(forPassword: myPassword, salt: Data(getARandomString() + "abc"), settings: myKeyDerivationSettings) // GOOD
85+
let _ = myEncryptor.key(forPassword: myPassword, salt: Data("123" + "abc"), settings: myKeyDerivationSettings) // BAD (constant salt) [NOT DETECTED]
8686
let _ = myEncryptor.key(forPassword: myPassword, salt: Data("123\(getARandomString())abc"), settings: myKeyDerivationSettings) // GOOD [FALSE POSITIVE]
8787
let _ = myEncryptor.key(forPassword: myPassword, salt: Data("123\("const"))abc"), settings: myKeyDerivationSettings) // BAD (constant salt)
8888

@@ -96,9 +96,9 @@ func test(myPassword: String) {
9696

9797
var myMutableString3 = "123"
9898
myMutableString3 += getARandomString()
99-
let _ = myEncryptor.key(forPassword: myPassword, salt: Data(myMutableString3), settings: myKeyDerivationSettings) // GOOD [FALSE POSITIVE]
99+
let _ = myEncryptor.key(forPassword: myPassword, salt: Data(myMutableString3), settings: myKeyDerivationSettings) // GOOD
100100

101101
var myMutableString4 = getARandomString()
102102
myMutableString4 += "abc"
103-
let _ = myEncryptor.key(forPassword: myPassword, salt: Data(myMutableString4), settings: myKeyDerivationSettings) // GOOD [FALSE POSITIVE]
103+
let _ = myEncryptor.key(forPassword: myPassword, salt: Data(myMutableString4), settings: myKeyDerivationSettings) // GOOD
104104
}

0 commit comments

Comments
 (0)