mirror of
https://github.com/github/codeql.git
synced 2026-04-30 11:15:13 +02:00
Added taint propagation steps for hashes in NonConstantTimeCryptoComparison.ql
This commit is contained in:
Artem Smotrakov
committed by
Fosstars
Fosstars
parent
c96d939cf5
commit
1f2a9cdda7
@@ -75,6 +75,55 @@ private class ProduceCiphertextCall extends ProduceCryptoCall {
|
||||
}
|
||||
}
|
||||
|
||||
/** Holds if `fromNode` to `toNode` is a dataflow step that updates a cryptographic operation. */
|
||||
private predicate updateCryptoOperationStep(DataFlow2::Node fromNode, DataFlow2::Node toNode) {
|
||||
exists(MethodAccess call, Method m |
|
||||
m = call.getMethod() and
|
||||
call.getQualifier() = toNode.asExpr() and
|
||||
call.getArgument(0) = fromNode.asExpr()
|
||||
|
|
||||
m.hasQualifiedName("java.security", "Signature", "update")
|
||||
or
|
||||
m.hasQualifiedName("javax.crypto", ["Mac", "Cipher"], "update")
|
||||
or
|
||||
m.hasQualifiedName("javax.crypto", ["Mac", "Cipher"], "doFinal") and
|
||||
not m.hasStringSignature("doFinal(byte[], int)")
|
||||
)
|
||||
}
|
||||
|
||||
/** Holds if `fromNode` to `toNode` is a dataflow step that creates a hash. */
|
||||
private predicate createMessageDigestStep(DataFlow2::Node fromNode, DataFlow2::Node toNode) {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
m.getDeclaringType().hasQualifiedName("java.security", "MessageDigest") and
|
||||
m.hasStringSignature("digest()") and
|
||||
ma.getQualifier() = fromNode.asExpr() and
|
||||
ma = toNode.asExpr()
|
||||
)
|
||||
or
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
m.getDeclaringType().hasQualifiedName("java.security", "MessageDigest") and
|
||||
m.hasStringSignature("digest(byte[], int, int)") and
|
||||
ma.getQualifier() = fromNode.asExpr() and
|
||||
ma.getArgument(0) = toNode.asExpr()
|
||||
)
|
||||
or
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
m.getDeclaringType().hasQualifiedName("java.security", "MessageDigest") and
|
||||
m.hasStringSignature("digest(byte[])") and
|
||||
ma.getArgument(0) = fromNode.asExpr() and
|
||||
ma = toNode.asExpr()
|
||||
)
|
||||
}
|
||||
|
||||
/** Holds if `fromNode` to `toNode` is a dataflow step that updates a hash. */
|
||||
private predicate updateMessageDigestStep(DataFlow2::Node fromNode, DataFlow2::Node toNode) {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
m.hasQualifiedName("java.security", "MessageDigest", "update") and
|
||||
ma.getArgument(0) = fromNode.asExpr() and
|
||||
ma.getQualifier() = toNode.asExpr()
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* A config that tracks data flow from remote user input to a cryptographic operation
|
||||
* such as cipher, MAC or signature.
|
||||
@@ -88,20 +137,12 @@ private class UserInputInCryptoOperationConfig extends TaintTracking2::Configura
|
||||
exists(ProduceCryptoCall call | call.getQualifier() = sink.asExpr())
|
||||
}
|
||||
|
||||
/** Holds if `fromNode` to `toNode` is a dataflow step that updates a cryptographic operation. */
|
||||
override predicate isAdditionalTaintStep(DataFlow2::Node fromNode, DataFlow2::Node toNode) {
|
||||
exists(MethodAccess call, Method m |
|
||||
m = call.getMethod() and
|
||||
call.getQualifier() = toNode.asExpr() and
|
||||
call.getArgument(0) = fromNode.asExpr()
|
||||
|
|
||||
m.hasQualifiedName("java.security", "Signature", "update")
|
||||
or
|
||||
m.hasQualifiedName("javax.crypto", ["Mac", "Cipher"], "update")
|
||||
or
|
||||
m.hasQualifiedName("javax.crypto", ["Mac", "Cipher"], "doFinal") and
|
||||
not m.hasStringSignature("doFinal(byte[], int)")
|
||||
)
|
||||
updateCryptoOperationStep(fromNode, toNode)
|
||||
or
|
||||
createMessageDigestStep(fromNode, toNode)
|
||||
or
|
||||
updateMessageDigestStep(fromNode, toNode)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -5,13 +5,13 @@ edges
|
||||
| NonConstantTimeCryptoComparison.java:37:21:37:29 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:39:43:39:51 | actualMac |
|
||||
| NonConstantTimeCryptoComparison.java:57:28:57:40 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:58:40:58:48 | signature |
|
||||
| NonConstantTimeCryptoComparison.java:68:21:68:29 | signature : byte[] | NonConstantTimeCryptoComparison.java:69:40:69:48 | signature |
|
||||
| NonConstantTimeCryptoComparison.java:86:22:86:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:88:45:88:47 | tag |
|
||||
| NonConstantTimeCryptoComparison.java:98:24:98:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:99:40:99:42 | tag |
|
||||
| NonConstantTimeCryptoComparison.java:108:52:108:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:110:40:110:42 | tag : ByteBuffer |
|
||||
| NonConstantTimeCryptoComparison.java:110:40:110:42 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:110:40:110:50 | array(...) |
|
||||
| NonConstantTimeCryptoComparison.java:120:52:120:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:121:49:121:51 | tag |
|
||||
| NonConstantTimeCryptoComparison.java:137:22:137:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:138:40:138:42 | tag |
|
||||
| NonConstantTimeCryptoComparison.java:168:34:168:50 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:171:26:171:36 | computedTag |
|
||||
| NonConstantTimeCryptoComparison.java:87:22:87:41 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:89:45:89:47 | tag |
|
||||
| NonConstantTimeCryptoComparison.java:102:24:102:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:103:40:103:42 | tag |
|
||||
| NonConstantTimeCryptoComparison.java:116:52:116:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:118:40:118:42 | tag : ByteBuffer |
|
||||
| NonConstantTimeCryptoComparison.java:118:40:118:42 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:118:40:118:50 | array(...) |
|
||||
| NonConstantTimeCryptoComparison.java:128:52:128:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:129:49:129:51 | tag |
|
||||
| NonConstantTimeCryptoComparison.java:146:22:146:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:147:40:147:42 | tag |
|
||||
| NonConstantTimeCryptoComparison.java:177:34:177:50 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:180:26:180:36 | computedTag |
|
||||
nodes
|
||||
| NonConstantTimeCryptoComparison.java:20:28:20:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:21:43:21:51 | actualMac | semmle.label | actualMac |
|
||||
@@ -24,27 +24,27 @@ nodes
|
||||
| NonConstantTimeCryptoComparison.java:58:40:58:48 | signature | semmle.label | signature |
|
||||
| NonConstantTimeCryptoComparison.java:68:21:68:29 | signature : byte[] | semmle.label | signature : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:69:40:69:48 | signature | semmle.label | signature |
|
||||
| NonConstantTimeCryptoComparison.java:86:22:86:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:88:45:88:47 | tag | semmle.label | tag |
|
||||
| NonConstantTimeCryptoComparison.java:98:24:98:26 | tag : byte[] | semmle.label | tag : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:99:40:99:42 | tag | semmle.label | tag |
|
||||
| NonConstantTimeCryptoComparison.java:108:52:108:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
|
||||
| NonConstantTimeCryptoComparison.java:110:40:110:42 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
|
||||
| NonConstantTimeCryptoComparison.java:110:40:110:50 | array(...) | semmle.label | array(...) |
|
||||
| NonConstantTimeCryptoComparison.java:120:52:120:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
|
||||
| NonConstantTimeCryptoComparison.java:121:49:121:51 | tag | semmle.label | tag |
|
||||
| NonConstantTimeCryptoComparison.java:137:22:137:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:138:40:138:42 | tag | semmle.label | tag |
|
||||
| NonConstantTimeCryptoComparison.java:168:34:168:50 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:171:26:171:36 | computedTag | semmle.label | computedTag |
|
||||
| NonConstantTimeCryptoComparison.java:87:22:87:41 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:89:45:89:47 | tag | semmle.label | tag |
|
||||
| NonConstantTimeCryptoComparison.java:102:24:102:26 | tag : byte[] | semmle.label | tag : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:103:40:103:42 | tag | semmle.label | tag |
|
||||
| NonConstantTimeCryptoComparison.java:116:52:116:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
|
||||
| NonConstantTimeCryptoComparison.java:118:40:118:42 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
|
||||
| NonConstantTimeCryptoComparison.java:118:40:118:50 | array(...) | semmle.label | array(...) |
|
||||
| NonConstantTimeCryptoComparison.java:128:52:128:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
|
||||
| NonConstantTimeCryptoComparison.java:129:49:129:51 | tag | semmle.label | tag |
|
||||
| NonConstantTimeCryptoComparison.java:146:22:146:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:147:40:147:42 | tag | semmle.label | tag |
|
||||
| NonConstantTimeCryptoComparison.java:177:34:177:50 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
|
||||
| NonConstantTimeCryptoComparison.java:180:26:180:36 | computedTag | semmle.label | computedTag |
|
||||
#select
|
||||
| NonConstantTimeCryptoComparison.java:21:43:21:51 | actualMac | NonConstantTimeCryptoComparison.java:20:28:20:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:21:43:21:51 | actualMac | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:30:66:30:93 | castToObjectArray(...) | NonConstantTimeCryptoComparison.java:29:28:29:40 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:30:66:30:93 | castToObjectArray(...) | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:39:43:39:51 | actualMac | NonConstantTimeCryptoComparison.java:37:21:37:29 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:39:43:39:51 | actualMac | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:58:40:58:48 | signature | NonConstantTimeCryptoComparison.java:57:28:57:40 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:58:40:58:48 | signature | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:69:40:69:48 | signature | NonConstantTimeCryptoComparison.java:68:21:68:29 | signature : byte[] | NonConstantTimeCryptoComparison.java:69:40:69:48 | signature | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:88:45:88:47 | tag | NonConstantTimeCryptoComparison.java:86:22:86:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:88:45:88:47 | tag | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:99:40:99:42 | tag | NonConstantTimeCryptoComparison.java:98:24:98:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:99:40:99:42 | tag | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:110:40:110:50 | array(...) | NonConstantTimeCryptoComparison.java:108:52:108:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:110:40:110:50 | array(...) | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:121:49:121:51 | tag | NonConstantTimeCryptoComparison.java:120:52:120:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:121:49:121:51 | tag | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:171:26:171:36 | computedTag | NonConstantTimeCryptoComparison.java:168:34:168:50 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:171:26:171:36 | computedTag | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:89:45:89:47 | tag | NonConstantTimeCryptoComparison.java:87:22:87:41 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:89:45:89:47 | tag | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:103:40:103:42 | tag | NonConstantTimeCryptoComparison.java:102:24:102:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:103:40:103:42 | tag | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:118:40:118:50 | array(...) | NonConstantTimeCryptoComparison.java:116:52:116:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:118:40:118:50 | array(...) | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:129:49:129:51 | tag | NonConstantTimeCryptoComparison.java:128:52:128:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:129:49:129:51 | tag | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
| NonConstantTimeCryptoComparison.java:180:26:180:36 | computedTag | NonConstantTimeCryptoComparison.java:177:34:177:50 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:180:26:180:36 | computedTag | Using a non-constant-time algorithm for comparing results of a cryptographic operation. |
|
||||
|
||||
@@ -79,38 +79,46 @@ public class NonConstantTimeCryptoComparison {
|
||||
return MessageDigest.isEqual(expected, signature);
|
||||
}
|
||||
|
||||
// BAD: compare ciphertexts using a non-constant-time method
|
||||
// BAD: compare ciphertexts (custom MAC) using a non-constant-time method
|
||||
public boolean unsafeCheckCiphertext(Socket socket, byte[] plaintext, Key key) throws Exception {
|
||||
byte[] hash = MessageDigest.getInstance("SHA-256").digest(plaintext);
|
||||
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
|
||||
cipher.init(Cipher.ENCRYPT_MODE, key);
|
||||
byte[] tag = cipher.doFinal(plaintext);
|
||||
byte[] tag = cipher.doFinal(hash);
|
||||
byte[] expected = socket.getInputStream().readAllBytes();
|
||||
return Objects.deepEquals(expected, tag);
|
||||
}
|
||||
|
||||
// BAD: compare ciphertexts using a non-constant-time method
|
||||
// BAD: compare ciphertexts (custom MAC) using a non-constant-time method
|
||||
public boolean unsafeCheckCiphertextWithOutputArray(byte[] expected, Socket socket, Key key) throws Exception {
|
||||
byte[] plaintext = socket.getInputStream().readAllBytes();
|
||||
MessageDigest md = MessageDigest.getInstance("SHA-512");
|
||||
md.update(plaintext);
|
||||
byte[] hash = md.digest();
|
||||
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
|
||||
cipher.init(Cipher.ENCRYPT_MODE, key);
|
||||
byte[] plaintext = socket.getInputStream().readAllBytes();
|
||||
cipher.update(plaintext);
|
||||
cipher.update(hash);
|
||||
byte[] tag = new byte[1024];
|
||||
cipher.doFinal(tag, 0);
|
||||
return Arrays.equals(expected, tag);
|
||||
}
|
||||
|
||||
// BAD: compare ciphertexts using a non-constant-time method
|
||||
// BAD: compare ciphertexts (custom MAC) using a non-constant-time method
|
||||
public boolean unsafeCheckCiphertextWithByteBuffer(Socket socket, byte[] plaintext, Key key) throws Exception {
|
||||
MessageDigest md = MessageDigest.getInstance("SHA-512");
|
||||
md.update(plaintext);
|
||||
byte[] hash = new byte[1024];
|
||||
md.digest(hash, 0, hash.length);
|
||||
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
|
||||
cipher.init(Cipher.ENCRYPT_MODE, key);
|
||||
cipher.update(plaintext);
|
||||
cipher.update(hash);
|
||||
ByteBuffer tag = ByteBuffer.wrap(new byte[1024]);
|
||||
cipher.doFinal(ByteBuffer.wrap(plaintext), tag);
|
||||
byte[] expected = socket.getInputStream().readNBytes(1024);
|
||||
return Arrays.equals(expected, tag.array());
|
||||
}
|
||||
|
||||
// BAD: compare ciphertexts using a non-constant-time method
|
||||
// BAD: compare ciphertexts (custom MAC) using a non-constant-time method
|
||||
public boolean unsafeCheckCiphertextWithByteBufferEquals(byte[] expected, Socket socket, Key key) throws Exception {
|
||||
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
|
||||
cipher.init(Cipher.ENCRYPT_MODE, key);
|
||||
@@ -121,11 +129,12 @@ public class NonConstantTimeCryptoComparison {
|
||||
return ByteBuffer.wrap(expected).equals(tag);
|
||||
}
|
||||
|
||||
// GOOD: compare ciphertexts using a constant-time method
|
||||
// GOOD: compare ciphertexts (custom MAC) using a constant-time method
|
||||
public boolean saferCheckCiphertext(Socket socket, byte[] plaintext, Key key) throws Exception {
|
||||
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
|
||||
cipher.init(Cipher.ENCRYPT_MODE, key);
|
||||
byte[] tag = cipher.doFinal(plaintext);
|
||||
byte[] hash = MessageDigest.getInstance("SHA-256").digest(plaintext);
|
||||
byte[] tag = cipher.doFinal(hash);
|
||||
byte[] expected = socket.getInputStream().readAllBytes();
|
||||
return MessageDigest.isEqual(expected, tag);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user