Skip to content

Commit 59869ac

Browse files
committed
Java: teach Encryption.qll about MessageDigest.getInstance
We already modelled usage of the protected `MessageDigest(String algo)` constructor as a crypto algorithm specification. For some reason we did not model the more commonly used public `MessageDigest.getInstance` method.
1 parent fcc2b66 commit 59869ac

File tree

6 files changed

+34
-13
lines changed

6 files changed

+34
-13
lines changed

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,16 @@ abstract class JavaSecurityAlgoSpec extends CryptoAlgoSpec { }
220220
class JavaSecurityMessageDigest extends JavaSecurityAlgoSpec {
221221
JavaSecurityMessageDigest() {
222222
exists(Constructor c | c.getAReference() = this |
223-
c.getDeclaringType().getQualifiedName() = "java.security.MessageDigest"
223+
c.getDeclaringType().hasQualifiedName("java.security", "MessageDigest")
224+
)
225+
or
226+
exists(Method m | m.getAReference() = this |
227+
m.getDeclaringType().hasQualifiedName("java.security", "MessageDigest") and
228+
m.getName() = "getInstance"
224229
)
225230
}
226231

227-
override Expr getAlgoSpec() { result = this.(ConstructorCall).getArgument(0) }
232+
override Expr getAlgoSpec() { result = this.(Call).getArgument(0) }
228233
}
229234

230235
class JavaSecuritySignature extends JavaSecurityAlgoSpec {

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,23 @@
22

33
import java.util.Arrays;
44
import java.util.List;
5+
import java.security.MessageDigest;
56

67
class Test {
78
List<String> badStrings = Arrays.asList(
8-
"DES",
9+
"DES",
910
"des",
1011
"des_function",
1112
"function_using_des",
1213
"EncryptWithDES");
13-
14+
1415
List<String> goodStrings = Arrays.asList(
1516
"AES",
1617
"AES_function",
1718
// false negative - can't think of a good way to detect this without
1819
// catching things we shouldn't
1920
"AESEncryption");
20-
21+
2122
List<String> unknownStrings = Arrays.asList(
2223
// not a use of RC2 (camelCase is tricky)
2324
"GetPrc2",
@@ -29,4 +30,12 @@ class Test {
2930
"species",
3031
// can't detect unknown algorithms
3132
"SOMENEWACRONYM");
32-
}
33+
public static abstract class SomeDigest extends MessageDigest {
34+
public SomeDigest() {
35+
super("some");
36+
}
37+
}
38+
public void test() throws Exception {
39+
MessageDigest.getInstance("another");
40+
}
41+
}
Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
| Test.java:8:4:8:8 | "DES" |
2-
| Test.java:9:4:9:8 | "des" |
3-
| Test.java:10:4:10:17 | "des_function" |
4-
| Test.java:11:4:11:23 | "function_using_des" |
5-
| Test.java:12:4:12:19 | "EncryptWithDES" |
1+
| Test.java:9:4:9:8 | "DES" |
2+
| Test.java:10:4:10:8 | "des" |
3+
| Test.java:11:4:11:17 | "des_function" |
4+
| Test.java:12:4:12:23 | "function_using_des" |
5+
| Test.java:13:4:13:19 | "EncryptWithDES" |
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| Test.java:35:4:35:17 | super(...) | Test.java:35:10:35:15 | "some" |
2+
| Test.java:39:3:39:38 | getInstance(...) | Test.java:39:29:39:37 | "another" |
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import default
2+
import semmle.code.java.security.Encryption
3+
4+
from CryptoAlgoSpec s
5+
select s, s.getAlgoSpec()
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
| Test.java:15:4:15:8 | "AES" |
2-
| Test.java:16:4:16:17 | "AES_function" |
1+
| Test.java:16:4:16:8 | "AES" |
2+
| Test.java:17:4:17:17 | "AES_function" |

0 commit comments

Comments
 (0)