Skip to content

Commit 86c04e6

Browse files
luchua-bcluchua-bc
committed
Detect the scenario of passwords concatenated with a salt to reduce FPs
1 parent 39103af commit 86c04e6

File tree

3 files changed

+52
-17
lines changed

3 files changed

+52
-17
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-759/HashWithoutSalt.ql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,20 @@ class HashWithoutSaltConfiguration extends TaintTracking::Configuration {
7575
)
7676
)
7777
}
78+
79+
/**
80+
* Holds if a password is concatenated with a salt then hashed together through the call `System.arraycopy(password.getBytes(), ...)`. For example,
81+
* `System.arraycopy(password.getBytes(), 0, allBytes, 0, password.getBytes().length);`
82+
* `System.arraycopy(salt, 0, allBytes, password.getBytes().length, salt.length);`
83+
* `byte[] messageDigest = md.digest(allBytes);`
84+
*/
85+
override predicate isSanitizer(DataFlow::Node node) {
86+
exists(MethodAccess ma |
87+
ma.getMethod().getDeclaringType().hasQualifiedName("java.lang", "System") and
88+
ma.getMethod().hasName("arraycopy") and
89+
ma.getArgument(0) = node.asExpr()
90+
)
91+
}
7892
}
7993

8094
from DataFlow::PathNode source, DataFlow::PathNode sink, HashWithoutSaltConfiguration c

‎java/ql/test/experimental/query-tests/security/CWE-759/HashWithoutSalt.expected

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
edges
2-
| HashWithoutSalt.java:9:36:9:43 | password : String | HashWithoutSalt.java:9:36:9:54 | getBytes(...) |
3-
| HashWithoutSalt.java:15:13:15:20 | password : String | HashWithoutSalt.java:15:13:15:31 | getBytes(...) |
2+
| HashWithoutSalt.java:10:36:10:43 | password : String | HashWithoutSalt.java:10:36:10:54 | getBytes(...) |
3+
| HashWithoutSalt.java:17:13:17:20 | password : String | HashWithoutSalt.java:17:13:17:31 | getBytes(...) |
44
nodes
5-
| HashWithoutSalt.java:9:36:9:43 | password : String | semmle.label | password : String |
6-
| HashWithoutSalt.java:9:36:9:54 | getBytes(...) | semmle.label | getBytes(...) |
7-
| HashWithoutSalt.java:15:13:15:20 | password : String | semmle.label | password : String |
8-
| HashWithoutSalt.java:15:13:15:31 | getBytes(...) | semmle.label | getBytes(...) |
5+
| HashWithoutSalt.java:10:36:10:43 | password : String | semmle.label | password : String |
6+
| HashWithoutSalt.java:10:36:10:54 | getBytes(...) | semmle.label | getBytes(...) |
7+
| HashWithoutSalt.java:17:13:17:20 | password : String | semmle.label | password : String |
8+
| HashWithoutSalt.java:17:13:17:31 | getBytes(...) | semmle.label | getBytes(...) |
99
#select
10-
| HashWithoutSalt.java:9:36:9:54 | getBytes(...) | HashWithoutSalt.java:9:36:9:43 | password : String | HashWithoutSalt.java:9:36:9:54 | getBytes(...) | $@ is hashed without a salt. | HashWithoutSalt.java:9:36:9:43 | password | The password |
11-
| HashWithoutSalt.java:15:13:15:31 | getBytes(...) | HashWithoutSalt.java:15:13:15:20 | password : String | HashWithoutSalt.java:15:13:15:31 | getBytes(...) | $@ is hashed without a salt. | HashWithoutSalt.java:15:13:15:20 | password | The password |
10+
| HashWithoutSalt.java:10:36:10:54 | getBytes(...) | HashWithoutSalt.java:10:36:10:43 | password : String | HashWithoutSalt.java:10:36:10:54 | getBytes(...) | $@ is hashed without a salt. | HashWithoutSalt.java:10:36:10:43 | password | The password |
11+
| HashWithoutSalt.java:17:13:17:31 | getBytes(...) | HashWithoutSalt.java:17:13:17:20 | password : String | HashWithoutSalt.java:17:13:17:31 | getBytes(...) | $@ is hashed without a salt. | HashWithoutSalt.java:17:13:17:20 | password | The password |

‎java/ql/test/experimental/query-tests/security/CWE-759/HashWithoutSalt.java

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,61 @@
11
import java.security.MessageDigest;
22
import java.security.NoSuchAlgorithmException;
33
import java.security.SecureRandom;
4+
import java.util.Base64;
45

56
public class HashWithoutSalt {
67
// BAD - Hash without a salt.
7-
public void getSHA256Hash(String password) throws NoSuchAlgorithmException {
8+
public String getSHA256Hash(String password) throws NoSuchAlgorithmException {
89
MessageDigest md = MessageDigest.getInstance("SHA-256");
910
byte[] messageDigest = md.digest(password.getBytes());
11+
return Base64.getEncoder().encodeToString(messageDigest);
1012
}
1113

1214
// BAD - Hash without a salt.
13-
public void getSHA256Hash2(String password) throws NoSuchAlgorithmException {
15+
public String getSHA256Hash2(String password) throws NoSuchAlgorithmException {
1416
MessageDigest md = MessageDigest.getInstance("SHA-256");
1517
md.update(password.getBytes());
1618
byte[] messageDigest = md.digest();
19+
return Base64.getEncoder().encodeToString(messageDigest);
1720
}
1821

1922
// GOOD - Hash with a salt.
20-
public void getSHA256Hash(String password, byte[] salt) throws NoSuchAlgorithmException {
23+
public String getSHA256Hash(String password, byte[] salt) throws NoSuchAlgorithmException {
2124
MessageDigest md = MessageDigest.getInstance("SHA-256");
2225
md.update(salt);
2326
byte[] messageDigest = md.digest(password.getBytes());
27+
return Base64.getEncoder().encodeToString(messageDigest);
2428
}
2529

2630
// GOOD - Hash with a salt.
27-
public void getSHA256Hash2(String password, byte[] salt) throws NoSuchAlgorithmException {
31+
public String getSHA256Hash2(String password, byte[] salt) throws NoSuchAlgorithmException {
2832
MessageDigest md = MessageDigest.getInstance("SHA-256");
2933
md.update(salt);
3034
md.update(password.getBytes());
3135
byte[] messageDigest = md.digest();
36+
return Base64.getEncoder().encodeToString(messageDigest);
37+
}
38+
39+
// GOOD - Hash with a salt concatenated with the password.
40+
public String getSHA256Hash3(String password, byte[] salt) throws NoSuchAlgorithmException {
41+
MessageDigest md = MessageDigest.getInstance("SHA-256");
42+
43+
byte[] passBytes = password.getBytes();
44+
byte[] allBytes = new byte[passBytes.length + salt.length];
45+
System.arraycopy(passBytes, 0, allBytes, 0, passBytes.length);
46+
System.arraycopy(salt, 0, allBytes, passBytes.length, salt.length);
47+
byte[] messageDigest = md.digest(allBytes);
48+
49+
byte[] cipherBytes = new byte[32 + salt.length]; // SHA-256 is 32 bytes long
50+
System.arraycopy(messageDigest, 0, cipherBytes, 0, 32);
51+
System.arraycopy(salt, 0, cipherBytes, 32, salt.length);
52+
return Base64.getEncoder().encodeToString(cipherBytes);
3253
}
3354

3455
public static byte[] getSalt() throws NoSuchAlgorithmException {
35-
SecureRandom sr = SecureRandom.getInstance("SHA1PRNG");
36-
byte[] salt = new byte[16];
37-
sr.nextBytes(salt);
38-
return salt;
39-
}
56+
SecureRandom sr = SecureRandom.getInstance("SHA1PRNG");
57+
byte[] salt = new byte[16];
58+
sr.nextBytes(salt);
59+
return salt;
60+
}
4061
}

0 commit comments

Comments
 (0)