Skip to content

Commit 5ba9347

Browse files
smowtonsmowton
authored
Merge pull request github#6006 from artem-smotrakov/timing-attacks
Java: Timing attacks while comparing results of cryptographic operations
2 parents 15db6df + 171dc26 commit 5ba9347

File tree

15 files changed

+814
-0
lines changed

15 files changed

+814
-0
lines changed

‎java/change-notes/2021-06-22-more-steps-for-bytebuffer-inputstream.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* Added more taint propagation steps for `InputStream` and `ByteBuffer`.

‎java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCheckOnSignatureQuery.qll

Lines changed: 323 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,323 @@
1+
/**
2+
* Provides classes and predicates for queries that detect timing attacks.
3+
*/
4+
5+
import semmle.code.java.controlflow.Guards
6+
import semmle.code.java.dataflow.TaintTracking
7+
import semmle.code.java.dataflow.TaintTracking2
8+
import semmle.code.java.dataflow.DataFlow3
9+
import semmle.code.java.dataflow.FlowSources
10+
11+
/** A method call that produces cryptographic result. */
12+
abstract private class ProduceCryptoCall extends MethodAccess {
13+
Expr output;
14+
15+
/** Gets the result of cryptographic operation. */
16+
Expr output() { result = output }
17+
18+
/** Gets a type of cryptographic operation such as MAC, signature or ciphertext. */
19+
abstract string getResultType();
20+
}
21+
22+
/** A method call that produces a MAC. */
23+
private class ProduceMacCall extends ProduceCryptoCall {
24+
ProduceMacCall() {
25+
getMethod().getDeclaringType().hasQualifiedName("javax.crypto", "Mac") and
26+
(
27+
getMethod().hasStringSignature(["doFinal()", "doFinal(byte[])"]) and this = output
28+
or
29+
getMethod().hasStringSignature("doFinal(byte[], int)") and getArgument(0) = output
30+
)
31+
}
32+
33+
override string getResultType() { result = "MAC" }
34+
}
35+
36+
/** A method call that produces a signature. */
37+
private class ProduceSignatureCall extends ProduceCryptoCall {
38+
ProduceSignatureCall() {
39+
getMethod().getDeclaringType().hasQualifiedName("java.security", "Signature") and
40+
(
41+
getMethod().hasStringSignature("sign()") and this = output
42+
or
43+
getMethod().hasStringSignature("sign(byte[], int, int)") and getArgument(0) = output
44+
)
45+
}
46+
47+
override string getResultType() { result = "signature" }
48+
}
49+
50+
/**
51+
* A config that tracks data flow from initializing a cipher for encryption
52+
* to producing a ciphertext using this cipher.
53+
*/
54+
private class InitializeEncryptorConfig extends DataFlow3::Configuration {
55+
InitializeEncryptorConfig() { this = "InitializeEncryptorConfig" }
56+
57+
override predicate isSource(DataFlow::Node source) {
58+
exists(MethodAccess ma |
59+
ma.getMethod().hasQualifiedName("javax.crypto", "Cipher", "init") and
60+
ma.getArgument(0).(VarAccess).getVariable().hasName("ENCRYPT_MODE") and
61+
ma.getQualifier() = source.asExpr()
62+
)
63+
}
64+
65+
override predicate isSink(DataFlow::Node sink) {
66+
exists(MethodAccess ma |
67+
ma.getMethod().hasQualifiedName("javax.crypto", "Cipher", "doFinal") and
68+
ma.getQualifier() = sink.asExpr()
69+
)
70+
}
71+
}
72+
73+
/** A method call that produces a ciphertext. */
74+
private class ProduceCiphertextCall extends ProduceCryptoCall {
75+
ProduceCiphertextCall() {
76+
exists(Method m | m = this.getMethod() |
77+
m.getDeclaringType().hasQualifiedName("javax.crypto", "Cipher") and
78+
(
79+
m.hasStringSignature(["doFinal()", "doFinal(byte[])", "doFinal(byte[], int, int)"]) and
80+
this = output
81+
or
82+
m.hasStringSignature("doFinal(byte[], int)") and getArgument(0) = output
83+
or
84+
m.hasStringSignature([
85+
"doFinal(byte[], int, int, byte[])", "doFinal(byte[], int, int, byte[], int)"
86+
]) and
87+
getArgument(3) = output
88+
or
89+
m.hasStringSignature("doFinal(ByteBuffer, ByteBuffer)") and
90+
getArgument(1) = output
91+
)
92+
) and
93+
exists(InitializeEncryptorConfig config |
94+
config.hasFlowTo(DataFlow3::exprNode(this.getQualifier()))
95+
)
96+
}
97+
98+
override string getResultType() { result = "ciphertext" }
99+
}
100+
101+
/** Holds if `fromNode` to `toNode` is a dataflow step that updates a cryptographic operation. */
102+
private predicate updateCryptoOperationStep(DataFlow2::Node fromNode, DataFlow2::Node toNode) {
103+
exists(MethodAccess call, Method m |
104+
m = call.getMethod() and
105+
call.getQualifier() = toNode.asExpr() and
106+
call.getArgument(0) = fromNode.asExpr()
107+
|
108+
m.hasQualifiedName("java.security", "Signature", "update")
109+
or
110+
m.hasQualifiedName("javax.crypto", ["Mac", "Cipher"], "update")
111+
or
112+
m.hasQualifiedName("javax.crypto", ["Mac", "Cipher"], "doFinal") and
113+
not m.hasStringSignature("doFinal(byte[], int)")
114+
)
115+
}
116+
117+
/** Holds if `fromNode` to `toNode` is a dataflow step that creates a hash. */
118+
private predicate createMessageDigestStep(DataFlow2::Node fromNode, DataFlow2::Node toNode) {
119+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
120+
m.getDeclaringType().hasQualifiedName("java.security", "MessageDigest") and
121+
m.hasStringSignature("digest()") and
122+
ma.getQualifier() = fromNode.asExpr() and
123+
ma = toNode.asExpr()
124+
)
125+
or
126+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
127+
m.getDeclaringType().hasQualifiedName("java.security", "MessageDigest") and
128+
m.hasStringSignature("digest(byte[], int, int)") and
129+
ma.getQualifier() = fromNode.asExpr() and
130+
ma.getArgument(0) = toNode.asExpr()
131+
)
132+
or
133+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
134+
m.getDeclaringType().hasQualifiedName("java.security", "MessageDigest") and
135+
m.hasStringSignature("digest(byte[])") and
136+
ma.getArgument(0) = fromNode.asExpr() and
137+
ma = toNode.asExpr()
138+
)
139+
}
140+
141+
/** Holds if `fromNode` to `toNode` is a dataflow step that updates a hash. */
142+
private predicate updateMessageDigestStep(DataFlow2::Node fromNode, DataFlow2::Node toNode) {
143+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
144+
m.hasQualifiedName("java.security", "MessageDigest", "update") and
145+
ma.getArgument(0) = fromNode.asExpr() and
146+
ma.getQualifier() = toNode.asExpr()
147+
)
148+
}
149+
150+
/**
151+
* A config that tracks data flow from remote user input to a cryptographic operation
152+
* such as cipher, MAC or signature.
153+
*/
154+
private class UserInputInCryptoOperationConfig extends TaintTracking2::Configuration {
155+
UserInputInCryptoOperationConfig() { this = "UserInputInCryptoOperationConfig" }
156+
157+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
158+
159+
override predicate isSink(DataFlow::Node sink) {
160+
exists(ProduceCryptoCall call | call.getQualifier() = sink.asExpr())
161+
}
162+
163+
override predicate isAdditionalTaintStep(DataFlow2::Node fromNode, DataFlow2::Node toNode) {
164+
updateCryptoOperationStep(fromNode, toNode)
165+
or
166+
createMessageDigestStep(fromNode, toNode)
167+
or
168+
updateMessageDigestStep(fromNode, toNode)
169+
}
170+
}
171+
172+
/** A source that produces result of cryptographic operation. */
173+
class CryptoOperationSource extends DataFlow::Node {
174+
ProduceCryptoCall call;
175+
176+
CryptoOperationSource() { call.output() = this.asExpr() }
177+
178+
/** Holds if remote user input was used in the cryptographic operation. */
179+
predicate includesUserInput() {
180+
exists(
181+
DataFlow2::PathNode source, DataFlow2::PathNode sink, UserInputInCryptoOperationConfig config
182+
|
183+
config.hasFlowPath(source, sink)
184+
|
185+
sink.getNode().asExpr() = call.getQualifier()
186+
)
187+
}
188+
189+
/** Gets a method call that produces cryptographic result. */
190+
ProduceCryptoCall getCall() { result = call }
191+
}
192+
193+
/** Methods that use a non-constant-time algorithm for comparing inputs. */
194+
private class NonConstantTimeEqualsCall extends MethodAccess {
195+
NonConstantTimeEqualsCall() {
196+
getMethod()
197+
.hasQualifiedName("java.lang", "String", ["equals", "contentEquals", "equalsIgnoreCase"]) or
198+
getMethod().hasQualifiedName("java.nio", "ByteBuffer", ["equals", "compareTo"])
199+
}
200+
}
201+
202+
/** A static method that uses a non-constant-time algorithm for comparing inputs. */
203+
private class NonConstantTimeComparisonCall extends StaticMethodAccess {
204+
NonConstantTimeComparisonCall() {
205+
getMethod().hasQualifiedName("java.util", "Arrays", ["equals", "deepEquals"]) or
206+
getMethod().hasQualifiedName("java.util", "Objects", "deepEquals") or
207+
getMethod()
208+
.hasQualifiedName("org.apache.commons.lang3", "StringUtils",
209+
["equals", "equalsAny", "equalsAnyIgnoreCase", "equalsIgnoreCase"])
210+
}
211+
}
212+
213+
/**
214+
* A config that tracks data flow from remote user input to methods
215+
* that compare inputs using a non-constant-time algorithm.
216+
*/
217+
private class UserInputInComparisonConfig extends TaintTracking2::Configuration {
218+
UserInputInComparisonConfig() { this = "UserInputInComparisonConfig" }
219+
220+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
221+
222+
override predicate isSink(DataFlow::Node sink) {
223+
exists(NonConstantTimeEqualsCall call |
224+
sink.asExpr() = [call.getAnArgument(), call.getQualifier()]
225+
)
226+
or
227+
exists(NonConstantTimeComparisonCall call | sink.asExpr() = call.getAnArgument())
228+
}
229+
}
230+
231+
/** Holds if `expr` looks like a constant. */
232+
private predicate looksLikeConstant(Expr expr) {
233+
expr.isCompileTimeConstant()
234+
or
235+
expr.(VarAccess).getVariable().isFinal() and expr.getType() instanceof TypeString
236+
}
237+
238+
/**
239+
* Holds if `firstObject` and `secondObject` are compared using a method
240+
* that does not use a constant-time algorithm, for example, `String.equals()`.
241+
*/
242+
private predicate isNonConstantTimeEqualsCall(Expr firstObject, Expr secondObject) {
243+
exists(NonConstantTimeEqualsCall call |
244+
firstObject = call.getQualifier() and
245+
secondObject = call.getAnArgument()
246+
or
247+
firstObject = call.getAnArgument() and
248+
secondObject = call.getQualifier()
249+
)
250+
}
251+
252+
/**
253+
* Holds if `firstInput` and `secondInput` are compared using a static method
254+
* that does not use a constant-time algorithm, for example, `Arrays.equals()`.
255+
*/
256+
private predicate isNonConstantTimeComparisonCall(Expr firstInput, Expr secondInput) {
257+
exists(NonConstantTimeComparisonCall call |
258+
firstInput = call.getArgument(0) and secondInput = call.getArgument(1)
259+
or
260+
firstInput = call.getArgument(1) and secondInput = call.getArgument(0)
261+
)
262+
}
263+
264+
/**
265+
* Holds if there is a fast-fail check while comparing `firstArray` and `secondArray`.
266+
*/
267+
private predicate existsFailFastCheck(Expr firstArray, Expr secondArray) {
268+
exists(
269+
Guard guard, EqualityTest eqTest, boolean branch, Stmt fastFailingStmt,
270+
ArrayAccess firstArrayAccess, ArrayAccess secondArrayAccess
271+
|
272+
guard = eqTest and
273+
// For `==` false branch is fail fast; for `!=` true branch is fail fast
274+
branch = eqTest.polarity().booleanNot() and
275+
(
276+
fastFailingStmt instanceof ReturnStmt or
277+
fastFailingStmt instanceof BreakStmt or
278+
fastFailingStmt instanceof ThrowStmt
279+
) and
280+
guard.controls(fastFailingStmt.getBasicBlock(), branch) and
281+
DataFlow::localExprFlow(firstArrayAccess, eqTest.getLeftOperand()) and
282+
DataFlow::localExprFlow(secondArrayAccess, eqTest.getRightOperand())
283+
|
284+
firstArrayAccess.getArray() = firstArray and secondArray = secondArrayAccess
285+
or
286+
secondArrayAccess.getArray() = firstArray and secondArray = firstArrayAccess
287+
)
288+
}
289+
290+
/** A sink that compares input using a non-constant-time algorithm. */
291+
class NonConstantTimeComparisonSink extends DataFlow::Node {
292+
Expr anotherParameter;
293+
294+
NonConstantTimeComparisonSink() {
295+
(
296+
isNonConstantTimeEqualsCall(this.asExpr(), anotherParameter)
297+
or
298+
isNonConstantTimeComparisonCall(this.asExpr(), anotherParameter)
299+
or
300+
existsFailFastCheck(this.asExpr(), anotherParameter)
301+
) and
302+
not looksLikeConstant(anotherParameter)
303+
}
304+
305+
/** Holds if remote user input was used in the comparison. */
306+
predicate includesUserInput() {
307+
exists(UserInputInComparisonConfig config |
308+
config.hasFlowTo(DataFlow2::exprNode(anotherParameter))
309+
)
310+
}
311+
}
312+
313+
/**
314+
* A configuration that tracks data flow from cryptographic operations
315+
* to methods that compare data using a non-constant-time algorithm.
316+
*/
317+
class NonConstantTimeCryptoComparisonConfig extends TaintTracking::Configuration {
318+
NonConstantTimeCryptoComparisonConfig() { this = "NonConstantTimeCryptoComparisonConfig" }
319+
320+
override predicate isSource(DataFlow::Node source) { source instanceof CryptoOperationSource }
321+
322+
override predicate isSink(DataFlow::Node sink) { sink instanceof NonConstantTimeComparisonSink }
323+
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<include src="TimingAttackAgainstSignature.qhelp" />
4+
</qhelp>

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* @name Possible timing attack against signature validation
3+
* @description When checking a signature over a message, a constant-time algorithm should be used.
4+
* Otherwise, there is a risk of a timing attack that allows an attacker
5+
* to forge a valid signature for an arbitrary message. For a successful attack,
6+
* the attacker has to be able to send to the validation procedure both the message and the signature.
7+
* @kind path-problem
8+
* @problem.severity warning
9+
* @precision medium
10+
* @id java/possible-timing-attack-against-signature
11+
* @tags security
12+
* external/cwe/cwe-208
13+
*/
14+
15+
import java
16+
import NonConstantTimeCheckOnSignatureQuery
17+
import DataFlow::PathGraph
18+
19+
from DataFlow::PathNode source, DataFlow::PathNode sink, NonConstantTimeCryptoComparisonConfig conf
20+
where conf.hasFlowPath(source, sink)
21+
select sink.getNode(), source, sink, "Possible timing attack against $@ validation.", source,
22+
source.getNode().(CryptoOperationSource).getCall().getResultType()

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
public boolean validate(HttpRequest request, SecretKey key) throws Exception {
2+
byte[] message = getMessageFrom(request);
3+
byte[] signature = getSignatureFrom(request);
4+
5+
Mac mac = Mac.getInstance("HmacSHA256");
6+
mac.init(new SecretKeySpec(key.getEncoded(), "HmacSHA256"));
7+
byte[] actual = mac.doFinal(message);
8+
return MessageDigest.isEqual(signature, actual);
9+
}

0 commit comments

Comments
 (0)