Skip to content

Commit

Permalink
Ruby: weak crypto: do not report weak hash algorithms
Browse files Browse the repository at this point in the history
Weak hash algorithms such as MD5 and SHA1 are often
used in non security sensitive contexts and reporting
all uses is far too noisy.
  • Loading branch information
aibaars committed Nov 4, 2022
1 parent 265838a commit 98f4c29
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 21 deletions.
4 changes: 4 additions & 0 deletions ruby/ql/src/change-notes/2022-11-04-weak-crypto-hash.md
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `rb/weak-cryptographic-algorithm` has been updated to no longer report uses of hash functions such as `MD5` and `SHA1` even if they are known to be weak. These hash algorithms are used very often in non-sensitive contexts, making the query too imprecise in practice.
8 changes: 6 additions & 2 deletions ruby/ql/src/queries/security/cwe-327/BrokenCryptoAlgorithm.ql
Expand Up @@ -15,8 +15,12 @@ import codeql.ruby.Concepts

from Cryptography::CryptographicOperation operation, string msgPrefix
where
operation.getAlgorithm().isWeak() and
msgPrefix = "The cryptographic algorithm " + operation.getAlgorithm().getName()
exists(Cryptography::CryptographicAlgorithm algorithm |
algorithm = operation.getAlgorithm() and
algorithm.isWeak() and
msgPrefix = "The cryptographic algorithm " + algorithm.getName() and
not algorithm instanceof Cryptography::HashingAlgorithm
)
or
operation.getBlockMode().isWeak() and msgPrefix = "The block mode " + operation.getBlockMode()
select operation, msgPrefix + " is broken or weak, and should not be used."
Expand Up @@ -17,13 +17,3 @@
| broken_crypto.rb:75:1:75:24 | call to new | The cryptographic algorithm RC4 is broken or weak, and should not be used. |
| broken_crypto.rb:77:1:77:29 | call to new | The cryptographic algorithm RC4 is broken or weak, and should not be used. |
| broken_crypto.rb:79:1:79:35 | call to new | The cryptographic algorithm RC4 is broken or weak, and should not be used. |
| broken_crypto.rb:81:1:81:28 | call to hexdigest | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
| broken_crypto.rb:84:1:84:31 | call to base64digest | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
| broken_crypto.rb:87:1:87:20 | call to digest | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
| broken_crypto.rb:89:1:89:21 | call to update | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
| broken_crypto.rb:90:1:90:17 | ... << ... | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
| broken_crypto.rb:95:1:95:34 | call to bubblebabble | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
| broken_crypto.rb:97:11:97:37 | call to file | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
| broken_crypto.rb:103:1:103:21 | call to digest | The cryptographic algorithm SHA1 is broken or weak, and should not be used. |
| broken_crypto.rb:104:1:104:17 | ... << ... | The cryptographic algorithm SHA1 is broken or weak, and should not be used. |
| broken_crypto.rb:106:1:106:37 | call to digest | The cryptographic algorithm SHA1 is broken or weak, and should not be used. |
18 changes: 9 additions & 9 deletions ruby/ql/test/query-tests/security/cwe-327/broken_crypto.rb
Expand Up @@ -78,30 +78,30 @@
# BAD: weak encryption algorithm
OpenSSL::Cipher::RC4.new 'hmac-md5'

Digest::MD5.hexdigest('foo') # BAD: weak hash algorithm
Digest::MD5.hexdigest('foo') # OK: don't report hash algorithm even if it is weak
Digest::SHA256.hexdigest('foo') # GOOD: strong hash algorithm

Digest::MD5.base64digest('foo') # BAD: weak hash algorithm
Digest::MD5.base64digest('foo') # OK: don't report hash algorithm even if it is weak

md5 = Digest::MD5.new
md5.digest 'message' # BAD: weak hash algorithm
md5.digest 'message' # OK: don't report hash algorithm even if it is weak

md5.update 'message1' # BAD: weak hash algorithm
md5.update 'message1' # # OK: don't report hash algorithm even if it is weak
md5 << 'message2' # << is an alias for update

sha256 = Digest::SHA256.new
sha256.digest 'message' # GOOD: strong hash algorithm

Digest::MD5.bubblebabble 'message' # BAD: weak hash algorithm
Digest::MD5.bubblebabble 'message' # OK: don't report hash algorithm even if it is weak

filemd5 = Digest::MD5.file 'testfile'
filemd5 = Digest::MD5.file 'testfile' # OK: don't report hash algorithm even if it is weak
filemd5.hexdigest

Digest("MD5").hexdigest('foo') # BAD: weak hash algorithm
Digest("MD5").hexdigest('foo') # OK: don't report hash algorithm even if it is weak

sha1 = OpenSSL::Digest.new('SHA1')
sha1.digest 'message' # BAD: weak hash algorithm
sha1.digest 'message' # OK: don't report hash algorithm even if it is weak
sha1 << 'message' # << is an alias for update

OpenSSL::Digest.digest('SHA1', "abc") # BAD: weak hash algorithm
OpenSSL::Digest.digest('SHA1', "abc") # OK: don't report hash algorithm even if it is weak
OpenSSL::Digest.digest('SHA3-512', "abc") # GOOD: strong hash algorithm

0 comments on commit 98f4c29

Please sign in to comment.