hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-28 18:25:24 +02:00
Code Issues Packages Projects Releases Wiki Activity

Removed hashes from NotConstantTimeCryptoComparison.ql

Browse Source
This commit is contained in:
Artem Smotrakov
2021-06-14 08:57:10 +02:00
committed by Fosstars
parent 8a69b7b3ac
commit f245dc3ac8
8 changed files with 25 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
java/ql/src/experimental/Security/CWE/CWE-208/NotConstantTimeCryptoComparison.qhelp
View File

@@ -3,7 +3,7 @@
<overview>
<p>
When comparing results of cryptographic operations, such as MAC or cryptographic hash,
When comparing results of cryptographic operations, such as MAC or digital signature,
a constant time algorithm should be used. In other words, the comparison time should not depend on
the content of the input. Otherwise, an attacker may be able to implement a timing attack.
A successful timing attack may result in leaking secrets or authentication bypass.
@@ -20,15 +20,15 @@ and does not depend on the contents of the arrays.
<example>
<p>
The following example uses <code>Arrays.equals()</code> method for comparing cryptographic hashes.
The following example uses <code>Arrays.equals()</code> method for comparing MAC.
This method implements a not-constant time algorithm:
</p>
<sample src="UnsafeCryptoHashComparison.java" />
<sample src="UnsafeMacComparison.java" />
<p>
The next example example uses a safe not-constant time algorithm for comparing cryptographic hashes:
The next example example uses a safe not-constant time algorithm for comparing MAC:
</p>
<sample src="SafeCryptoHashComparison.java" />
<sample src="SafeMacComparison.java" />
</example>

3
java/ql/src/experimental/Security/CWE/CWE-208/NotConstantTimeCryptoComparison.ql
View File

@@ -26,9 +26,6 @@ private class ReturnCryptoOperatinoResultMethod extends Method {
or
getDeclaringType().hasQualifiedName("java.security", "Signature") and
hasName("sign")
or
getDeclaringType().hasQualifiedName("java.security", "MessageDigest") and
hasName("digest")
}
}

5
java/ql/src/experimental/Security/CWE/CWE-208/SafeCryptoHashComparison.java
View File

@@ -1,5 +0,0 @@
public boolean checkHash(byte[] expectedHash, byte[] data) throws Exception {
MessageDigest md = MessageDigest.getInstance("SHA-256");
byte[] actualHash = md.digest(data);
return MessageDigest.isEqual(expectedHash, actualHash);
}

6
java/ql/src/experimental/Security/CWE/CWE-208/SafeMacComparison.java Normal file
View File

@@ -0,0 +1,6 @@
public boolean check(byte[] expected, byte[] data, SecretKey key) throws Exception {
Mac mac = Mac.getInstance("HmacSHA256");
mac.init(new SecretKeySpec(key.getEncoded(), "HmacSHA256"));
byte[] actual = mac.doFinal(data);
return MessageDigest.isEqual(expected, actual);
}

5
java/ql/src/experimental/Security/CWE/CWE-208/UnsafeCryptoHashComparison.java
View File

@@ -1,5 +0,0 @@
public boolean checkHash(byte[] expectedHash, byte[] data) throws Exception {
MessageDigest md = MessageDigest.getInstance("SHA-256");
byte[] actualHash = md.digest(data);
return Arrays.equals(expectedHash, actualHash);
}

6
java/ql/src/experimental/Security/CWE/CWE-208/UnsafeMacComparison.java Normal file
View File

@@ -0,0 +1,6 @@
public boolean check(byte[] expected, byte[] data, SecretKey key) throws Exception {
Mac mac = Mac.getInstance("HmacSHA256");
mac.init(new SecretKeySpec(key.getEncoded(), "HmacSHA256"));
byte[] actual = mac.doFinal(data);
return Arrays.equals(expected, actual);
}

20
java/ql/test/experimental/query-tests/security/CWE-208/NotConstantTimeCryptoComparison.expected
View File

@@ -1,19 +1,15 @@
edges
| NotConstantTimeCryptoComparison.java:14:28:14:44 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:15:43:15:51 | actualMac |
| NotConstantTimeCryptoComparison.java:28:36:28:50 | digest(...) : byte[] | NotConstantTimeCryptoComparison.java:29:16:29:21 | actual |
| NotConstantTimeCryptoComparison.java:44:28:44:40 | sign(...) : byte[] | NotConstantTimeCryptoComparison.java:45:40:45:48 | signature |
| NotConstantTimeCryptoComparison.java:61:22:61:46 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:62:40:62:42 | tag |
| NotConstantTimeCryptoComparison.java:30:28:30:40 | sign(...) : byte[] | NotConstantTimeCryptoComparison.java:31:40:31:48 | signature |
| NotConstantTimeCryptoComparison.java:47:22:47:46 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:48:40:48:42 | tag |
nodes
| NotConstantTimeCryptoComparison.java:14:28:14:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
| NotConstantTimeCryptoComparison.java:15:43:15:51 | actualMac | semmle.label | actualMac |
| NotConstantTimeCryptoComparison.java:28:36:28:50 | digest(...) : byte[] | semmle.label | digest(...) : byte[] |
| NotConstantTimeCryptoComparison.java:29:16:29:21 | actual | semmle.label | actual |
| NotConstantTimeCryptoComparison.java:44:28:44:40 | sign(...) : byte[] | semmle.label | sign(...) : byte[] |
| NotConstantTimeCryptoComparison.java:45:40:45:48 | signature | semmle.label | signature |
| NotConstantTimeCryptoComparison.java:61:22:61:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
| NotConstantTimeCryptoComparison.java:62:40:62:42 | tag | semmle.label | tag |
| NotConstantTimeCryptoComparison.java:30:28:30:40 | sign(...) : byte[] | semmle.label | sign(...) : byte[] |
| NotConstantTimeCryptoComparison.java:31:40:31:48 | signature | semmle.label | signature |
| NotConstantTimeCryptoComparison.java:47:22:47:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] |
| NotConstantTimeCryptoComparison.java:48:40:48:42 | tag | semmle.label | tag |
#select
| NotConstantTimeCryptoComparison.java:15:43:15:51 | actualMac | NotConstantTimeCryptoComparison.java:14:28:14:44 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:15:43:15:51 | actualMac | Using a not-constant time algorithm for comparison results of a cryptographic operation. |
| NotConstantTimeCryptoComparison.java:29:16:29:21 | actual | NotConstantTimeCryptoComparison.java:28:36:28:50 | digest(...) : byte[] | NotConstantTimeCryptoComparison.java:29:16:29:21 | actual | Using a not-constant time algorithm for comparison results of a cryptographic operation. |
| NotConstantTimeCryptoComparison.java:45:40:45:48 | signature | NotConstantTimeCryptoComparison.java:44:28:44:40 | sign(...) : byte[] | NotConstantTimeCryptoComparison.java:45:40:45:48 | signature | Using a not-constant time algorithm for comparison results of a cryptographic operation. |
| NotConstantTimeCryptoComparison.java:62:40:62:42 | tag | NotConstantTimeCryptoComparison.java:61:22:61:46 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:62:40:62:42 | tag | Using a not-constant time algorithm for comparison results of a cryptographic operation. |
| NotConstantTimeCryptoComparison.java:31:40:31:48 | signature | NotConstantTimeCryptoComparison.java:30:28:30:40 | sign(...) : byte[] | NotConstantTimeCryptoComparison.java:31:40:31:48 | signature | Using a not-constant time algorithm for comparison results of a cryptographic operation. |
| NotConstantTimeCryptoComparison.java:48:40:48:42 | tag | NotConstantTimeCryptoComparison.java:47:22:47:46 | doFinal(...) : byte[] | NotConstantTimeCryptoComparison.java:48:40:48:42 | tag | Using a not-constant time algorithm for comparison results of a cryptographic operation. |

14
java/ql/test/experimental/query-tests/security/CWE-208/NotConstantTimeCryptoComparison.java
View File

@@ -22,20 +22,6 @@ public class NotConstantTimeCryptoComparison {
return MessageDigest.isEqual(expectedMac, actualMac);
}
// BAD: compare hashes using a not-constant time method
public boolean unsafeCheckMessageDigest(String expectedHash, byte[] data) throws Exception {
MessageDigest md = MessageDigest.getInstance("SHA-256");
String actual = new String(md.digest(data));
return actual.equals(expectedHash);
}
// GOOD: compare hashes using a constant time method
public boolean saferCheckMessageDigest(byte[] expected, byte[] data) throws Exception {
MessageDigest md = MessageDigest.getInstance("SHA-256");
byte[] actual = md.digest(data);
return MessageDigest.isEqual(expected, actual);
}
// BAD: compare signatures using a not-constant time method
public boolean unsafeCheckSignatures(byte[] expected, byte[] data, PrivateKey key) throws Exception {
Signature engine = Signature.getInstance("SHA256withRSA");