Skip to content

Commit 5dbcf1d

Browse files
artem-smotrakovartem-smotrakov
committed
Covered Object.deepEquals() in NotConstantTimeCryptoComparison.ql
1 parent 5c474f6 commit 5dbcf1d

File tree

3 files changed

+18
-13
lines changed

3 files changed

+18
-13
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ private class NonConstantTimeCryptoComparisonConfig extends TaintTracking::Confi
5353
m.hasName("equals") and
5454
ma.getAnArgument() = sink.asExpr()
5555
or
56+
m.getDeclaringType().hasQualifiedName("java.util", "Objects") and
57+
m.hasName("deepEquals") and
58+
ma.getAnArgument() = sink.asExpr()
59+
or
5660
m.getDeclaringType().hasQualifiedName("org.apache.commons.lang3", "StringUtils") and
5761
m.hasName(["equals", "equalsAny", "equalsAnyIgnoreCase", "equalsIgnoreCase"]) and
5862
ma.getAnArgument() = sink.asExpr()

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
edges
2-
| NotConstantTimeCryptoComparison.java:14:28:14:44 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:15:43:15:51 | actualMac |
3-
| NotConstantTimeCryptoComparison.java:30:28:30:40 | sign(...) : byte[] | NotConstantTimeCryptoComparison.java:31:40:31:48 | signature |
4-
| NotConstantTimeCryptoComparison.java:47:22:47:46 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:48:40:48:42 | tag |
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 |
55
nodes
6-
| NotConstantTimeCryptoComparison.java:14:28:14:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
7-
| NotConstantTimeCryptoComparison.java:15:43:15:51 | actualMac | semmle.label | actualMac |
8-
| NotConstantTimeCryptoComparison.java:30:28:30:40 | sign(...) : byte[] | semmle.label | sign(...) : byte[] |
9-
| NotConstantTimeCryptoComparison.java:31:40:31:48 | signature | semmle.label | signature |
10-
| NotConstantTimeCryptoComparison.java:47:22:47:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
11-
| NotConstantTimeCryptoComparison.java:48:40:48:42 | tag | semmle.label | tag |
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 |
1212
#select
13-
| NotConstantTimeCryptoComparison.java:15:43:15:51 | actualMac | NotConstantTimeCryptoComparison.java:14:28:14:44 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:15:43:15:51 | actualMac | Using a not-constant time algorithm for comparison results of a cryptographic operation. |
14-
| NotConstantTimeCryptoComparison.java:31:40:31:48 | signature | NotConstantTimeCryptoComparison.java:30:28:30:40 | sign(...) : byte[] | NotConstantTimeCryptoComparison.java:31:40:31:48 | signature | Using a not-constant time algorithm for comparison results of a cryptographic operation. |
15-
| NotConstantTimeCryptoComparison.java:48:40:48:42 | tag | NotConstantTimeCryptoComparison.java:47:22:47:46 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:48:40:48:42 | tag | Using a not-constant time algorithm for comparison results of a cryptographic operation. |
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. |

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.security.PrivateKey;
44
import java.security.Signature;
55
import java.util.Arrays;
6+
import java.util.Objects;
67
import javax.crypto.Cipher;
78
import javax.crypto.Mac;
89

@@ -45,7 +46,7 @@ public boolean unsafeCheckCustomMac(byte[] expected, byte[] plaintext, Key key)
4546
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
4647
cipher.init(Cipher.ENCRYPT_MODE, key);
4748
byte[] tag = cipher.doFinal(plaintext);
48-
return Arrays.equals(expected, tag);
49+
return Objects.deepEquals(expected, tag);
4950
}
5051

5152
// GOOD: compare ciphertexts using a constant time method

0 commit comments

Comments
 (0)