Update the query to work with wrapper classes

Author: luchua-bc

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

@@ -89,13 +89,15 @@ class HashWithoutSaltConfiguration extends TaintTracking::Configuration {
       ma.getArgument(0) = node.asExpr() and // Detect wrapper methods that invoke `md.update(salt)`
       ma != mua and
       (
+        ma.getQualifier().getType() instanceof Interface
+        or
         mua.getQualifier().(VarAccess).getVariable().getAnAccess() = ma.getQualifier()
         or
         mua.getAnArgument().(VarAccess).getVariable().getAnAccess() = ma.getQualifier()
         or
         mua.getQualifier().(VarAccess).getVariable().getAnAccess() = ma.getAnArgument()
         or
-        mua.getAnArgument().(VarAccess).getVariable().getAnAccess() = ma.getAnArgument()
+        mua.getArgument(0).(VarAccess).getVariable().getAnAccess() = ma.getAnArgument()
       ) and
       isMDUpdateCall(mua.getMethod())
     )

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

@@ -0,0 +1,11 @@
+import java.security.NoSuchAlgorithmException;
+
+public interface HASH {
+  void init() throws NoSuchAlgorithmException;
+
+  int getBlockSize();
+
+  void update(byte[] foo, int start, int len) throws NoSuchAlgorithmException;
+
+  byte[] digest() throws NoSuchAlgorithmException;
+}

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

@@ -1,11 +1,19 @@
 edges
 | HashWithoutSalt.java:10:36:10:43 | password : String | HashWithoutSalt.java:10:36:10:54 | getBytes(...) |
 | HashWithoutSalt.java:17:13:17:20 | password : String | HashWithoutSalt.java:17:13:17:31 | getBytes(...) |
+| HashWithoutSalt.java:98:22:98:29 | password : String | HashWithoutSalt.java:99:17:99:25 | passBytes : byte[] |
+| HashWithoutSalt.java:99:17:99:25 | passBytes : byte[] | SHA256.java:14:22:14:31 | foo : byte[] |
+| SHA256.java:14:22:14:31 | foo : byte[] | SHA256.java:15:15:15:17 | foo |
 nodes
 | HashWithoutSalt.java:10:36:10:43 | password : String | semmle.label | password : String |
 | HashWithoutSalt.java:10:36:10:54 | getBytes(...) | semmle.label | getBytes(...) |
 | HashWithoutSalt.java:17:13:17:20 | password : String | semmle.label | password : String |
 | HashWithoutSalt.java:17:13:17:31 | getBytes(...) | semmle.label | getBytes(...) |
+| HashWithoutSalt.java:98:22:98:29 | password : String | semmle.label | password : String |
+| HashWithoutSalt.java:99:17:99:25 | passBytes : byte[] | semmle.label | passBytes : byte[] |
+| SHA256.java:14:22:14:31 | foo : byte[] | semmle.label | foo : byte[] |
+| SHA256.java:15:15:15:17 | foo | semmle.label | foo |
 #select
 | 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 |
 | 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 |
+| SHA256.java:15:15:15:17 | foo | HashWithoutSalt.java:98:22:98:29 | password : String | SHA256.java:15:15:15:17 | foo | $@ is hashed without a salt. | HashWithoutSalt.java:98:22:98:29 | password | The password |

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

@@ -100,6 +100,22 @@ public String getSHA256Hash6(String password) throws NoSuchAlgorithmException {
 		return Base64.getEncoder().encodeToString(sha256.digest());
 	}
 
+	// GOOD - Invoke a wrapper implementation with a salt, which is only detectable when a class type is declared (not interface).
+	public String getSHA256Hash7(byte[] passphrase) throws NoSuchAlgorithmException, ClassNotFoundException, IllegalAccessException, InstantiationException {
+		Class c = Class.forName("SHA256");
+		HASH sha256 = (HASH) (c.newInstance());
+		byte[] tmp = new byte[4];
+		byte[] key = new byte[32 * 2];
+		for (int i = 0; i < 2; i++) {
+			sha256.init();
+			tmp[3] = (byte) i;
+			sha256.update(tmp, 0, tmp.length);
+			sha256.update(passphrase, 0, passphrase.length);
+			System.arraycopy(sha256.digest(), 0, key, i * 32, 32);
+		}
+		return Base64.getEncoder().encodeToString(key);
+	}
+
 	private String hash(String payload) throws NoSuchAlgorithmException {
 		MessageDigest alg = MessageDigest.getInstance("SHA-256");
 		return Base64.getEncoder().encodeToString(alg.digest(payload.getBytes(java.nio.charset.StandardCharsets.UTF_8)));

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

@@ -1,7 +1,7 @@
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 
-public class SHA256 {
+public class SHA256 implements HASH {
   MessageDigest md;
   public int getBlockSize() {return 32;}
   public void init() throws NoSuchAlgorithmException {