Fix tests and fix getStringValue method

Author: egregius313

java/ql/src/Security/CWE/CWE-327/MaybeBrokenCryptoAlgorithm.ql

@@ -27,7 +27,9 @@ import InsecureCryptoFlow::PathGraph
 string getStringValue(DataFlow::Node algo) {
   result = algo.asExpr().(StringLiteral).getValue()
   or
-  result = algo.asExpr().(PropertiesGetPropertyMethodCall).getPropertyValue()
+  exists(string value | value = algo.asExpr().(PropertiesGetPropertyMethodCall).getPropertyValue() |
+    result = value and not value.regexpMatch(getSecureAlgorithmRegex())
+  )
 }
 
 from InsecureCryptoFlow::PathNode source, InsecureCryptoFlow::PathNode sink, CryptoAlgoSpec c

java/ql/test/query-tests/security/CWE-327/semmle/tests/BrokenCryptoAlgorithm.expected

@@ -1,8 +1,12 @@
 edges
+| WeakHashing.java:21:86:21:90 | "MD5" : String | WeakHashing.java:21:56:21:91 | getProperty(...) |
 nodes
 | Test.java:19:45:19:49 | "DES" | semmle.label | "DES" |
 | Test.java:42:33:42:37 | "RC2" | semmle.label | "RC2" |
+| WeakHashing.java:21:56:21:91 | getProperty(...) | semmle.label | getProperty(...) |
+| WeakHashing.java:21:86:21:90 | "MD5" : String | semmle.label | "MD5" : String |
 subpaths
 #select
 | Test.java:19:20:19:50 | getInstance(...) | Test.java:19:45:19:49 | "DES" | Test.java:19:45:19:49 | "DES" | Cryptographic algorithm $@ is weak and should not be used. | Test.java:19:45:19:49 | "DES" | DES |
 | Test.java:42:14:42:38 | getInstance(...) | Test.java:42:33:42:37 | "RC2" | Test.java:42:33:42:37 | "RC2" | Cryptographic algorithm $@ is weak and should not be used. | Test.java:42:33:42:37 | "RC2" | RC2 |
+| WeakHashing.java:21:30:21:92 | getInstance(...) | WeakHashing.java:21:86:21:90 | "MD5" : String | WeakHashing.java:21:56:21:91 | getProperty(...) | Cryptographic algorithm $@ is weak and should not be used. | WeakHashing.java:21:86:21:90 | "MD5" | MD5 |

java/ql/test/query-tests/security/CWE-327/semmle/tests/MaybeBrokenCryptoAlgorithm.expected

@@ -2,7 +2,11 @@ edges
 nodes
 | Test.java:34:48:34:52 | "foo" | semmle.label | "foo" |
 | WeakHashing.java:15:55:15:83 | getProperty(...) | semmle.label | getProperty(...) |
+| WeakHashing.java:18:56:18:95 | getProperty(...) | semmle.label | getProperty(...) |
+| WeakHashing.java:21:56:21:91 | getProperty(...) | semmle.label | getProperty(...) |
 subpaths
 #select
 | Test.java:34:21:34:53 | new SecretKeySpec(...) | Test.java:34:48:34:52 | "foo" | Test.java:34:48:34:52 | "foo" | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | Test.java:34:48:34:52 | "foo" | foo |
 | WeakHashing.java:15:29:15:84 | getInstance(...) | WeakHashing.java:15:55:15:83 | getProperty(...) | WeakHashing.java:15:55:15:83 | getProperty(...) | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | WeakHashing.java:15:55:15:83 | getProperty(...) | MD5 |
+| WeakHashing.java:18:30:18:96 | getInstance(...) | WeakHashing.java:18:56:18:95 | getProperty(...) | WeakHashing.java:18:56:18:95 | getProperty(...) | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | WeakHashing.java:18:56:18:95 | getProperty(...) | MD5 |
+| WeakHashing.java:21:30:21:92 | getInstance(...) | WeakHashing.java:21:56:21:91 | getProperty(...) | WeakHashing.java:21:56:21:91 | getProperty(...) | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | WeakHashing.java:21:56:21:91 | getProperty(...) | MD5 |

java/ql/test/query-tests/security/CWE-327/semmle/tests/WeakHashing.java

@@ -16,14 +16,14 @@ void hashing() throws NoSuchAlgorithmException, IOException {
 
         // BAD: Using a weak hashing algorithm even with a secure default
         MessageDigest bad2 = MessageDigest.getInstance(props.getProperty("hashAlg1", "SHA-256"));
+
+        // BAD: Using a strong hashing algorithm but with a weak default
+        MessageDigest bad3 = MessageDigest.getInstance(props.getProperty("hashAlg2", "MD5"));
 
         // GOOD: Using a strong hashing algorithm
         MessageDigest ok = MessageDigest.getInstance(props.getProperty("hashAlg2"));
 
-        // OK: Using a strong hashing algorithm even with a weak default
-        MessageDigest ok2 = MessageDigest.getInstance(props.getProperty("hashAlg2", "MD5"));
-
         // OK: Property does not exist and default is secure
-        MessageDigest ok3 = MessageDigest.getInstance(props.getProperty("hashAlg3", "SHA-256"));
+        MessageDigest ok2 = MessageDigest.getInstance(props.getProperty("hashAlg3", "SHA-256"));
     }
 }