Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ruby: Improve weak crypto query #11129

Merged
merged 1 commit into from
Nov 4, 2022
Merged

Ruby: Improve weak crypto query #11129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ruby/ql/src/change-notes/2022-11-04-weak-crypto-hash.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
View file
Open in desktop
Original file line number Diff line number Diff line change
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."
10 changes: 0 additions & 10 deletions ruby/ql/test/query-tests/security/cwe-327/BrokenCryptoAlgorithm.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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