Skip to content

Commit 8b55776

Browse files
artem-smotrakovartem-smotrakov
committed
Narrow NonConstantTimeCryptoComparison.ql to timing attack on signatures and MACs only
1 parent c359852 commit 8b55776

File tree

4 files changed

+218
-168
lines changed

4 files changed

+218
-168
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCryptoComparison.qhelp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,16 @@
33

44
<overview>
55
<p>
6-
When comparing results of cryptographic operations, such as MAC or digital signature,
7-
a constant-time algorithm should be used. In other words, the comparison time should not depend on
8-
the content of the input. Otherwise, attackers may be able to implement a timing attack
9-
if they can control input. A successful timing attack may result in leaking secrets or authentication bypass.
6+
A constant-time algorithm should be used for checking a MAC or a digital signature.
7+
In other words, the comparison time should not depend on the content of the input.
8+
Otherwise, attackers may be able to implement a timing attack if they control inputs.
9+
A successful attack may uncover a valid MAC or signature that in turn can result in authentication bypass.
1010
</p>
1111
</overview>
1212

1313
<recommendation>
1414
<p>
15-
Use <code>MessageDigest.isEqual()</code> method to compare results of cryptographic operations.
15+
Use <code>MessageDigest.isEqual()</code> method to check MACs and signatures.
1616
If this method is used, then the calculation time depends only on the length of input byte arrays,
1717
and does not depend on the contents of the arrays.
1818
</p>

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

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
/**
2-
* @name Using a non-constant-time algorithm for comparing results of a cryptographic operation
3-
* @description When comparing results of a cryptographic operation, a constant-time algorithm should be used.
4-
* Otherwise, attackers may be able to implement a timing attack if they can control input.
5-
* A successful attack may result in leaking secrets or authentication bypass.
2+
* @name Using a non-constant-time algorithm for comparing MAC or signature
3+
* @description When checking MAC or signature, a constant-time algorithm should be used.
4+
* Otherwise, attackers may be able to implement a timing attack if they control inputs.
5+
* A successful attack may uncover a valid MAC or signature that in turn can result in authentication bypass.
66
* @kind path-problem
77
* @problem.severity warning
88
* @precision high
9-
* @id java/non-constant-time-crypto-comparison
9+
* @id java/non-constant-time-in-signature-check
1010
* @tags security
1111
* external/cwe/cwe-208
1212
*/
@@ -25,6 +25,9 @@ abstract private class ProduceCryptoCall extends MethodAccess {
2525

2626
/** Return the result of cryptographic operation. */
2727
Expr output() { result = output }
28+
29+
/** Return a type of the result of cryptographic operation such as MAC, signature or ciphertext. */
30+
abstract string getResultType();
2831
}
2932

3033
/** A method call that produces a MAC. */
@@ -37,6 +40,8 @@ private class ProduceMacCall extends ProduceCryptoCall {
3740
getMethod().hasStringSignature("doFinal(byte[], int)") and getArgument(0) = output
3841
)
3942
}
43+
44+
override string getResultType() { result = "MAC" }
4045
}
4146

4247
/** A method call that produces a signature. */
@@ -49,6 +54,8 @@ private class ProduceSignatureCall extends ProduceCryptoCall {
4954
getMethod().hasStringSignature("sign(byte[], int, int)") and getArgument(0) = output
5055
)
5156
}
57+
58+
override string getResultType() { result = "signature" }
5259
}
5360

5461
/**
@@ -98,6 +105,8 @@ private class ProduceCiphertextCall extends ProduceCryptoCall {
98105
config.hasFlowTo(DataFlow3::exprNode(this.getQualifier()))
99106
)
100107
}
108+
109+
override string getResultType() { result = "ciphertext" }
101110
}
102111

103112
/** Holds if `fromNode` to `toNode` is a dataflow step that updates a cryptographic operation. */
@@ -173,13 +182,9 @@ private class UserInputInCryptoOperationConfig extends TaintTracking2::Configura
173182

174183
/** A source that produces result of cryptographic operation. */
175184
private class CryptoOperationSource extends DataFlow::Node {
176-
Expr cryptoOperation;
185+
ProduceCryptoCall call;
177186

178-
CryptoOperationSource() {
179-
exists(ProduceCryptoCall call | call.output() = this.asExpr() |
180-
cryptoOperation = call.getQualifier()
181-
)
182-
}
187+
CryptoOperationSource() { call.output() = this.asExpr() }
183188

184189
/** Holds if remote user input was used in the cryptographic operation. */
185190
predicate includesUserInput() {
@@ -188,9 +193,11 @@ private class CryptoOperationSource extends DataFlow::Node {
188193
|
189194
config.hasFlowPath(source, sink)
190195
|
191-
sink.getNode().asExpr() = cryptoOperation
196+
sink.getNode().asExpr() = call.getQualifier()
192197
)
193198
}
199+
200+
ProduceCryptoCall getCall() { result = call }
194201
}
195202

196203
/** Methods that use a non-constant-time algorithm for comparing inputs. */
@@ -329,8 +336,8 @@ from DataFlow::PathNode source, DataFlow::PathNode sink, NonConstantTimeCryptoCo
329336
where
330337
conf.hasFlowPath(source, sink) and
331338
(
332-
source.getNode().(CryptoOperationSource).includesUserInput() or
339+
source.getNode().(CryptoOperationSource).includesUserInput() and
333340
sink.getNode().(NonConstantTimeComparisonSink).includesUserInput()
334341
)
335-
select sink.getNode(), source, sink,
336-
"Using a non-constant-time algorithm for comparing results of a cryptographic operation."
342+
select sink.getNode(), source, sink, "Using a non-constant-time method for cheching a $@.", source,
343+
source.getNode().(CryptoOperationSource).getCall().getResultType()

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

Lines changed: 47 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,50 @@
11
edges
2-
| NonConstantTimeCryptoComparison.java:20:28:20:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:21:43:21:51 | actualMac |
3-
| NonConstantTimeCryptoComparison.java:29:28:29:40 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:30:84:30:92 | actualMac : byte[] |
4-
| NonConstantTimeCryptoComparison.java:30:84:30:92 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:30:66:30:93 | castToObjectArray(...) |
5-
| NonConstantTimeCryptoComparison.java:37:21:37:29 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:39:43:39:51 | actualMac |
6-
| NonConstantTimeCryptoComparison.java:57:28:57:40 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:58:40:58:48 | signature |
7-
| NonConstantTimeCryptoComparison.java:68:21:68:29 | signature : byte[] | NonConstantTimeCryptoComparison.java:69:40:69:48 | signature |
8-
| NonConstantTimeCryptoComparison.java:87:22:87:41 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:89:45:89:47 | tag |
9-
| NonConstantTimeCryptoComparison.java:102:24:102:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:103:40:103:42 | tag |
10-
| NonConstantTimeCryptoComparison.java:116:52:116:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:118:40:118:42 | tag : ByteBuffer |
11-
| NonConstantTimeCryptoComparison.java:118:40:118:42 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:118:40:118:50 | array(...) |
12-
| NonConstantTimeCryptoComparison.java:128:52:128:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:129:49:129:51 | tag |
13-
| NonConstantTimeCryptoComparison.java:146:22:146:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:147:40:147:42 | tag |
14-
| NonConstantTimeCryptoComparison.java:177:34:177:50 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:180:26:180:36 | computedTag |
2+
| NonConstantTimeCryptoComparison.java:21:32:21:48 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:23:47:23:55 | actualMac |
3+
| NonConstantTimeCryptoComparison.java:33:32:33:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:35:88:35:96 | actualMac : byte[] |
4+
| NonConstantTimeCryptoComparison.java:35:88:35:96 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:35:70:35:97 | castToObjectArray(...) |
5+
| NonConstantTimeCryptoComparison.java:46:25:46:33 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:48:47:48:55 | actualMac |
6+
| NonConstantTimeCryptoComparison.java:71:32:71:44 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:73:44:73:52 | signature |
7+
| NonConstantTimeCryptoComparison.java:85:25:85:33 | signature : byte[] | NonConstantTimeCryptoComparison.java:87:44:87:52 | signature |
8+
| NonConstantTimeCryptoComparison.java:111:26:111:45 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:113:49:113:51 | tag |
9+
| NonConstantTimeCryptoComparison.java:128:28:128:30 | tag : byte[] | NonConstantTimeCryptoComparison.java:130:44:130:46 | tag |
10+
| NonConstantTimeCryptoComparison.java:146:56:146:58 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:148:44:148:46 | tag : ByteBuffer |
11+
| NonConstantTimeCryptoComparison.java:148:44:148:46 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:148:44:148:54 | array(...) |
12+
| NonConstantTimeCryptoComparison.java:160:56:160:58 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:162:53:162:55 | tag |
13+
| NonConstantTimeCryptoComparison.java:185:26:185:50 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:187:44:187:46 | tag |
14+
| NonConstantTimeCryptoComparison.java:220:34:220:50 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:223:26:223:36 | computedTag |
1515
nodes
16-
| NonConstantTimeCryptoComparison.java:20:28:20:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
17-
| NonConstantTimeCryptoComparison.java:21:43:21:51 | actualMac | semmle.label | actualMac |
18-
| NonConstantTimeCryptoComparison.java:29:28:29:40 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
19-
| NonConstantTimeCryptoComparison.java:30:66:30:93 | castToObjectArray(...) | semmle.label | castToObjectArray(...) |
20-
| NonConstantTimeCryptoComparison.java:30:84:30:92 | actualMac : byte[] | semmle.label | actualMac : byte[] |
21-
| NonConstantTimeCryptoComparison.java:37:21:37:29 | actualMac : byte[] | semmle.label | actualMac : byte[] |
22-
| NonConstantTimeCryptoComparison.java:39:43:39:51 | actualMac | semmle.label | actualMac |
23-
| NonConstantTimeCryptoComparison.java:57:28:57:40 | sign(...) : byte[] | semmle.label | sign(...) : byte[] |
24-
| NonConstantTimeCryptoComparison.java:58:40:58:48 | signature | semmle.label | signature |
25-
| NonConstantTimeCryptoComparison.java:68:21:68:29 | signature : byte[] | semmle.label | signature : byte[] |
26-
| NonConstantTimeCryptoComparison.java:69:40:69:48 | signature | semmle.label | signature |
27-
| NonConstantTimeCryptoComparison.java:87:22:87:41 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
28-
| NonConstantTimeCryptoComparison.java:89:45:89:47 | tag | semmle.label | tag |
29-
| NonConstantTimeCryptoComparison.java:102:24:102:26 | tag : byte[] | semmle.label | tag : byte[] |
30-
| NonConstantTimeCryptoComparison.java:103:40:103:42 | tag | semmle.label | tag |
31-
| NonConstantTimeCryptoComparison.java:116:52:116:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
32-
| NonConstantTimeCryptoComparison.java:118:40:118:42 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
33-
| NonConstantTimeCryptoComparison.java:118:40:118:50 | array(...) | semmle.label | array(...) |
34-
| NonConstantTimeCryptoComparison.java:128:52:128:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
35-
| NonConstantTimeCryptoComparison.java:129:49:129:51 | tag | semmle.label | tag |
36-
| NonConstantTimeCryptoComparison.java:146:22:146:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
37-
| NonConstantTimeCryptoComparison.java:147:40:147:42 | tag | semmle.label | tag |
38-
| NonConstantTimeCryptoComparison.java:177:34:177:50 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
39-
| NonConstantTimeCryptoComparison.java:180:26:180:36 | computedTag | semmle.label | computedTag |
16+
| NonConstantTimeCryptoComparison.java:21:32:21:48 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
17+
| NonConstantTimeCryptoComparison.java:23:47:23:55 | actualMac | semmle.label | actualMac |
18+
| NonConstantTimeCryptoComparison.java:33:32:33:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
19+
| NonConstantTimeCryptoComparison.java:35:70:35:97 | castToObjectArray(...) | semmle.label | castToObjectArray(...) |
20+
| NonConstantTimeCryptoComparison.java:35:88:35:96 | actualMac : byte[] | semmle.label | actualMac : byte[] |
21+
| NonConstantTimeCryptoComparison.java:46:25:46:33 | actualMac : byte[] | semmle.label | actualMac : byte[] |
22+
| NonConstantTimeCryptoComparison.java:48:47:48:55 | actualMac | semmle.label | actualMac |
23+
| NonConstantTimeCryptoComparison.java:71:32:71:44 | sign(...) : byte[] | semmle.label | sign(...) : byte[] |
24+
| NonConstantTimeCryptoComparison.java:73:44:73:52 | signature | semmle.label | signature |
25+
| NonConstantTimeCryptoComparison.java:85:25:85:33 | signature : byte[] | semmle.label | signature : byte[] |
26+
| NonConstantTimeCryptoComparison.java:87:44:87:52 | signature | semmle.label | signature |
27+
| NonConstantTimeCryptoComparison.java:111:26:111:45 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
28+
| NonConstantTimeCryptoComparison.java:113:49:113:51 | tag | semmle.label | tag |
29+
| NonConstantTimeCryptoComparison.java:128:28:128:30 | tag : byte[] | semmle.label | tag : byte[] |
30+
| NonConstantTimeCryptoComparison.java:130:44:130:46 | tag | semmle.label | tag |
31+
| NonConstantTimeCryptoComparison.java:146:56:146:58 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
32+
| NonConstantTimeCryptoComparison.java:148:44:148:46 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
33+
| NonConstantTimeCryptoComparison.java:148:44:148:54 | array(...) | semmle.label | array(...) |
34+
| NonConstantTimeCryptoComparison.java:160:56:160:58 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
35+
| NonConstantTimeCryptoComparison.java:162:53:162:55 | tag | semmle.label | tag |
36+
| NonConstantTimeCryptoComparison.java:185:26:185:50 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
37+
| NonConstantTimeCryptoComparison.java:187:44:187:46 | tag | semmle.label | tag |
38+
| NonConstantTimeCryptoComparison.java:220:34:220:50 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
39+
| NonConstantTimeCryptoComparison.java:223:26:223:36 | computedTag | semmle.label | computedTag |
4040
#select
41-
| 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. |
42-
| 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. |
43-
| 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. |
44-
| 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. |
45-
| 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. |
46-
| 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. |
47-
| 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. |
48-
| 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. |
49-
| 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. |
50-
| 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. |
41+
| NonConstantTimeCryptoComparison.java:23:47:23:55 | actualMac | NonConstantTimeCryptoComparison.java:21:32:21:48 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:23:47:23:55 | actualMac | Using a non-constant-time method for cheching a $@. | NonConstantTimeCryptoComparison.java:21:32:21:48 | doFinal(...) : byte[] | MAC |
42+
| NonConstantTimeCryptoComparison.java:35:70:35:97 | castToObjectArray(...) | NonConstantTimeCryptoComparison.java:33:32:33:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:35:70:35:97 | castToObjectArray(...) | Using a non-constant-time method for cheching a $@. | NonConstantTimeCryptoComparison.java:33:32:33:44 | doFinal(...) : byte[] | MAC |
43+
| NonConstantTimeCryptoComparison.java:48:47:48:55 | actualMac | NonConstantTimeCryptoComparison.java:46:25:46:33 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:48:47:48:55 | actualMac | Using a non-constant-time method for cheching a $@. | NonConstantTimeCryptoComparison.java:46:25:46:33 | actualMac : byte[] | MAC |
44+
| NonConstantTimeCryptoComparison.java:73:44:73:52 | signature | NonConstantTimeCryptoComparison.java:71:32:71:44 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:73:44:73:52 | signature | Using a non-constant-time method for cheching a $@. | NonConstantTimeCryptoComparison.java:71:32:71:44 | sign(...) : byte[] | signature |
45+
| NonConstantTimeCryptoComparison.java:87:44:87:52 | signature | NonConstantTimeCryptoComparison.java:85:25:85:33 | signature : byte[] | NonConstantTimeCryptoComparison.java:87:44:87:52 | signature | Using a non-constant-time method for cheching a $@. | NonConstantTimeCryptoComparison.java:85:25:85:33 | signature : byte[] | signature |
46+
| NonConstantTimeCryptoComparison.java:113:49:113:51 | tag | NonConstantTimeCryptoComparison.java:111:26:111:45 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:113:49:113:51 | tag | Using a non-constant-time method for cheching a $@. | NonConstantTimeCryptoComparison.java:111:26:111:45 | doFinal(...) : byte[] | ciphertext |
47+
| NonConstantTimeCryptoComparison.java:130:44:130:46 | tag | NonConstantTimeCryptoComparison.java:128:28:128:30 | tag : byte[] | NonConstantTimeCryptoComparison.java:130:44:130:46 | tag | Using a non-constant-time method for cheching a $@. | NonConstantTimeCryptoComparison.java:128:28:128:30 | tag : byte[] | ciphertext |
48+
| NonConstantTimeCryptoComparison.java:148:44:148:54 | array(...) | NonConstantTimeCryptoComparison.java:146:56:146:58 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:148:44:148:54 | array(...) | Using a non-constant-time method for cheching a $@. | NonConstantTimeCryptoComparison.java:146:56:146:58 | tag : ByteBuffer | ciphertext |
49+
| NonConstantTimeCryptoComparison.java:162:53:162:55 | tag | NonConstantTimeCryptoComparison.java:160:56:160:58 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:162:53:162:55 | tag | Using a non-constant-time method for cheching a $@. | NonConstantTimeCryptoComparison.java:160:56:160:58 | tag : ByteBuffer | ciphertext |
50+
| NonConstantTimeCryptoComparison.java:187:44:187:46 | tag | NonConstantTimeCryptoComparison.java:185:26:185:50 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:187:44:187:46 | tag | Using a non-constant-time method for cheching a $@. | NonConstantTimeCryptoComparison.java:185:26:185:50 | doFinal(...) : byte[] | ciphertext |

0 commit comments

Comments
 (0)