Merge pull request github#4833 from luchua-bc/java-broken-crypto-algorithms

Java: Add missing broken crypto algorithms

Author: aschackmull

java/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.qhelp

@@ -12,7 +12,7 @@ data.</p>
 </overview>
 <recommendation>
 
-<p>Ensure that you use a strong, modern cryptographic algorithm. Use at least AES-128 or RSA-2048.</p>
+<p>Ensure that you use a strong, modern cryptographic algorithm. Use at least AES-128 or RSA-2048. Do not use the ECB encryption mode since it is vulnerable to replay and other attacks.</p>
 
 </recommendation>
 <example>

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

@@ -97,7 +97,9 @@ string getAnInsecureAlgorithmName() {
   result = "RC2" or
   result = "RC4" or
   result = "RC5" or
-  result = "ARCFOUR" // a variant of RC4
+  result = "ARCFOUR" or // a variant of RC4
+  result = "ECB" or // encryption mode ECB like AES/ECB/NoPadding is vulnerable to replay and other attacks
+  result = "AES/CBC/PKCS[5|7]Padding" // CBC mode of operation with PKCS#5 (or PKCS#7) padding is vulnerable to padding oracle attacks
 }
 
 /**
@@ -139,7 +141,7 @@ string getASecureAlgorithmName() {
   result = "SHA512" or
   result = "CCM" or
   result = "GCM" or
-  result = "AES" or
+  result = "AES([^a-zA-Z](?!ECB|CBC/PKCS[5|7]Padding)).*" or
   result = "Blowfish" or
   result = "ECIES"
 }

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

@@ -10,7 +10,9 @@ class Test {
 			"des",
 			"des_function",
 			"function_using_des",
-			"EncryptWithDES");
+			"EncryptWithDES",
+			"AES/ECB/NoPadding",
+			"AES/CBC/PKCS5Padding");
 
 	List<String> goodStrings = Arrays.asList(
 			"AES",

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

@@ -1,2 +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" |
+| Test.java:37:4:37:17 | super(...) | Test.java:37:10:37:15 | "some" |
+| Test.java:41:3:41:38 | getInstance(...) | Test.java:41:29:41:37 | "another" |

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

@@ -3,3 +3,5 @@
 | Test.java:11:4:11:17 | "des_function" |
 | Test.java:12:4:12:23 | "function_using_des" |
 | Test.java:13:4:13:19 | "EncryptWithDES" |
+| Test.java:14:4:14:22 | "AES/ECB/NoPadding" |
+| Test.java:15:4:15:25 | "AES/CBC/PKCS5Padding" |

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

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