Skip to content

Commit 75f6795

Browse files
artem-smotrakovartem-smotrakov
committed
Covered Arrays.deepEquals() in NonConstantTimeCryptoComparison.ql
1 parent 5dbcf1d commit 75f6795

File tree

3 files changed

+39
-17
lines changed

3 files changed

+39
-17
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-208/NotConstantTimeCryptoComparison.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ private class NonConstantTimeCryptoComparisonConfig extends TaintTracking::Confi
5050
sink.asExpr() = [ma.getQualifier(), ma.getAnArgument()]
5151
or
5252
m.getDeclaringType().hasQualifiedName("java.util", "Arrays") and
53-
m.hasName("equals") and
53+
m.hasName(["equals", "deepEquals"]) and
5454
ma.getAnArgument() = sink.asExpr()
5555
or
5656
m.getDeclaringType().hasQualifiedName("java.util", "Objects") and

‎java/ql/test/experimental/query-tests/security/CWE-208/NotConstantTimeCryptoComparison.java renamed to ‎java/ql/test/experimental/query-tests/security/CWE-208/NonConstantTimeCryptoComparison.java

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,39 @@
77
import javax.crypto.Cipher;
88
import javax.crypto.Mac;
99

10-
public class NotConstantTimeCryptoComparison {
10+
public class NonConstantTimeCryptoComparison {
1111

12-
// BAD: compare MACs using a not-constant time method
12+
// BAD: compare MACs using a non-constant time method
1313
public boolean unsafeMacCheck(byte[] expectedMac, byte[] data) throws Exception {
1414
Mac mac = Mac.getInstance("HmacSHA256");
1515
byte[] actualMac = mac.doFinal(data);
1616
return Arrays.equals(expectedMac, actualMac);
1717
}
1818

19+
20+
// BAD: compare MACs using a non-constant time method
21+
public boolean unsafeMacCheckWithArraysDeepEquals(byte[] expectedMac, byte[] data) throws Exception {
22+
Mac mac = Mac.getInstance("HmacSHA256");
23+
byte[] actualMac = mac.doFinal(data);
24+
return Arrays.deepEquals(cast(expectedMac), cast(actualMac));
25+
}
26+
27+
private static Object[] cast(byte[] array) {
28+
Object[] result = new Object[array.length];
29+
for (int i = 0; i < array.length; i++) {
30+
result[i] = array[i];
31+
}
32+
return result;
33+
}
34+
1935
// GOOD: compare MACs using a constant time method
2036
public boolean saferMacCheck(byte[] expectedMac, byte[] data) throws Exception {
2137
Mac mac = Mac.getInstance("HmacSHA256");
2238
byte[] actualMac = mac.doFinal(data);
2339
return MessageDigest.isEqual(expectedMac, actualMac);
2440
}
2541

26-
// BAD: compare signatures using a not-constant time method
42+
// BAD: compare signatures using a non-constant time method
2743
public boolean unsafeCheckSignatures(byte[] expected, byte[] data, PrivateKey key) throws Exception {
2844
Signature engine = Signature.getInstance("SHA256withRSA");
2945
engine.initSign(key);
@@ -41,7 +57,7 @@ public boolean saferCheckSignatures(byte[] expected, byte[] data, PrivateKey key
4157
return MessageDigest.isEqual(expected, signature);
4258
}
4359

44-
// BAD: compare ciphertexts using a not-constant time method
60+
// BAD: compare ciphertexts using a non-constant time method
4561
public boolean unsafeCheckCustomMac(byte[] expected, byte[] plaintext, Key key) throws Exception {
4662
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
4763
cipher.init(Cipher.ENCRYPT_MODE, key);

‎java/ql/test/experimental/query-tests/security/CWE-208/NotConstantTimeCryptoComparison.expected

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
11
edges
2-
| NotConstantTimeCryptoComparison.java:15:28:15:44 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:16:43:16:51 | actualMac |
3-
| NotConstantTimeCryptoComparison.java:31:28:31:40 | sign(...) : byte[] | NotConstantTimeCryptoComparison.java:32:40:32:48 | signature |
4-
| NotConstantTimeCryptoComparison.java:48:22:48:46 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:49:45:49:47 | tag |
2+
| NonConstantTimeCryptoComparison.java:15:28:15:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:16:43:16:51 | actualMac |
3+
| NonConstantTimeCryptoComparison.java:23:28:23:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:24:58:24:66 | actualMac : byte[] |
4+
| NonConstantTimeCryptoComparison.java:24:58:24:66 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:24:53:24:67 | cast(...) |
5+
| NonConstantTimeCryptoComparison.java:47:28:47:40 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:48:40:48:48 | signature |
6+
| NonConstantTimeCryptoComparison.java:64:22:64:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:65:45:65:47 | tag |
57
nodes
6-
| NotConstantTimeCryptoComparison.java:15:28:15:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
7-
| NotConstantTimeCryptoComparison.java:16:43:16:51 | actualMac | semmle.label | actualMac |
8-
| NotConstantTimeCryptoComparison.java:31:28:31:40 | sign(...) : byte[] | semmle.label | sign(...) : byte[] |
9-
| NotConstantTimeCryptoComparison.java:32:40:32:48 | signature | semmle.label | signature |
10-
| NotConstantTimeCryptoComparison.java:48:22:48:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
11-
| NotConstantTimeCryptoComparison.java:49:45:49:47 | tag | semmle.label | tag |
8+
| NonConstantTimeCryptoComparison.java:15:28:15:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
9+
| NonConstantTimeCryptoComparison.java:16:43:16:51 | actualMac | semmle.label | actualMac |
10+
| NonConstantTimeCryptoComparison.java:23:28:23:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
11+
| NonConstantTimeCryptoComparison.java:24:53:24:67 | cast(...) | semmle.label | cast(...) |
12+
| NonConstantTimeCryptoComparison.java:24:58:24:66 | actualMac : byte[] | semmle.label | actualMac : byte[] |
13+
| NonConstantTimeCryptoComparison.java:47:28:47:40 | sign(...) : byte[] | semmle.label | sign(...) : byte[] |
14+
| NonConstantTimeCryptoComparison.java:48:40:48:48 | signature | semmle.label | signature |
15+
| NonConstantTimeCryptoComparison.java:64:22:64:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
16+
| NonConstantTimeCryptoComparison.java:65:45:65:47 | tag | semmle.label | tag |
1217
#select
13-
| NotConstantTimeCryptoComparison.java:16:43:16:51 | actualMac | NotConstantTimeCryptoComparison.java:15:28:15:44 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:16:43:16:51 | actualMac | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
14-
| NotConstantTimeCryptoComparison.java:32:40:32:48 | signature | NotConstantTimeCryptoComparison.java:31:28:31:40 | sign(...) : byte[] | NotConstantTimeCryptoComparison.java:32:40:32:48 | signature | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
15-
| NotConstantTimeCryptoComparison.java:49:45:49:47 | tag | NotConstantTimeCryptoComparison.java:48:22:48:46 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:49:45:49:47 | tag | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
18+
| NonConstantTimeCryptoComparison.java:16:43:16:51 | actualMac | NonConstantTimeCryptoComparison.java:15:28:15:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:16:43:16:51 | actualMac | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
19+
| NonConstantTimeCryptoComparison.java:24:53:24:67 | cast(...) | NonConstantTimeCryptoComparison.java:23:28:23:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:24:53:24:67 | cast(...) | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
20+
| NonConstantTimeCryptoComparison.java:48:40:48:48 | signature | NonConstantTimeCryptoComparison.java:47:28:47:40 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:48:40:48:48 | signature | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
21+
| NonConstantTimeCryptoComparison.java:65:45:65:47 | tag | NonConstantTimeCryptoComparison.java:64:22:64:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:65:45:65:47 | tag | Using a non-constant time algorithm for comparing results of a cryptographic operation. |

0 commit comments

Comments
 (0)