Timing attacks while comparing the headers value

This commit is contained in:
root
2022-02-06 13:46:42 -05:00
committed by Chris Smowton
parent 8b926f6859
commit 49feeb1c36
3 changed files with 101 additions and 0 deletions

View File

@@ -0,0 +1,28 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
A constant-time algorithm should be used for checking the value of headers.
In other words, the comparison time should not depend on the content of the input.
An attacker may be able to forge the value of the header.
</p>
</overview>
<recommendation>
<p>
Use <code>MessageDigest.isEqual()</code> method to check the value of headers.
If this method is used, then the calculation time depends only on the length of input byte arrays,
and does not depend on the contents of the arrays.
</p>
</recommendation>
<example>
<p>
The following example uses <code>Arrays.equals()</code> method for validating a csrf token .
This method implements a non-constant-time algorithm.
Both the message and the signature come from an untrusted HTTP request:
</p>
<sample src="UnsafecsrfComparison.java" />
</qhelp>

View File

@@ -0,0 +1,52 @@
import java
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PathGraph
private class NonConstantTimeEqualsCall extends MethodAccess {
NonConstantTimeEqualsCall() {
this.getMethod().hasQualifiedName("java.lang", "String", ["equals", "contentEquals", "equalsIgnoreCase"]) or
this.getMethod().hasQualifiedName("java.nio", "ByteBuffer", ["equals", "compareTo"])
}
}
private predicate isNonConstantTimeEqualsCall(Expr firstObject, Expr secondObject) {
exists(NonConstantTimeEqualsCall call |
firstObject = call.getQualifier() and
secondObject = call.getAnArgument()
or
firstObject = call.getAnArgument() and
secondObject = call.getQualifier()
)
}
class NonConstantTimeComparisonSink extends DataFlow::Node {
Expr anotherParameter;
NonConstantTimeComparisonSink() {
isNonConstantTimeEqualsCall(this.asExpr(), anotherParameter)
}
}
class ClientSuppliedIpTokenCheck extends DataFlow::Node {
ClientSuppliedIpTokenCheck() {
exists(MethodAccess ma |
ma.getMethod().hasName("getHeader") and
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue().toLowerCase() in [
"x-auth-token", "x-csrf-token", "http_x_csrf_token", "x-csrf-param", "x-csrf-header",
"http_x_csrf_token"
] and
ma = this.asExpr()
)
}
}
class NonConstantTimeComparisonConfig extends TaintTracking::Configuration {
NonConstantTimeComparisonConfig() { this = "NonConstantTimeCryptoComparisonConfig" }
override predicate isSource(DataFlow::Node source) { source instanceof ClientSuppliedIpTokenCheck }
override predicate isSink(DataFlow::Node sink) { sink instanceof NonConstantTimeComparisonSink }
}
from DataFlow::PathNode source, DataFlow::PathNode sink, NonConstantTimeComparisonConfig conf
where conf.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "Possible timing attack against $@ validation.", source,
source.getNode()

View File

@@ -0,0 +1,21 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.Cookie;
public boolean validateCsrfTokenInRequest(HttpServletRequest request) {
if (cookies != null) {
for (Cookie cookie : cookies) {
if (cookie.getName().equals(CSRF-TOKEN){
csrfCookieValue = cookie.getValue();
}
}
}
if (compareCsrfTokens(csrfCookieValue)) {
return true;
}
}
private boolean compareCsrfTokens(String csrfTokenInCookie) {
if(csrfTokenInCookie == null || !csrfTokenInCookie.equals(request.getHeader("X-CSRF-TOKEN"))) {
return false;
}
}