mirror of
https://github.com/github/codeql.git
synced 2026-04-28 18:25:24 +02:00
Less duplicate code in java/non-constant-time-crypto-comparison
This commit is contained in:
Artem Smotrakov
committed by
Fosstars
Fosstars
parent
40e513ba52
commit
d01dc35011
@@ -17,40 +17,88 @@ import semmle.code.java.dataflow.TaintTracking2
|
||||
import semmle.code.java.dataflow.FlowSources
|
||||
import DataFlow::PathGraph
|
||||
|
||||
private class ByteBuffer extends Class {
|
||||
ByteBuffer() { hasQualifiedName("java.nio", "ByteBuffer") }
|
||||
}
|
||||
|
||||
abstract private class ProduceCryptoCall extends MethodAccess {
|
||||
Expr output;
|
||||
|
||||
Expr output() { result = output }
|
||||
}
|
||||
|
||||
private class ProduceMacCall extends ProduceCryptoCall {
|
||||
ProduceMacCall() {
|
||||
getMethod().hasQualifiedName("javax.crypto", "Mac", "doFinal") and
|
||||
(
|
||||
getMethod().getReturnType() instanceof Array and this = output
|
||||
or
|
||||
getMethod().getParameterType(0) instanceof Array and getArgument(0) = output
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private class ProduceSignatureCall extends ProduceCryptoCall {
|
||||
ProduceSignatureCall() {
|
||||
getMethod().hasQualifiedName("java.security", "Signature", "sign") and
|
||||
(
|
||||
getMethod().getReturnType() instanceof Array and this = output
|
||||
or
|
||||
getMethod().getParameterType(0) instanceof Array and getArgument(0) = output
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private class ProduceCiphertextCall extends ProduceCryptoCall {
|
||||
ProduceCiphertextCall() {
|
||||
getMethod().hasQualifiedName("javax.crypto", "Cipher", "doFinal") and
|
||||
(
|
||||
getMethod().getReturnType() instanceof Array and this = output
|
||||
or
|
||||
getMethod().getParameterType([0, 3]) instanceof Array and getArgument([0, 3]) = output
|
||||
or
|
||||
getMethod().getParameterType(1) instanceof ByteBuffer and
|
||||
getArgument(1) = output
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private class UserInputInCryptoOperationConfig extends TaintTracking2::Configuration {
|
||||
UserInputInCryptoOperationConfig() { this = "UserInputInCryptoOperationConfig" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
(
|
||||
(
|
||||
m.hasQualifiedName("javax.crypto", ["Mac", "Cipher"], "doFinal") or
|
||||
m.hasQualifiedName("java.security", "Signature", "sign")
|
||||
) and
|
||||
ma.getQualifier() = sink.asExpr()
|
||||
)
|
||||
)
|
||||
exists(ProduceCryptoCall call | call.getQualifier() = sink.asExpr())
|
||||
}
|
||||
|
||||
override predicate isAdditionalTaintStep(DataFlow2::Node fromNode, DataFlow2::Node toNode) {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
exists(MethodAccess call, Method m |
|
||||
m = call.getMethod() and
|
||||
call.getQualifier() = toNode.asExpr() and
|
||||
call.getArgument(0) = fromNode.asExpr()
|
||||
|
|
||||
(
|
||||
(
|
||||
m.hasQualifiedName("javax.crypto", ["Mac", "Cipher"], ["doFinal", "update"]) or
|
||||
m.hasQualifiedName("java.security", "Signature", ["sign", "update"])
|
||||
) and
|
||||
ma.getAnArgument() = fromNode.asExpr() and
|
||||
ma.getQualifier() = toNode.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)")
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
abstract private class CryptoOperationSource extends DataFlow::Node {
|
||||
private class CryptoOperationSource extends DataFlow::Node {
|
||||
Expr cryptoOperation;
|
||||
|
||||
CryptoOperationSource() {
|
||||
exists(ProduceCryptoCall call | call.output() = this.asExpr() |
|
||||
cryptoOperation = call.getQualifier()
|
||||
)
|
||||
}
|
||||
|
||||
predicate includesUserInput() {
|
||||
exists(
|
||||
DataFlow2::PathNode source, DataFlow2::PathNode sink, UserInputInCryptoOperationConfig config
|
||||
@@ -62,121 +110,68 @@ abstract private class CryptoOperationSource extends DataFlow::Node {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A source that produces a MAC.
|
||||
*/
|
||||
private class MacSource extends CryptoOperationSource {
|
||||
MacSource() {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
m.hasQualifiedName("javax.crypto", "Mac", "doFinal") and
|
||||
(
|
||||
m.getReturnType() instanceof Array and ma = this.asExpr()
|
||||
or
|
||||
m.getParameterType(0) instanceof Array and ma.getArgument(0) = this.asExpr()
|
||||
) and
|
||||
cryptoOperation = ma.getQualifier()
|
||||
)
|
||||
private class NonConstantTimeEqualsCall extends MethodAccess {
|
||||
NonConstantTimeEqualsCall() {
|
||||
getMethod()
|
||||
.hasQualifiedName("java.lang", "String", ["equals", "contentEquals", "equalsIgnoreCase"]) or
|
||||
getMethod().hasQualifiedName("java.nio", "ByteBuffer", ["equals", "compareTo"])
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A source that produces a signature.
|
||||
*/
|
||||
private class SignatureSource extends CryptoOperationSource {
|
||||
SignatureSource() {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
m.hasQualifiedName("java.security", "Signature", "sign") and
|
||||
(
|
||||
m.getReturnType() instanceof Array and ma = this.asExpr()
|
||||
or
|
||||
m.getParameterType(0) instanceof Array and ma.getArgument(0) = this.asExpr()
|
||||
) and
|
||||
cryptoOperation = ma.getQualifier()
|
||||
)
|
||||
private class NonConstantTimeComparisonCall extends StaticMethodAccess {
|
||||
NonConstantTimeComparisonCall() {
|
||||
getMethod().hasQualifiedName("java.util", "Arrays", ["equals", "deepEquals"]) or
|
||||
getMethod().hasQualifiedName("java.util", "Objects", "deepEquals") or
|
||||
getMethod()
|
||||
.hasQualifiedName("org.apache.commons.lang3", "StringUtils",
|
||||
["equals", "equalsAny", "equalsAnyIgnoreCase", "equalsIgnoreCase"])
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A source that produces a ciphertext.
|
||||
*/
|
||||
private class CiphertextSource extends CryptoOperationSource {
|
||||
CiphertextSource() {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
m.hasQualifiedName("javax.crypto", "Cipher", "doFinal") and
|
||||
(
|
||||
m.getReturnType() instanceof Array and ma = this.asExpr()
|
||||
or
|
||||
m.getParameterType([0, 3]) instanceof Array and ma.getArgument([0, 3]) = this.asExpr()
|
||||
or
|
||||
m.getParameterType(1).(RefType).hasQualifiedName("java.nio", "ByteBuffer") and
|
||||
ma.getArgument(1) = this.asExpr()
|
||||
) and
|
||||
cryptoOperation = ma.getQualifier()
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private class UserInputComparisonConfig extends TaintTracking2::Configuration {
|
||||
UserInputComparisonConfig() { this = "UserInputComparisonConfig" }
|
||||
private class UserInputInComparisonConfig extends TaintTracking2::Configuration {
|
||||
UserInputInComparisonConfig() { this = "UserInputInComparisonConfig" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
(
|
||||
m.hasQualifiedName("java.lang", "String", ["equals", "contentEquals", "equalsIgnoreCase"]) or
|
||||
m.hasQualifiedName("java.nio", "ByteBuffer", ["equals", "compareTo"]) or
|
||||
m.hasQualifiedName("java.util", "Arrays", ["equals", "deepEquals"]) or
|
||||
m.hasQualifiedName("java.util", "Objects", "deepEquals") or
|
||||
m.hasQualifiedName("org.apache.commons.lang3", "StringUtils",
|
||||
["equals", "equalsAny", "equalsAnyIgnoreCase", "equalsIgnoreCase"])
|
||||
) and
|
||||
[ma.getAnArgument(), ma.getQualifier()] = sink.asExpr()
|
||||
exists(NonConstantTimeEqualsCall call |
|
||||
sink.asExpr() = [call.getAnArgument(), call.getQualifier()]
|
||||
)
|
||||
or
|
||||
exists(NonConstantTimeComparisonCall call | sink.asExpr() = call.getAnArgument())
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A sink that compares input using a non-constant time algorithm.
|
||||
*/
|
||||
private class KnownNonConstantTimeComparisonSink extends DataFlow::Node {
|
||||
private class NonConstantTimeComparisonSink extends DataFlow::Node {
|
||||
Expr anotherParameter;
|
||||
|
||||
KnownNonConstantTimeComparisonSink() {
|
||||
exists(MethodAccess ma, Method m | m = ma.getMethod() |
|
||||
(
|
||||
m.hasQualifiedName("java.lang", "String", ["equals", "contentEquals", "equalsIgnoreCase"]) or
|
||||
m.hasQualifiedName("java.nio", "ByteBuffer", ["equals", "compareTo"])
|
||||
) and
|
||||
(
|
||||
this.asExpr() = ma.getQualifier() and
|
||||
anotherParameter = ma.getAnArgument() and
|
||||
not ma.getAnArgument().isCompileTimeConstant()
|
||||
NonConstantTimeComparisonSink() {
|
||||
not anotherParameter.isCompileTimeConstant() and
|
||||
(
|
||||
exists(NonConstantTimeEqualsCall call |
|
||||
this.asExpr() = call.getQualifier() and
|
||||
anotherParameter = call.getAnArgument()
|
||||
or
|
||||
this.asExpr() = ma.getAnArgument() and
|
||||
anotherParameter = ma.getQualifier() and
|
||||
not ma.getQualifier().isCompileTimeConstant()
|
||||
this.asExpr() = call.getAnArgument() and
|
||||
anotherParameter = call.getQualifier()
|
||||
)
|
||||
)
|
||||
or
|
||||
exists(StaticMethodAccess ma, Method m | m = ma.getMethod() |
|
||||
(
|
||||
m.hasQualifiedName("java.util", "Arrays", ["equals", "deepEquals"]) or
|
||||
m.hasQualifiedName("java.util", "Objects", "deepEquals") or
|
||||
m.hasQualifiedName("org.apache.commons.lang3", "StringUtils",
|
||||
["equals", "equalsAny", "equalsAnyIgnoreCase", "equalsIgnoreCase"])
|
||||
) and
|
||||
ma.getAnArgument() = this.asExpr() and
|
||||
(
|
||||
this.asExpr() = ma.getArgument(0) and anotherParameter = ma.getArgument(1)
|
||||
or
|
||||
this.asExpr() = ma.getArgument(1) and anotherParameter = ma.getArgument(0)
|
||||
or
|
||||
exists(NonConstantTimeComparisonCall call |
|
||||
call.getAnArgument() = this.asExpr() and
|
||||
(
|
||||
this.asExpr() = call.getArgument(0) and anotherParameter = call.getArgument(1)
|
||||
or
|
||||
this.asExpr() = call.getArgument(1) and anotherParameter = call.getArgument(0)
|
||||
)
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
predicate includesUserInput() {
|
||||
exists(UserInputComparisonConfig config |
|
||||
exists(UserInputInComparisonConfig config |
|
||||
config.hasFlowTo(DataFlow2::exprNode(anotherParameter))
|
||||
)
|
||||
}
|
||||
@@ -191,9 +186,7 @@ private class NonConstantTimeCryptoComparisonConfig extends TaintTracking::Confi
|
||||
|
||||
override predicate isSource(DataFlow::Node source) { source instanceof CryptoOperationSource }
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
sink instanceof KnownNonConstantTimeComparisonSink
|
||||
}
|
||||
override predicate isSink(DataFlow::Node sink) { sink instanceof NonConstantTimeComparisonSink }
|
||||
}
|
||||
|
||||
from DataFlow::PathNode source, DataFlow::PathNode sink, NonConstantTimeCryptoComparisonConfig conf
|
||||
@@ -201,7 +194,7 @@ where
|
||||
conf.hasFlowPath(source, sink) and
|
||||
(
|
||||
source.getNode().(CryptoOperationSource).includesUserInput() or
|
||||
sink.getNode().(KnownNonConstantTimeComparisonSink).includesUserInput()
|
||||
sink.getNode().(NonConstantTimeComparisonSink).includesUserInput()
|
||||
)
|
||||
select sink.getNode(), source, sink,
|
||||
"Using a non-constant time algorithm for comparing results of a cryptographic operation."
|
||||
|
||||
Reference in New Issue
Block a user