mirror of
https://github.com/github/codeql.git
synced 2026-04-29 18:55:14 +02:00
More sinks for java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCryptoComparison.ql
This commit is contained in:
committed by
Fosstars
parent
dfa3b523d0
commit
8e6d227dc0
@@ -8,7 +8,6 @@
|
||||
* @precision high
|
||||
* @id java/non-constant-time-crypto-comparison
|
||||
* @tags security
|
||||
* external/cwe/cwe-385
|
||||
* external/cwe/cwe-208
|
||||
*/
|
||||
|
||||
@@ -17,19 +16,97 @@ import semmle.code.java.dataflow.TaintTracking
|
||||
import DataFlow::PathGraph
|
||||
|
||||
/**
|
||||
* A method that returns the result of a cryptographic operation
|
||||
* such as encryption, decryption, signing, etc.
|
||||
* A source that produces a MAC.
|
||||
*/
|
||||
private class ReturnCryptoOperationResultMethod extends Method {
|
||||
ReturnCryptoOperationResultMethod() {
|
||||
getDeclaringType().hasQualifiedName("javax.crypto", ["Mac", "Cipher"]) and
|
||||
hasName("doFinal")
|
||||
or
|
||||
getDeclaringType().hasQualifiedName("java.security", "Signature") and
|
||||
hasName("sign")
|
||||
private class MacSource extends DataFlow::Node {
|
||||
MacSource() {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
m.hasQualifiedName("javax.crypto", "Mac", "doFinal") and
|
||||
(
|
||||
m.getReturnType() instanceof Array and ma = this.asExpr()
|
||||
or
|
||||
m.getParameterType(0) instanceof Array and ma.getArgument(0) = this.asExpr()
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A source that produces a signature.
|
||||
*/
|
||||
private class SignatureSource extends DataFlow::Node {
|
||||
SignatureSource() {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
m.hasQualifiedName("java.security", "Signature", "sign") and
|
||||
(
|
||||
m.getReturnType() instanceof Array and ma = this.asExpr()
|
||||
or
|
||||
m.getParameterType(0) instanceof Array and ma.getArgument(0) = this.asExpr()
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A source that produces a ciphertext.
|
||||
*/
|
||||
private class CiphertextSource extends DataFlow::Node {
|
||||
CiphertextSource() {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
m.hasQualifiedName("javax.crypto", "Cipher", "doFinal") and
|
||||
(
|
||||
m.getReturnType() instanceof Array and ma = this.asExpr()
|
||||
or
|
||||
m.getParameterType([0, 3]) instanceof Array and ma.getArgument([0, 3]) = this.asExpr()
|
||||
or
|
||||
m.getParameterType(1).(RefType).hasQualifiedName("java.nio", "ByteBuffer") and
|
||||
ma.getArgument(1) = this.asExpr()
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A sink that compares input using a non-constant time algorithm.
|
||||
*/
|
||||
private class KnownNonConstantTimeComparisonSink extends DataFlow::Node {
|
||||
KnownNonConstantTimeComparisonSink() {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
m.hasQualifiedName("java.lang", "String", ["equals", "contentEquals", "equalsIgnoreCase"]) and
|
||||
this.asExpr() = [ma.getQualifier(), ma.getAnArgument()]
|
||||
or
|
||||
m.hasQualifiedName("java.nio", "ByteBuffer", ["equals", "compareTo"]) and
|
||||
this.asExpr() = [ma.getQualifier(), ma.getAnArgument()]
|
||||
or
|
||||
m.hasQualifiedName("java.util", "Arrays", ["equals", "deepEquals"]) and
|
||||
ma.getAnArgument() = this.asExpr()
|
||||
or
|
||||
m.hasQualifiedName("java.util", "Objects", "deepEquals") and
|
||||
ma.getAnArgument() = this.asExpr()
|
||||
or
|
||||
m.hasQualifiedName("org.apache.commons.lang3", "StringUtils",
|
||||
["equals", "equalsAny", "equalsAnyIgnoreCase", "equalsIgnoreCase"]) and
|
||||
ma.getAnArgument() = this.asExpr()
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `fromNode` to `toNode` is a dataflow step
|
||||
* that converts a `ByteBuffer` to a byte array and vice versa.
|
||||
*/
|
||||
private predicate convertByteBufferStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
m.hasQualifiedName("java.nio", "ByteBuffer", "array") and
|
||||
ma.getQualifier() = fromNode.asExpr() and
|
||||
ma = toNode.asExpr()
|
||||
or
|
||||
m.hasQualifiedName("java.nio", "ByteBuffer", "wrap") and
|
||||
ma.getAnArgument() = fromNode.asExpr() and
|
||||
ma = toNode.asExpr()
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* A configuration that tracks data flow from cryptographic operations
|
||||
* to methods that compare data using a non-constant time algorithm.
|
||||
@@ -38,29 +115,19 @@ private class NonConstantTimeCryptoComparisonConfig extends TaintTracking::Confi
|
||||
NonConstantTimeCryptoComparisonConfig() { this = "NonConstantTimeCryptoComparisonConfig" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) {
|
||||
exists(MethodAccess ma | ma.getMethod() instanceof ReturnCryptoOperationResultMethod |
|
||||
ma = source.asExpr()
|
||||
)
|
||||
source instanceof MacSource
|
||||
or
|
||||
source instanceof SignatureSource
|
||||
or
|
||||
source instanceof CiphertextSource
|
||||
}
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
m.getDeclaringType() instanceof TypeString and
|
||||
m.hasName(["equals", "contentEquals", "equalsIgnoreCase"]) and
|
||||
sink.asExpr() = [ma.getQualifier(), ma.getAnArgument()]
|
||||
or
|
||||
m.getDeclaringType().hasQualifiedName("java.util", "Arrays") and
|
||||
m.hasName(["equals", "deepEquals"]) and
|
||||
ma.getAnArgument() = sink.asExpr()
|
||||
or
|
||||
m.getDeclaringType().hasQualifiedName("java.util", "Objects") and
|
||||
m.hasName("deepEquals") and
|
||||
ma.getAnArgument() = sink.asExpr()
|
||||
or
|
||||
m.getDeclaringType().hasQualifiedName("org.apache.commons.lang3", "StringUtils") and
|
||||
m.hasName(["equals", "equalsAny", "equalsAnyIgnoreCase", "equalsIgnoreCase"]) and
|
||||
ma.getAnArgument() = sink.asExpr()
|
||||
)
|
||||
sink instanceof KnownNonConstantTimeComparisonSink
|
||||
}
|
||||
|
||||
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
|
||||
convertByteBufferStep(fromNode, toNode)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,21 +1,41 @@
|
||||
edges
|
||||
| NonConstantTimeCryptoComparison.java:15:28:15:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:16:43:16:51 | actualMac |
|
||||
| NonConstantTimeCryptoComparison.java:23:28:23:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:24:58:24:66 | actualMac : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:24:58:24:66 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:24:53:24:67 | cast(...) |
|
||||
| NonConstantTimeCryptoComparison.java:16:28:16:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:17:43:17:51 | actualMac |
|
||||
| NonConstantTimeCryptoComparison.java:23:28:23:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:24:84:24:92 | actualMac : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:24:84:24:92 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:24:66:24:93 | castToObjectArray(...) |
|
||||
| NonConstantTimeCryptoComparison.java:31:21:31:29 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:32:43:32:51 | actualMac |
|
||||
| NonConstantTimeCryptoComparison.java:47:28:47:40 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:48:40:48:48 | signature |
|
||||
| NonConstantTimeCryptoComparison.java:64:22:64:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:65:45:65:47 | tag |
|
||||
| NonConstantTimeCryptoComparison.java:56:21:56:29 | signature : byte[] | NonConstantTimeCryptoComparison.java:57:40:57:48 | signature |
|
||||
| NonConstantTimeCryptoComparison.java:73:22:73:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:74:45:74:47 | tag |
|
||||
| NonConstantTimeCryptoComparison.java:83:24:83:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:84:40:84:42 | tag |
|
||||
| NonConstantTimeCryptoComparison.java:93:52:93:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:94:40:94:50 | array(...) |
|
||||
| NonConstantTimeCryptoComparison.java:103:52:103:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:104:49:104:51 | tag |
|
||||
nodes
|
||||
| NonConstantTimeCryptoComparison.java:15:28:15:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:16:43:16:51 | actualMac | semmle.label | actualMac |
|
||||
| NonConstantTimeCryptoComparison.java:16:28:16:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:17:43:17:51 | actualMac | semmle.label | actualMac |
|
||||
| NonConstantTimeCryptoComparison.java:23:28:23:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:24:53:24:67 | cast(...) | semmle.label | cast(...) |
|
||||
| NonConstantTimeCryptoComparison.java:24:58:24:66 | actualMac : byte[] | semmle.label | actualMac : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:24:66:24:93 | castToObjectArray(...) | semmle.label | castToObjectArray(...) |
|
||||
| NonConstantTimeCryptoComparison.java:24:84:24:92 | actualMac : byte[] | semmle.label | actualMac : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:31:21:31:29 | actualMac : byte[] | semmle.label | actualMac : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:32:43:32:51 | actualMac | semmle.label | actualMac |
|
||||
| NonConstantTimeCryptoComparison.java:47:28:47:40 | sign(...) : byte[] | semmle.label | sign(...) : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:48:40:48:48 | signature | semmle.label | signature |
|
||||
| NonConstantTimeCryptoComparison.java:64:22:64:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:65:45:65:47 | tag | semmle.label | tag |
|
||||
| NonConstantTimeCryptoComparison.java:56:21:56:29 | signature : byte[] | semmle.label | signature : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:57:40:57:48 | signature | semmle.label | signature |
|
||||
| NonConstantTimeCryptoComparison.java:73:22:73:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:74:45:74:47 | tag | semmle.label | tag |
|
||||
| NonConstantTimeCryptoComparison.java:83:24:83:26 | tag : byte[] | semmle.label | tag : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:84:40:84:42 | tag | semmle.label | tag |
|
||||
| NonConstantTimeCryptoComparison.java:93:52:93:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
|
||||
| NonConstantTimeCryptoComparison.java:94:40:94:50 | array(...) | semmle.label | array(...) |
|
||||
| NonConstantTimeCryptoComparison.java:103:52:103:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
|
||||
| NonConstantTimeCryptoComparison.java:104:49:104:51 | tag | semmle.label | tag |
|
||||
#select
|
||||
| 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. |
|
||||
| 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. |
|
||||
| NonConstantTimeCryptoComparison.java:17:43:17:51 | actualMac | NonConstantTimeCryptoComparison.java:16:28:16:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:17:43:17:51 | actualMac | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:24:66:24:93 | castToObjectArray(...) | NonConstantTimeCryptoComparison.java:23:28:23:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:24:66:24:93 | castToObjectArray(...) | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:32:43:32:51 | actualMac | NonConstantTimeCryptoComparison.java:31:21:31:29 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:32:43:32:51 | actualMac | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
|
||||
| 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. |
|
||||
| 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. |
|
||||
| NonConstantTimeCryptoComparison.java:57:40:57:48 | signature | NonConstantTimeCryptoComparison.java:56:21:56:29 | signature : byte[] | NonConstantTimeCryptoComparison.java:57:40:57:48 | signature | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:74:45:74:47 | tag | NonConstantTimeCryptoComparison.java:73:22:73:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:74:45:74:47 | tag | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:84:40:84:42 | tag | NonConstantTimeCryptoComparison.java:83:24:83:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:84:40:84:42 | tag | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:94:40:94:50 | array(...) | NonConstantTimeCryptoComparison.java:93:52:93:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:94:40:94:50 | array(...) | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:104:49:104:51 | tag | NonConstantTimeCryptoComparison.java:103:52:103:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:104:49:104:51 | tag | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import java.nio.ByteBuffer;
|
||||
import java.security.Key;
|
||||
import java.security.MessageDigest;
|
||||
import java.security.PrivateKey;
|
||||
@@ -10,26 +11,25 @@ import javax.crypto.Mac;
|
||||
public class NonConstantTimeCryptoComparison {
|
||||
|
||||
// BAD: compare MACs using a non-constant time method
|
||||
public boolean unsafeMacCheck(byte[] expectedMac, byte[] data) throws Exception {
|
||||
public boolean unsafeMacCheckWithArrayEquals(byte[] expectedMac, byte[] data) throws Exception {
|
||||
Mac mac = Mac.getInstance("HmacSHA256");
|
||||
byte[] actualMac = mac.doFinal(data);
|
||||
return Arrays.equals(expectedMac, actualMac);
|
||||
}
|
||||
|
||||
|
||||
// BAD: compare MACs using a non-constant time method
|
||||
public boolean unsafeMacCheckWithArraysDeepEquals(byte[] expectedMac, byte[] data) throws Exception {
|
||||
Mac mac = Mac.getInstance("HmacSHA256");
|
||||
byte[] actualMac = mac.doFinal(data);
|
||||
return Arrays.deepEquals(cast(expectedMac), cast(actualMac));
|
||||
return Arrays.deepEquals(castToObjectArray(expectedMac), castToObjectArray(actualMac));
|
||||
}
|
||||
|
||||
private static Object[] cast(byte[] array) {
|
||||
Object[] result = new Object[array.length];
|
||||
for (int i = 0; i < array.length; i++) {
|
||||
result[i] = array[i];
|
||||
}
|
||||
return result;
|
||||
// BAD: compare MACs using a non-constant time method
|
||||
public boolean unsafeMacCheckWithDoFinalWithOutputArray(byte[] expectedMac, byte[] data) throws Exception {
|
||||
Mac mac = Mac.getInstance("HmacSHA256");
|
||||
byte[] actualMac = new byte[256];
|
||||
mac.doFinal(actualMac, 0);
|
||||
return Arrays.equals(expectedMac, actualMac);
|
||||
}
|
||||
|
||||
// GOOD: compare MACs using a constant time method
|
||||
@@ -48,6 +48,15 @@ public class NonConstantTimeCryptoComparison {
|
||||
return Arrays.equals(expected, signature);
|
||||
}
|
||||
|
||||
// BAD: compare signatures using a non-constant time method
|
||||
public boolean unsafeCheckSignaturesWithOutputArray(byte[] expected, byte[] data, PrivateKey key) throws Exception {
|
||||
Signature engine = Signature.getInstance("SHA256withRSA");
|
||||
engine.initSign(key);
|
||||
byte[] signature = new byte[1024];
|
||||
engine.sign(signature, 0, 1024);
|
||||
return Arrays.equals(expected, signature);
|
||||
}
|
||||
|
||||
// GOOD: compare signatures using a constant time method
|
||||
public boolean saferCheckSignatures(byte[] expected, byte[] data, PrivateKey key) throws Exception {
|
||||
Signature engine = Signature.getInstance("SHA256withRSA");
|
||||
@@ -58,19 +67,57 @@ public class NonConstantTimeCryptoComparison {
|
||||
}
|
||||
|
||||
// BAD: compare ciphertexts using a non-constant time method
|
||||
public boolean unsafeCheckCustomMac(byte[] expected, byte[] plaintext, Key key) throws Exception {
|
||||
public boolean unsafeCheckCiphertext(byte[] expected, byte[] plaintext, Key key) throws Exception {
|
||||
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
|
||||
cipher.init(Cipher.ENCRYPT_MODE, key);
|
||||
byte[] tag = cipher.doFinal(plaintext);
|
||||
return Objects.deepEquals(expected, tag);
|
||||
}
|
||||
|
||||
// BAD: compare ciphertexts using a non-constant time method
|
||||
public boolean unsafeCheckCiphertextWithOutputArray(byte[] expected, byte[] plaintext, Key key) throws Exception {
|
||||
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
|
||||
cipher.init(Cipher.ENCRYPT_MODE, key);
|
||||
cipher.update(plaintext);
|
||||
byte[] tag = new byte[1024];
|
||||
cipher.doFinal(tag, 0);
|
||||
return Arrays.equals(expected, tag);
|
||||
}
|
||||
|
||||
// BAD: compare ciphertexts using a non-constant time method
|
||||
public boolean unsafeCheckCiphertextWithByteBuffer(byte[] expected, byte[] plaintext, Key key) throws Exception {
|
||||
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
|
||||
cipher.init(Cipher.ENCRYPT_MODE, key);
|
||||
cipher.update(plaintext);
|
||||
ByteBuffer tag = ByteBuffer.wrap(new byte[1024]);
|
||||
cipher.doFinal(ByteBuffer.wrap(plaintext), tag);
|
||||
return Arrays.equals(expected, tag.array());
|
||||
}
|
||||
|
||||
// BAD: compare ciphertexts using a non-constant time method
|
||||
public boolean unsafeCheckCiphertextWithByteBufferEquals(byte[] expected, byte[] plaintext, Key key) throws Exception {
|
||||
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
|
||||
cipher.init(Cipher.ENCRYPT_MODE, key);
|
||||
cipher.update(plaintext);
|
||||
ByteBuffer tag = ByteBuffer.wrap(new byte[1024]);
|
||||
cipher.doFinal(ByteBuffer.wrap(plaintext), tag);
|
||||
return ByteBuffer.wrap(expected).equals(tag);
|
||||
}
|
||||
|
||||
// GOOD: compare ciphertexts using a constant time method
|
||||
public boolean saferCheckCustomMac(byte[] expected, byte[] plaintext, Key key) throws Exception {
|
||||
public boolean saferCheckCiphertext(byte[] expected, byte[] plaintext, Key key) throws Exception {
|
||||
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
|
||||
cipher.init(Cipher.ENCRYPT_MODE, key);
|
||||
byte[] tag = cipher.doFinal(plaintext);
|
||||
return MessageDigest.isEqual(expected, tag);
|
||||
}
|
||||
|
||||
private static Object[] castToObjectArray(byte[] array) {
|
||||
Object[] result = new Object[array.length];
|
||||
for (int i = 0; i < array.length; i++) {
|
||||
result[i] = array[i];
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
}
|
||||
Reference in New Issue
Block a user