Skip to content

Commit ce2db21

Browse files
luchua-bcluchua-bc
committed
Query to detect hash without salt
1 parent 49f902d commit ce2db21

File tree

6 files changed

+188
-0
lines changed

6 files changed

+188
-0
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
public class HashWithoutSalt {
2+
// BAD - Hash without a salt.
3+
public void getSHA256Hash(String password) throws NoSuchAlgorithmException {
4+
MessageDigest md = MessageDigest.getInstance("SHA-256");
5+
byte[] messageDigest = md.digest(password.getBytes());
6+
}
7+
8+
// GOOD - Hash with a salt.
9+
public void getSHA256Hash(String password, byte[] salt) throws NoSuchAlgorithmException {
10+
MessageDigest md = MessageDigest.getInstance("SHA-256");
11+
md.update(salt);
12+
byte[] messageDigest = md.digest(password.getBytes());
13+
}
14+
}

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>In cryptography, "salt" is random data that are used as an additional input to a one-way function that hashes a password or pass-phrase. It makes dictionary attacks more difficult.</p>
6+
7+
<p>Without a salt, it is much easier for attackers to pre-compute the hash value using dictionary attack techniques such as rainbow tables to crack passwords.</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>Use a long random salt of at least 32 bytes then use the combination of password and salt to hash a password or password phrase.</p>
12+
</recommendation>
13+
14+
<example>
15+
<p>The following example shows two ways of hashing. In the 'BAD' case, no salt is provided. In the 'GOOD' case, a salt is provided.</p>
16+
<sample src="HashWithoutSalt.java" />
17+
</example>
18+
19+
<references>
20+
<li>
21+
DZone:
22+
<a href="https://dzone.com/articles/a-look-at-java-cryptography">A Look at Java Cryptography</a>
23+
</li>
24+
<li>
25+
CWE:
26+
<a href="https://cwe.mitre.org/data/definitions/759.html">CWE-759: Use of a One-Way Hash without a Salt</a>
27+
</li>
28+
</references>
29+
</qhelp>

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

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/**
2+
* @id java/hash-without-salt
3+
* @name Use of a One-Way Hash without a Salt
4+
* @description Hashed passwords without a salt are vulnerable to dictionary attacks.
5+
* @kind path-problem
6+
* @tags security
7+
* external/cwe-759
8+
*/
9+
10+
import java
11+
import semmle.code.java.dataflow.TaintTracking
12+
import DataFlow::PathGraph
13+
14+
/** The Java class `java.security.MessageDigest` */
15+
class MessageDigest extends RefType {
16+
MessageDigest() { this.hasQualifiedName("java.security", "MessageDigest") }
17+
}
18+
19+
/** The method `digest()` declared in `java.security.MessageDigest`. */
20+
class MDDigestMethod extends Method {
21+
MDDigestMethod() {
22+
getDeclaringType() instanceof MessageDigest and
23+
hasName("digest")
24+
}
25+
}
26+
27+
/** The method `update()` declared in `java.security.MessageDigest`. */
28+
class MDUpdateMethod extends Method {
29+
MDUpdateMethod() {
30+
getDeclaringType() instanceof MessageDigest and
31+
hasName("update")
32+
}
33+
}
34+
35+
/**
36+
* Gets a regular expression for matching common names of variables that indicate the value being held is a password.
37+
*/
38+
string getPasswordRegex() { result = "(?i).*pass(wd|word|code|phrase).*" }
39+
40+
/** Finds variables that hold password information judging by their names. */
41+
class PasswordVarExpr extends Expr {
42+
PasswordVarExpr() {
43+
exists(Variable v | this = v.getAnAccess() | v.getName().regexpMatch(getPasswordRegex()))
44+
}
45+
}
46+
47+
/** Taint configuration tracking flow from an expression whose name suggests it holds password data to a method call that generates a hash without a salt. */
48+
class HashWithoutSaltConfiguration extends TaintTracking::Configuration {
49+
HashWithoutSaltConfiguration() { this = "HashWithoutSaltConfiguration" }
50+
51+
override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof PasswordVarExpr }
52+
53+
override predicate isSink(DataFlow::Node sink) {
54+
exists(
55+
MethodAccess mda, MethodAccess mua // invoke `md.digest()` with only one call of `md.update(password)`, that is, without the call of `md.update(digest)`
56+
|
57+
sink.asExpr() = mda.getQualifier() and
58+
mda.getMethod() instanceof MDDigestMethod and
59+
mda.getNumArgument() = 0 and // md.digest()
60+
mua.getMethod() instanceof MDUpdateMethod and // md.update(password)
61+
mua.getQualifier() = mda.getQualifier().(VarAccess).getVariable().getAnAccess() and
62+
not exists(MethodAccess mua2 |
63+
mua2.getMethod() instanceof MDUpdateMethod and // md.update(salt)
64+
mua2.getQualifier() = mua.getQualifier().(VarAccess).getVariable().getAnAccess() and
65+
mua2 != mua
66+
)
67+
)
68+
or
69+
// invoke `md.digest(password)` without another call of `md.update(salt)`
70+
exists(MethodAccess mda |
71+
sink.asExpr() = mda and
72+
mda.getMethod() instanceof MDDigestMethod and // md.digest(password)
73+
mda.getNumArgument() = 1 and
74+
not exists(MethodAccess mua |
75+
mua.getMethod() instanceof MDUpdateMethod and // md.update(salt)
76+
mua.getQualifier() = mda.getQualifier().(VarAccess).getVariable().getAnAccess()
77+
)
78+
)
79+
}
80+
81+
/** Holds for additional steps such as `passwordStr.getBytes()` */
82+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
83+
exists(MethodAccess ma |
84+
pred.asExpr() = ma.getAnArgument() and
85+
(succ.asExpr() = ma or succ.asExpr() = ma.getQualifier())
86+
)
87+
}
88+
}
89+
90+
from DataFlow::PathNode source, DataFlow::PathNode sink, HashWithoutSaltConfiguration c
91+
where c.hasFlowPath(source, sink)
92+
select sink.getNode(), source, sink, "$@ is hashed without a salt.", source.getNode(),
93+
"The password"

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
edges
2+
| HashWithoutSalt.java:9:36:9:43 | password : String | HashWithoutSalt.java:9:26:9:55 | digest(...) |
3+
| HashWithoutSalt.java:15:13:15:20 | password : String | HashWithoutSalt.java:16:26:16:27 | md |
4+
nodes
5+
| HashWithoutSalt.java:9:26:9:55 | digest(...) | semmle.label | digest(...) |
6+
| HashWithoutSalt.java:9:36:9:43 | password : String | semmle.label | password : String |
7+
| HashWithoutSalt.java:15:13:15:20 | password : String | semmle.label | password : String |
8+
| HashWithoutSalt.java:16:26:16:27 | md | semmle.label | md |
9+
#select
10+
| HashWithoutSalt.java:9:26:9:55 | digest(...) | HashWithoutSalt.java:9:36:9:43 | password : String | HashWithoutSalt.java:9:26:9:55 | digest(...) | $@ is hashed without a salt. | HashWithoutSalt.java:9:36:9:43 | password | The password |
11+
| HashWithoutSalt.java:16:26:16:27 | md | HashWithoutSalt.java:15:13:15:20 | password : String | HashWithoutSalt.java:16:26:16:27 | md | $@ is hashed without a salt. | HashWithoutSalt.java:15:13:15:20 | password | The password |

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import java.security.MessageDigest;
2+
import java.security.NoSuchAlgorithmException;
3+
import java.security.SecureRandom;
4+
5+
public class HashWithoutSalt {
6+
// BAD - Hash without a salt.
7+
public void getSHA256Hash(String password) throws NoSuchAlgorithmException {
8+
MessageDigest md = MessageDigest.getInstance("SHA-256");
9+
byte[] messageDigest = md.digest(password.getBytes());
10+
}
11+
12+
// BAD - Hash without a salt.
13+
public void getSHA256Hash2(String password) throws NoSuchAlgorithmException {
14+
MessageDigest md = MessageDigest.getInstance("SHA-256");
15+
md.update(password.getBytes());
16+
byte[] messageDigest = md.digest();
17+
}
18+
19+
// GOOD - Hash with a salt.
20+
public void getSHA256Hash(String password, byte[] salt) throws NoSuchAlgorithmException {
21+
MessageDigest md = MessageDigest.getInstance("SHA-256");
22+
md.update(salt);
23+
byte[] messageDigest = md.digest(password.getBytes());
24+
}
25+
26+
// GOOD - Hash with a salt.
27+
public void getSHA256Hash2(String password, byte[] salt) throws NoSuchAlgorithmException {
28+
MessageDigest md = MessageDigest.getInstance("SHA-256");
29+
md.update(salt);
30+
md.update(password.getBytes());
31+
byte[] messageDigest = md.digest();
32+
}
33+
34+
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+
}
40+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-759/HashWithoutSalt.ql

0 commit comments

Comments
 (0)