From 7f84cf6d72e23e78192de040aad73c8671e93fc4 Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Fri, 22 Nov 2024 18:56:18 +0100 Subject: [PATCH 1/3] Add test case --- .../CWE-327/semmle/tests/MaybeBrokenCryptoAlgorithm.expected | 2 ++ .../security/CWE-327/semmle/tests/WeakHashing.java | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/java/ql/test/query-tests/security/CWE-327/semmle/tests/MaybeBrokenCryptoAlgorithm.expected b/java/ql/test/query-tests/security/CWE-327/semmle/tests/MaybeBrokenCryptoAlgorithm.expected index da6f6312a896..a9365ae398dd 100644 --- a/java/ql/test/query-tests/security/CWE-327/semmle/tests/MaybeBrokenCryptoAlgorithm.expected +++ b/java/ql/test/query-tests/security/CWE-327/semmle/tests/MaybeBrokenCryptoAlgorithm.expected @@ -4,9 +4,11 @@ nodes | WeakHashing.java:15:55:15:83 | getProperty(...) | semmle.label | getProperty(...) | | WeakHashing.java:18:56:18:95 | getProperty(...) | semmle.label | getProperty(...) | | WeakHashing.java:21:56:21:91 | getProperty(...) | semmle.label | getProperty(...) | +| WeakHashing.java:30:55:30:64 | "SHA3-512" | semmle.label | "SHA3-512" | subpaths #select | Test.java:34:21:34:53 | new SecretKeySpec(...) | Test.java:34:48:34:52 | "foo" | Test.java:34:48:34:52 | "foo" | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | Test.java:34:48:34:52 | "foo" | foo | | WeakHashing.java:15:29:15:84 | getInstance(...) | WeakHashing.java:15:55:15:83 | getProperty(...) | WeakHashing.java:15:55:15:83 | getProperty(...) | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | WeakHashing.java:15:55:15:83 | getProperty(...) | MD5 | | WeakHashing.java:18:30:18:96 | getInstance(...) | WeakHashing.java:18:56:18:95 | getProperty(...) | WeakHashing.java:18:56:18:95 | getProperty(...) | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | WeakHashing.java:18:56:18:95 | getProperty(...) | MD5 | | WeakHashing.java:21:30:21:92 | getInstance(...) | WeakHashing.java:21:56:21:91 | getProperty(...) | WeakHashing.java:21:56:21:91 | getProperty(...) | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | WeakHashing.java:21:56:21:91 | getProperty(...) | MD5 | +| WeakHashing.java:30:29:30:65 | getInstance(...) | WeakHashing.java:30:55:30:64 | "SHA3-512" | WeakHashing.java:30:55:30:64 | "SHA3-512" | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | WeakHashing.java:30:55:30:64 | "SHA3-512" | SHA3-512 | diff --git a/java/ql/test/query-tests/security/CWE-327/semmle/tests/WeakHashing.java b/java/ql/test/query-tests/security/CWE-327/semmle/tests/WeakHashing.java index 6a3565fc1412..8858576cb904 100644 --- a/java/ql/test/query-tests/security/CWE-327/semmle/tests/WeakHashing.java +++ b/java/ql/test/query-tests/security/CWE-327/semmle/tests/WeakHashing.java @@ -25,5 +25,8 @@ void hashing() throws NoSuchAlgorithmException, IOException { // OK: Property does not exist and default is secure MessageDigest ok2 = MessageDigest.getInstance(props.getProperty("hashAlg3", "SHA-256")); + + // GOOD: Using a strong hashing algorithm + MessageDigest ok3 = MessageDigest.getInstance("SHA3-512"); } -} \ No newline at end of file +} From c6eaed343d25275580fa228c0c2013dc1a0af206 Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Fri, 22 Nov 2024 18:43:48 +0100 Subject: [PATCH 2/3] Java: add SHA3 family to list of secure crypto algorithms --- java/ql/lib/semmle/code/java/security/Encryption.qll | 2 +- java/ql/src/change-notes/2024-11-22-sha3.md | 4 ++++ .../CWE-327/semmle/tests/MaybeBrokenCryptoAlgorithm.expected | 2 -- 3 files changed, 5 insertions(+), 3 deletions(-) create mode 100644 java/ql/src/change-notes/2024-11-22-sha3.md diff --git a/java/ql/lib/semmle/code/java/security/Encryption.qll b/java/ql/lib/semmle/code/java/security/Encryption.qll index 6fc7f6b7d16a..80b41233bde3 100644 --- a/java/ql/lib/semmle/code/java/security/Encryption.qll +++ b/java/ql/lib/semmle/code/java/security/Encryption.qll @@ -250,7 +250,7 @@ string getASecureAlgorithmName() { result = [ "RSA", "SHA-?256", "SHA-?512", "CCM", "GCM", "AES(?![^a-zA-Z](ECB|CBC/PKCS[57]Padding))", - "Blowfish", "ECIES" + "Blowfish", "ECIES", "SHA3-(224|256|384|512)" ] } diff --git a/java/ql/src/change-notes/2024-11-22-sha3.md b/java/ql/src/change-notes/2024-11-22-sha3.md new file mode 100644 index 000000000000..61dbc35162e1 --- /dev/null +++ b/java/ql/src/change-notes/2024-11-22-sha3.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added SHA3 to the list of secure hashing algorithms. As a result the `java/potentially-weak-cryptographic-algorithm` query should no longer flag up uses of SHA3. diff --git a/java/ql/test/query-tests/security/CWE-327/semmle/tests/MaybeBrokenCryptoAlgorithm.expected b/java/ql/test/query-tests/security/CWE-327/semmle/tests/MaybeBrokenCryptoAlgorithm.expected index a9365ae398dd..da6f6312a896 100644 --- a/java/ql/test/query-tests/security/CWE-327/semmle/tests/MaybeBrokenCryptoAlgorithm.expected +++ b/java/ql/test/query-tests/security/CWE-327/semmle/tests/MaybeBrokenCryptoAlgorithm.expected @@ -4,11 +4,9 @@ nodes | WeakHashing.java:15:55:15:83 | getProperty(...) | semmle.label | getProperty(...) | | WeakHashing.java:18:56:18:95 | getProperty(...) | semmle.label | getProperty(...) | | WeakHashing.java:21:56:21:91 | getProperty(...) | semmle.label | getProperty(...) | -| WeakHashing.java:30:55:30:64 | "SHA3-512" | semmle.label | "SHA3-512" | subpaths #select | Test.java:34:21:34:53 | new SecretKeySpec(...) | Test.java:34:48:34:52 | "foo" | Test.java:34:48:34:52 | "foo" | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | Test.java:34:48:34:52 | "foo" | foo | | WeakHashing.java:15:29:15:84 | getInstance(...) | WeakHashing.java:15:55:15:83 | getProperty(...) | WeakHashing.java:15:55:15:83 | getProperty(...) | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | WeakHashing.java:15:55:15:83 | getProperty(...) | MD5 | | WeakHashing.java:18:30:18:96 | getInstance(...) | WeakHashing.java:18:56:18:95 | getProperty(...) | WeakHashing.java:18:56:18:95 | getProperty(...) | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | WeakHashing.java:18:56:18:95 | getProperty(...) | MD5 | | WeakHashing.java:21:30:21:92 | getInstance(...) | WeakHashing.java:21:56:21:91 | getProperty(...) | WeakHashing.java:21:56:21:91 | getProperty(...) | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | WeakHashing.java:21:56:21:91 | getProperty(...) | MD5 | -| WeakHashing.java:30:29:30:65 | getInstance(...) | WeakHashing.java:30:55:30:64 | "SHA3-512" | WeakHashing.java:30:55:30:64 | "SHA3-512" | Cryptographic algorithm $@ may not be secure, consider using a different algorithm. | WeakHashing.java:30:55:30:64 | "SHA3-512" | SHA3-512 | From 5eb91fd5168ae675bf1c8f6f89ae38641cc4da14 Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Mon, 25 Nov 2024 11:25:45 +0100 Subject: [PATCH 3/3] Drop SHA3-224 Drop the 224bits variant as it looks like SHA3-224 may be deprecated soon based on NIST's most recent draft revision of Transitioning the Use of Cryptographic Algorithms and Key Lengths --- java/ql/lib/semmle/code/java/security/Encryption.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/lib/semmle/code/java/security/Encryption.qll b/java/ql/lib/semmle/code/java/security/Encryption.qll index 80b41233bde3..e6608d85872a 100644 --- a/java/ql/lib/semmle/code/java/security/Encryption.qll +++ b/java/ql/lib/semmle/code/java/security/Encryption.qll @@ -250,7 +250,7 @@ string getASecureAlgorithmName() { result = [ "RSA", "SHA-?256", "SHA-?512", "CCM", "GCM", "AES(?![^a-zA-Z](ECB|CBC/PKCS[57]Padding))", - "Blowfish", "ECIES", "SHA3-(224|256|384|512)" + "Blowfish", "ECIES", "SHA3-(256|384|512)" ] }