Skip to content

Commit 0c3e8ce

Browse files
geoffw0geoffw0
committed
Swift: Make append methods and string interpolation barriers for swift/constant-salt.
1 parent 2543f3e commit 0c3e8ce

File tree

3 files changed

+11
-34
lines changed

3 files changed

+11
-34
lines changed

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,14 @@ private class DefaultSaltSink extends ConstantSaltSink {
7979
*/
8080
private class AppendConstantSaltBarrier extends ConstantSaltBarrier {
8181
AppendConstantSaltBarrier() {
82-
this.asExpr() = any(AddExpr ae).getAnOperand() or
82+
this.asExpr() = any(AddExpr ae).getAnOperand()
83+
or
8384
this.asExpr() = any(AssignAddExpr aae).getAnOperand()
85+
or
86+
exists(CallExpr ce |
87+
ce.getStaticTarget().getName() =
88+
["append(_:)", "appending(_:)", "appendLiteral(_:)", "appendInterpolation(_:)"] and
89+
this.asExpr() = ce.getAnArgument().getExpr()
90+
)
8491
}
8592
}

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

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,8 @@ 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:86:62:86:62 | 123 | rncryptor.swift:86:62:86:62 | "..." | provenance | |
15-
| rncryptor.swift:86:62:86:62 | "..." | rncryptor.swift:86:57:86:91 | call to Data.init(_:) | provenance | |
16-
| rncryptor.swift:86:87:86:87 | abc | rncryptor.swift:86:62:86:62 | "..." | provenance | |
17-
| rncryptor.swift:87:62:87:62 | 123 | rncryptor.swift:87:62:87:62 | "..." | provenance | |
18-
| rncryptor.swift:87:62:87:62 | "..." | rncryptor.swift:87:57:87:81 | call to Data.init(_:) | provenance | |
19-
| rncryptor.swift:87:68:87:68 | const | rncryptor.swift:87:62:87:62 | "..." | provenance | |
20-
| rncryptor.swift:87:76:87:76 | )abc | rncryptor.swift:87:62:87:62 | "..." | provenance | |
2114
| rncryptor.swift:89:25:89:25 | 123 | rncryptor.swift:91:62:91:62 | myMutableString1 | provenance | |
2215
| rncryptor.swift:91:62:91:62 | myMutableString1 | rncryptor.swift:91:57:91:78 | call to Data.init(_:) | provenance | |
23-
| rncryptor.swift:94:2:94:2 | [post] myMutableString2 | rncryptor.swift:95:62:95:62 | myMutableString2 | provenance | |
24-
| rncryptor.swift:94:26:94:26 | abc | rncryptor.swift:94:2:94:2 | [post] myMutableString2 | provenance | |
25-
| rncryptor.swift:94:26:94:26 | abc | rncryptor.swift:95:62:95:62 | myMutableString2 | provenance | AdditionalTaintStep |
26-
| rncryptor.swift:95:62:95:62 | myMutableString2 | rncryptor.swift:95:57:95:78 | call to Data.init(_:) | provenance | |
2716
| test.swift:29:3:29:3 | this string is constant | test.swift:33:10:33:28 | call to getConstantString() | provenance | |
2817
| 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 | |
2918
| test.swift:33:10:33:28 | call to getConstantString() | test.swift:33:10:33:30 | .utf8 | provenance | |
@@ -51,22 +40,9 @@ nodes
5140
| rncryptor.swift:76:152:76:152 | myConstantSalt2 | semmle.label | myConstantSalt2 |
5241
| rncryptor.swift:78:135:78:135 | myConstantSalt1 | semmle.label | myConstantSalt1 |
5342
| rncryptor.swift:79:160:79:160 | myConstantSalt2 | semmle.label | myConstantSalt2 |
54-
| rncryptor.swift:86:57:86:91 | call to Data.init(_:) | semmle.label | call to Data.init(_:) |
55-
| rncryptor.swift:86:62:86:62 | 123 | semmle.label | 123 |
56-
| rncryptor.swift:86:62:86:62 | "..." | semmle.label | "..." |
57-
| rncryptor.swift:86:87:86:87 | abc | semmle.label | abc |
58-
| rncryptor.swift:87:57:87:81 | call to Data.init(_:) | semmle.label | call to Data.init(_:) |
59-
| rncryptor.swift:87:62:87:62 | 123 | semmle.label | 123 |
60-
| rncryptor.swift:87:62:87:62 | "..." | semmle.label | "..." |
61-
| rncryptor.swift:87:68:87:68 | const | semmle.label | const |
62-
| rncryptor.swift:87:76:87:76 | )abc | semmle.label | )abc |
6343
| rncryptor.swift:89:25:89:25 | 123 | semmle.label | 123 |
6444
| rncryptor.swift:91:57:91:78 | call to Data.init(_:) | semmle.label | call to Data.init(_:) |
6545
| rncryptor.swift:91:62:91:62 | myMutableString1 | semmle.label | myMutableString1 |
66-
| rncryptor.swift:94:2:94:2 | [post] myMutableString2 | semmle.label | [post] myMutableString2 |
67-
| rncryptor.swift:94:26:94:26 | abc | semmle.label | abc |
68-
| rncryptor.swift:95:57:95:78 | call to Data.init(_:) | semmle.label | call to Data.init(_:) |
69-
| rncryptor.swift:95:62:95:62 | myMutableString2 | semmle.label | myMutableString2 |
7046
| test.swift:29:3:29:3 | this string is constant | semmle.label | this string is constant |
7147
| test.swift:33:2:33:34 | call to Array<Element>.init(_:) [Collection element] | semmle.label | call to Array<Element>.init(_:) [Collection element] |
7248
| test.swift:33:10:33:28 | call to getConstantString() | semmle.label | call to getConstantString() |
@@ -93,13 +69,7 @@ subpaths
9369
| 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 |
9470
| 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 |
9571
| 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 |
96-
| 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 |
97-
| 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 |
98-
| 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 |
99-
| 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 |
100-
| 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 |
10172
| 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 |
102-
| 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 |
10373
| 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 | [...] | [...] |
10474
| 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 |
10575
| 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: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,16 +83,16 @@ func test(myPassword: String) {
8383
let _ = myEncryptor.key(forPassword: myPassword, salt: Data("123" + getARandomString()), settings: myKeyDerivationSettings) // GOOD
8484
let _ = myEncryptor.key(forPassword: myPassword, salt: Data(getARandomString() + "abc"), settings: myKeyDerivationSettings) // GOOD
8585
let _ = myEncryptor.key(forPassword: myPassword, salt: Data("123" + "abc"), settings: myKeyDerivationSettings) // BAD (constant salt) [NOT DETECTED]
86-
let _ = myEncryptor.key(forPassword: myPassword, salt: Data("123\(getARandomString())abc"), settings: myKeyDerivationSettings) // GOOD [FALSE POSITIVE]
87-
let _ = myEncryptor.key(forPassword: myPassword, salt: Data("123\("const"))abc"), settings: myKeyDerivationSettings) // BAD (constant salt)
86+
let _ = myEncryptor.key(forPassword: myPassword, salt: Data("123\(getARandomString())abc"), settings: myKeyDerivationSettings) // GOOD
87+
let _ = myEncryptor.key(forPassword: myPassword, salt: Data("123\("const"))abc"), settings: myKeyDerivationSettings) // BAD (constant salt) [NOT DETECTED]
8888

8989
var myMutableString1 = "123"
9090
myMutableString1.append(getARandomString())
9191
let _ = myEncryptor.key(forPassword: myPassword, salt: Data(myMutableString1), settings: myKeyDerivationSettings) // GOOD [FALSE POSITIVE]
9292

9393
var myMutableString2 = getARandomString()
9494
myMutableString2.append("abc")
95-
let _ = myEncryptor.key(forPassword: myPassword, salt: Data(myMutableString2), settings: myKeyDerivationSettings) // GOOD [FALSE POSITIVE]
95+
let _ = myEncryptor.key(forPassword: myPassword, salt: Data(myMutableString2), settings: myKeyDerivationSettings) // GOOD
9696

9797
var myMutableString3 = "123"
9898
myMutableString3 += getARandomString()

0 commit comments

Comments
 (0)