Skip to content

Commit c96d939

Browse files
Covered custom fast-fail checks in NonConstantTimeCryptoComparison.ql
Co-authored-by: Marcono1234 <[email protected]>
1 parent 6500a1b commit c96d939

File tree

4 files changed

+173
-78
lines changed

4 files changed

+173
-78
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
<overview>
55
<p>
66
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
7+
a constant-time algorithm should be used. In other words, the comparison time should not depend on
88
the content of the input. Otherwise, attackers may be able to implement a timing attack
99
if they can control input. A successful timing attack may result in leaking secrets or authentication bypass.
1010
</p>
@@ -21,12 +21,12 @@ and does not depend on the contents of the arrays.
2121
<example>
2222
<p>
2323
The following example uses <code>Arrays.equals()</code> method for comparing MAC.
24-
This method implements a non-constant time algorithm:
24+
This method implements a non-constant-time algorithm:
2525
</p>
2626
<sample src="UnsafeMacComparison.java" />
2727

2828
<p>
29-
The next example uses a safe constant time algorithm for comparing MAC:
29+
The next example uses a safe constant-time algorithm for comparing MAC:
3030
</p>
3131
<sample src="SafeMacComparison.java" />
3232

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

Lines changed: 63 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
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.
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.
44
* Otherwise, attackers may be able to implement a timing attack if they can control input.
55
* A successful attack may result in leaking secrets or authentication bypass.
66
* @kind path-problem
@@ -12,6 +12,7 @@
1212
*/
1313

1414
import java
15+
import semmle.code.java.controlflow.Guards
1516
import semmle.code.java.dataflow.TaintTracking
1617
import semmle.code.java.dataflow.TaintTracking2
1718
import semmle.code.java.dataflow.FlowSources
@@ -146,7 +147,7 @@ private class NonConstantTimeComparisonCall extends StaticMethodAccess {
146147

147148
/**
148149
* A config that tracks data flow from remote user input to methods
149-
* that compare inputs using a non-constant time algorithm.
150+
* that compare inputs using a non-constant-time algorithm.
150151
*/
151152
private class UserInputInComparisonConfig extends TaintTracking2::Configuration {
152153
UserInputInComparisonConfig() { this = "UserInputInComparisonConfig" }
@@ -169,25 +170,69 @@ private predicate looksLikeConstant(Expr expr) {
169170
expr.(VarAccess).getVariable().isFinal() and expr.getType() instanceof TypeString
170171
}
171172

172-
/** A sink that compares input using a non-constant time algorithm. */
173+
/**
174+
* Holds if `firstObject` and `secondObject` are compared using a method
175+
* that does not use a constant-time algorithm, for example, `String.equals()`.
176+
*/
177+
private predicate isNonConstantEqualsCall(Expr firstObject, Expr secondObject) {
178+
exists(NonConstantTimeEqualsCall call |
179+
firstObject = call.getQualifier() and
180+
secondObject = call.getAnArgument()
181+
or
182+
firstObject = call.getAnArgument() and
183+
secondObject = call.getQualifier()
184+
)
185+
}
186+
187+
/**
188+
* Holds if `firstInput` and `secondInput` are compared using a static method
189+
* that does not use a constant-time algorithm, for example, `Arrays.equals()`.
190+
*/
191+
private predicate isNonConstantTimeComparisonCall(Expr firstInput, Expr secondInput) {
192+
exists(NonConstantTimeComparisonCall call |
193+
firstInput = call.getArgument(0) and secondInput = call.getArgument(1)
194+
or
195+
firstInput = call.getArgument(1) and secondInput = call.getArgument(0)
196+
)
197+
}
198+
199+
/**
200+
* Holds if there is a fast-fail check while comparing `firstArray` and `secondArray`.
201+
*/
202+
private predicate existsFailFastCheck(Expr firstArray, Expr secondArray) {
203+
exists(
204+
Guard guard, EqualityTest eqTest, boolean branch, Stmt fastFailingStmt,
205+
ArrayAccess firstArrayAccess, ArrayAccess secondArrayAccess
206+
|
207+
guard = eqTest and
208+
// For `==` false branch is fail fast; for `!=` true branch is fail fast
209+
branch = eqTest.polarity().booleanNot() and
210+
(
211+
fastFailingStmt instanceof ReturnStmt or
212+
fastFailingStmt instanceof BreakStmt or
213+
fastFailingStmt instanceof ThrowStmt
214+
) and
215+
guard.controls(fastFailingStmt.getBasicBlock(), branch) and
216+
DataFlow::localExprFlow(firstArrayAccess, eqTest.getLeftOperand()) and
217+
DataFlow::localExprFlow(secondArrayAccess, eqTest.getRightOperand())
218+
|
219+
firstArrayAccess.getArray() = firstArray and secondArray = secondArrayAccess
220+
or
221+
secondArrayAccess.getArray() = firstArray and secondArray = firstArrayAccess
222+
)
223+
}
224+
225+
/** A sink that compares input using a non-constant-time algorithm. */
173226
private class NonConstantTimeComparisonSink extends DataFlow::Node {
174227
Expr anotherParameter;
175228

176229
NonConstantTimeComparisonSink() {
177230
(
178-
exists(NonConstantTimeEqualsCall call |
179-
this.asExpr() = call.getQualifier() and
180-
anotherParameter = call.getAnArgument()
181-
or
182-
this.asExpr() = call.getAnArgument() and
183-
anotherParameter = call.getQualifier()
184-
)
231+
isNonConstantEqualsCall(this.asExpr(), anotherParameter)
232+
or
233+
isNonConstantTimeComparisonCall(this.asExpr(), anotherParameter)
185234
or
186-
exists(NonConstantTimeComparisonCall call | call.getAnArgument() = this.asExpr() |
187-
this.asExpr() = call.getArgument(0) and anotherParameter = call.getArgument(1)
188-
or
189-
this.asExpr() = call.getArgument(1) and anotherParameter = call.getArgument(0)
190-
)
235+
existsFailFastCheck(this.asExpr(), anotherParameter)
191236
) and
192237
not looksLikeConstant(anotherParameter)
193238
}
@@ -202,7 +247,7 @@ private class NonConstantTimeComparisonSink extends DataFlow::Node {
202247

203248
/**
204249
* A configuration that tracks data flow from cryptographic operations
205-
* to methods that compare data using a non-constant time algorithm.
250+
* to methods that compare data using a non-constant-time algorithm.
206251
*/
207252
private class NonConstantTimeCryptoComparisonConfig extends TaintTracking::Configuration {
208253
NonConstantTimeCryptoComparisonConfig() { this = "NonConstantTimeCryptoComparisonConfig" }
@@ -220,4 +265,4 @@ where
220265
sink.getNode().(NonConstantTimeComparisonSink).includesUserInput()
221266
)
222267
select sink.getNode(), source, sink,
223-
"Using a non-constant time algorithm for comparing results of a cryptographic operation."
268+
"Using a non-constant-time algorithm for comparing results of a cryptographic operation."
Lines changed: 47 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,50 @@
11
edges
2-
| NonConstantTimeCryptoComparison.java:19:28:19:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:20:43:20:51 | actualMac |
3-
| NonConstantTimeCryptoComparison.java:28:28:28:40 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:29:84:29:92 | actualMac : byte[] |
4-
| NonConstantTimeCryptoComparison.java:29:84:29:92 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:29:66:29:93 | castToObjectArray(...) |
5-
| NonConstantTimeCryptoComparison.java:36:21:36:29 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:38:43:38:51 | actualMac |
6-
| NonConstantTimeCryptoComparison.java:56:28:56:40 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:57:40:57:48 | signature |
7-
| NonConstantTimeCryptoComparison.java:67:21:67:29 | signature : byte[] | NonConstantTimeCryptoComparison.java:68:40:68:48 | signature |
8-
| NonConstantTimeCryptoComparison.java:85:22:85:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:87:45:87:47 | tag |
9-
| NonConstantTimeCryptoComparison.java:97:24:97:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:98:40:98:42 | tag |
10-
| NonConstantTimeCryptoComparison.java:107:52:107:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:109:40:109:42 | tag : ByteBuffer |
11-
| NonConstantTimeCryptoComparison.java:109:40:109:42 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:109:40:109:50 | array(...) |
12-
| NonConstantTimeCryptoComparison.java:119:52:119:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:120:49:120:51 | tag |
13-
| NonConstantTimeCryptoComparison.java:136:22:136:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:137:40:137:42 | tag |
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:86:22:86:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:88:45:88:47 | tag |
9+
| NonConstantTimeCryptoComparison.java:98:24:98:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:99:40:99:42 | tag |
10+
| NonConstantTimeCryptoComparison.java:108:52:108:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:110:40:110:42 | tag : ByteBuffer |
11+
| NonConstantTimeCryptoComparison.java:110:40:110:42 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:110:40:110:50 | array(...) |
12+
| NonConstantTimeCryptoComparison.java:120:52:120:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:121:49:121:51 | tag |
13+
| NonConstantTimeCryptoComparison.java:137:22:137:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:138:40:138:42 | tag |
14+
| NonConstantTimeCryptoComparison.java:168:34:168:50 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:171:26:171:36 | computedTag |
1415
nodes
15-
| NonConstantTimeCryptoComparison.java:19:28:19:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
16-
| NonConstantTimeCryptoComparison.java:20:43:20:51 | actualMac | semmle.label | actualMac |
17-
| NonConstantTimeCryptoComparison.java:28:28:28:40 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
18-
| NonConstantTimeCryptoComparison.java:29:66:29:93 | castToObjectArray(...) | semmle.label | castToObjectArray(...) |
19-
| NonConstantTimeCryptoComparison.java:29:84:29:92 | actualMac : byte[] | semmle.label | actualMac : byte[] |
20-
| NonConstantTimeCryptoComparison.java:36:21:36:29 | actualMac : byte[] | semmle.label | actualMac : byte[] |
21-
| NonConstantTimeCryptoComparison.java:38:43:38:51 | actualMac | semmle.label | actualMac |
22-
| NonConstantTimeCryptoComparison.java:56:28:56:40 | sign(...) : byte[] | semmle.label | sign(...) : byte[] |
23-
| NonConstantTimeCryptoComparison.java:57:40:57:48 | signature | semmle.label | signature |
24-
| NonConstantTimeCryptoComparison.java:67:21:67:29 | signature : byte[] | semmle.label | signature : byte[] |
25-
| NonConstantTimeCryptoComparison.java:68:40:68:48 | signature | semmle.label | signature |
26-
| NonConstantTimeCryptoComparison.java:85:22:85:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
27-
| NonConstantTimeCryptoComparison.java:87:45:87:47 | tag | semmle.label | tag |
28-
| NonConstantTimeCryptoComparison.java:97:24:97:26 | tag : byte[] | semmle.label | tag : byte[] |
29-
| NonConstantTimeCryptoComparison.java:98:40:98:42 | tag | semmle.label | tag |
30-
| NonConstantTimeCryptoComparison.java:107:52:107:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
31-
| NonConstantTimeCryptoComparison.java:109:40:109:42 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
32-
| NonConstantTimeCryptoComparison.java:109:40:109:50 | array(...) | semmle.label | array(...) |
33-
| NonConstantTimeCryptoComparison.java:119:52:119:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
34-
| NonConstantTimeCryptoComparison.java:120:49:120:51 | tag | semmle.label | tag |
35-
| NonConstantTimeCryptoComparison.java:136:22:136:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
36-
| NonConstantTimeCryptoComparison.java:137:40:137:42 | tag | semmle.label | tag |
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:86:22:86:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
28+
| NonConstantTimeCryptoComparison.java:88:45:88:47 | tag | semmle.label | tag |
29+
| NonConstantTimeCryptoComparison.java:98:24:98:26 | tag : byte[] | semmle.label | tag : byte[] |
30+
| NonConstantTimeCryptoComparison.java:99:40:99:42 | tag | semmle.label | tag |
31+
| NonConstantTimeCryptoComparison.java:108:52:108:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
32+
| NonConstantTimeCryptoComparison.java:110:40:110:42 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
33+
| NonConstantTimeCryptoComparison.java:110:40:110:50 | array(...) | semmle.label | array(...) |
34+
| NonConstantTimeCryptoComparison.java:120:52:120:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer |
35+
| NonConstantTimeCryptoComparison.java:121:49:121:51 | tag | semmle.label | tag |
36+
| NonConstantTimeCryptoComparison.java:137:22:137:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
37+
| NonConstantTimeCryptoComparison.java:138:40:138:42 | tag | semmle.label | tag |
38+
| NonConstantTimeCryptoComparison.java:168:34:168:50 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
39+
| NonConstantTimeCryptoComparison.java:171:26:171:36 | computedTag | semmle.label | computedTag |
3740
#select
38-
| NonConstantTimeCryptoComparison.java:20:43:20:51 | actualMac | NonConstantTimeCryptoComparison.java:19:28:19:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:20:43:20:51 | actualMac | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
39-
| NonConstantTimeCryptoComparison.java:29:66:29:93 | castToObjectArray(...) | NonConstantTimeCryptoComparison.java:28:28:28:40 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:29:66:29:93 | castToObjectArray(...) | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
40-
| NonConstantTimeCryptoComparison.java:38:43:38:51 | actualMac | NonConstantTimeCryptoComparison.java:36:21:36:29 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:38:43:38:51 | actualMac | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
41-
| NonConstantTimeCryptoComparison.java:57:40:57:48 | signature | NonConstantTimeCryptoComparison.java:56:28:56:40 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:57:40:57:48 | signature | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
42-
| NonConstantTimeCryptoComparison.java:68:40:68:48 | signature | NonConstantTimeCryptoComparison.java:67:21:67:29 | signature : byte[] | NonConstantTimeCryptoComparison.java:68:40:68:48 | signature | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
43-
| NonConstantTimeCryptoComparison.java:87:45:87:47 | tag | NonConstantTimeCryptoComparison.java:85:22:85:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:87:45:87:47 | tag | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
44-
| NonConstantTimeCryptoComparison.java:98:40:98:42 | tag | NonConstantTimeCryptoComparison.java:97:24:97:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:98:40:98:42 | tag | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
45-
| NonConstantTimeCryptoComparison.java:109:40:109:50 | array(...) | NonConstantTimeCryptoComparison.java:107:52:107:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:109:40:109:50 | array(...) | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
46-
| NonConstantTimeCryptoComparison.java:120:49:120:51 | tag | NonConstantTimeCryptoComparison.java:119:52:119:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:120:49:120:51 | tag | Using a non-constant time algorithm for comparing results of a cryptographic operation. |
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: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. |
47+
| 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. |
48+
| 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. |
49+
| 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. |
50+
| 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. |

0 commit comments

Comments
 (0)