Merge pull request github#3216 from aibaars/message-digest

Java: teach Encryption.qll about MessageDigest.getInstance

Author: aschackmull

java/ql/src/semmle/code/java/security/Encryption.qll

@@ -220,11 +220,16 @@ abstract class JavaSecurityAlgoSpec extends CryptoAlgoSpec { }
 class JavaSecurityMessageDigest extends JavaSecurityAlgoSpec {
   JavaSecurityMessageDigest() {
     exists(Constructor c | c.getAReference() = this |
-      c.getDeclaringType().getQualifiedName() = "java.security.MessageDigest"
+      c.getDeclaringType().hasQualifiedName("java.security", "MessageDigest")
+    )
+    or
+    exists(Method m | m.getAReference() = this |
+      m.getDeclaringType().hasQualifiedName("java.security", "MessageDigest") and
+      m.getName() = "getInstance"
     )
   }
 
-  override Expr getAlgoSpec() { result = this.(ConstructorCall).getArgument(0) }
+  override Expr getAlgoSpec() { result = this.(Call).getArgument(0) }
 }
 
 class JavaSecuritySignature extends JavaSecurityAlgoSpec {

java/ql/test/library-tests/Encryption/Test.java

@@ -2,22 +2,23 @@
 
 import java.util.Arrays;
 import java.util.List;
+import java.security.MessageDigest;
 
 class Test {
 	List<String> badStrings = Arrays.asList(
-			"DES", 
+			"DES",
 			"des",
 			"des_function",
 			"function_using_des",
 			"EncryptWithDES");
-			
+
 	List<String> goodStrings = Arrays.asList(
 			"AES",
 			"AES_function",
 			// false negative - can't think of a good way to detect this without
 			// catching things we shouldn't
 			"AESEncryption");
-			
+
 	List<String> unknownStrings = Arrays.asList(
 			// not a use of RC2 (camelCase is tricky)
 			"GetPrc2",
@@ -29,4 +30,12 @@ class Test {
 			"species",
 			// can't detect unknown algorithms
 			"SOMENEWACRONYM");
-}
+        public static abstract class SomeDigest extends MessageDigest {
+		public SomeDigest() {
+			super("some");
+		}
+	}
+	public void test() throws Exception {
+		MessageDigest.getInstance("another");
+	}
+}

java/ql/test/library-tests/Encryption/blacklist.expected

@@ -1,5 +1,5 @@
-| Test.java:8:4:8:8 | "DES" |
-| Test.java:9:4:9:8 | "des" |
-| Test.java:10:4:10:17 | "des_function" |
-| Test.java:11:4:11:23 | "function_using_des" |
-| Test.java:12:4:12:19 | "EncryptWithDES" |
+| Test.java:9:4:9:8 | "DES" |
+| Test.java:10:4:10:8 | "des" |
+| Test.java:11:4:11:17 | "des_function" |
+| Test.java:12:4:12:23 | "function_using_des" |
+| Test.java:13:4:13:19 | "EncryptWithDES" |

java/ql/test/library-tests/Encryption/cryptoalgospec.expected

@@ -0,0 +1,2 @@
+| Test.java:35:4:35:17 | super(...) | Test.java:35:10:35:15 | "some" |
+| Test.java:39:3:39:38 | getInstance(...) | Test.java:39:29:39:37 | "another" |

java/ql/test/library-tests/Encryption/cryptoalgospec.ql

@@ -0,0 +1,5 @@
+import default
+import semmle.code.java.security.Encryption
+
+from CryptoAlgoSpec s
+select s, s.getAlgoSpec()

java/ql/test/library-tests/Encryption/whitelist.expected

@@ -1,2 +1,2 @@
-| Test.java:15:4:15:8 | "AES" |
-| Test.java:16:4:16:17 | "AES_function" |
+| Test.java:16:4:16:8 | "AES" |
+| Test.java:17:4:17:17 | "AES_function" |