mirror of
https://github.com/github/codeql.git
synced 2026-02-10 12:11:07 +01:00
Better comments and descriptions
This commit is contained in:
committed by
Fosstars
parent
f245dc3ac8
commit
5c474f689d
@@ -21,12 +21,12 @@ and does not depend on the contents of the arrays.
|
||||
<example>
|
||||
<p>
|
||||
The following example uses <code>Arrays.equals()</code> method for comparing MAC.
|
||||
This method implements a not-constant time algorithm:
|
||||
This method implements a non-constant time algorithm:
|
||||
</p>
|
||||
<sample src="UnsafeMacComparison.java" />
|
||||
|
||||
<p>
|
||||
The next example example uses a safe not-constant time algorithm for comparing MAC:
|
||||
The next example uses a safe constant time algorithm for comparing MAC:
|
||||
</p>
|
||||
<sample src="SafeMacComparison.java" />
|
||||
|
||||
@@ -35,19 +35,11 @@ The next example example uses a safe not-constant time algorithm for comparing M
|
||||
<references>
|
||||
<li>
|
||||
Wikipedia:
|
||||
<a href="https://en.wikipedia.org/wiki/Timing_attack">Timint attack</a>.
|
||||
</li>
|
||||
li>
|
||||
Common Weakness Enumeration:
|
||||
<a href="https://cwe.mitre.org/data/definitions/208.html">CWE-208: Observable Timing Discrepancy</a>.
|
||||
</li>
|
||||
<li>
|
||||
Common Weakness Enumeration:
|
||||
<a href="https://cwe.mitre.org/data/definitions/385.html">CWE-385: Covert Timing Channel</a>.
|
||||
<a href="https://en.wikipedia.org/wiki/Timing_attack">Timing attack</a>.
|
||||
</li>
|
||||
<li>
|
||||
Java API Specification:
|
||||
<a href="https://docs.oracle.com/javase/8/docs/api/java/security/MessageDigest.html#isEqual-byte:A-byte:A-">MessageDigest.isEqual() method</a>
|
||||
<a href="https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/security/MessageDigest.html#isEqual(byte[],byte[])">MessageDigest.isEqual() method</a>
|
||||
</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
|
||||
@@ -1,13 +1,14 @@
|
||||
/**
|
||||
* @name Using a not-constant time algorithm for comparison results of a cryptographic operation
|
||||
* @name Using a non-constant time algorithm for comparing results of a cryptographic operation
|
||||
* @description When comparing results of a cryptographic operation, a constant time algorithm should be used.
|
||||
* Otherwise, an attacker may be able to implement a timing attack.
|
||||
* A successful attack may result in leaking secrets or authentication bypass.
|
||||
* @kind path-problem
|
||||
* @problem.severity error
|
||||
* @precision high
|
||||
* @id java/not-constant-time-crypto-comparison
|
||||
* @id java/non-constant-time-crypto-comparison
|
||||
* @tags security
|
||||
* external/cwe/cwe-385
|
||||
* external/cwe/cwe-208
|
||||
*/
|
||||
|
||||
@@ -16,11 +17,11 @@ import semmle.code.java.dataflow.TaintTracking
|
||||
import DataFlow::PathGraph
|
||||
|
||||
/**
|
||||
* A method that returns a result of a cryptographic operation
|
||||
* A method that returns the result of a cryptographic operation
|
||||
* such as encryption, decryption, signing, etc.
|
||||
*/
|
||||
private class ReturnCryptoOperatinoResultMethod extends Method {
|
||||
ReturnCryptoOperatinoResultMethod() {
|
||||
private class ReturnCryptoOperationResultMethod extends Method {
|
||||
ReturnCryptoOperationResultMethod() {
|
||||
getDeclaringType().hasQualifiedName("javax.crypto", ["Mac", "Cipher"]) and
|
||||
hasName("doFinal")
|
||||
or
|
||||
@@ -30,14 +31,14 @@ private class ReturnCryptoOperatinoResultMethod extends Method {
|
||||
}
|
||||
|
||||
/**
|
||||
* A configuration that tracks data flows from cryptographic operations
|
||||
* to methods that compare data using a not-constant time algorithm.
|
||||
* A configuration that tracks data flow from cryptographic operations
|
||||
* to methods that compare data using a non-constant time algorithm.
|
||||
*/
|
||||
private class NotConstantTimeCryptoComparisonConfig extends TaintTracking::Configuration {
|
||||
NotConstantTimeCryptoComparisonConfig() { this = "NotConstantTimeCryptoComparisonConfig" }
|
||||
private class NonConstantTimeCryptoComparisonConfig extends TaintTracking::Configuration {
|
||||
NonConstantTimeCryptoComparisonConfig() { this = "NonConstantTimeCryptoComparisonConfig" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) {
|
||||
exists(MethodAccess ma | ma.getMethod() instanceof ReturnCryptoOperatinoResultMethod |
|
||||
exists(MethodAccess ma | ma.getMethod() instanceof ReturnCryptoOperationResultMethod |
|
||||
ma = source.asExpr()
|
||||
)
|
||||
}
|
||||
@@ -59,7 +60,7 @@ private class NotConstantTimeCryptoComparisonConfig extends TaintTracking::Confi
|
||||
}
|
||||
}
|
||||
|
||||
from DataFlow::PathNode source, DataFlow::PathNode sink, NotConstantTimeCryptoComparisonConfig conf
|
||||
from DataFlow::PathNode source, DataFlow::PathNode sink, NonConstantTimeCryptoComparisonConfig conf
|
||||
where conf.hasFlowPath(source, sink)
|
||||
select sink.getNode(), source, sink,
|
||||
"Using a not-constant time algorithm for comparison results of a cryptographic operation."
|
||||
"Using a non-constant time algorithm for comparing results of a cryptographic operation."
|
||||
|
||||
Reference in New Issue
Block a user