-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Many languages: Update broken algo qhelp #20603
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.
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
base: main
Are you sure you want to change the base?
Conversation
Previously it focussed too much on the risk of data being decrypted, and didn't explain why using weak algorithms is a problem in other contexts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the qhelp documentation for broken cryptographic algorithm queries across multiple programming languages to provide a more comprehensive explanation of the security risks beyond just data decryption.
- Updates the overview text to explain that weak cryptographic algorithms may compromise confidentiality, integrity, and authenticity
- Adds specific examples of risks for encryption, hashing, and digital signature algorithms
- Standardizes the documentation across Rust, Ruby, Python, JavaScript, Java, and C++ implementations
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
rust/ql/src/queries/security/CWE-327/BrokenCryptoAlgorithm.qhelp | Enhanced overview with broader security context and specific risk examples |
ruby/ql/src/queries/security/cwe-327/BrokenCryptoAlgorithm.qhelp | Updated documentation with comprehensive security risk explanations |
python/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.qhelp | Improved overview text with detailed risk scenarios |
javascript/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.qhelp | Enhanced documentation with broader security risk context |
java/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.qhelp | Updated overview with comprehensive security guarantees explanation |
cpp/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.qhelp | Enhanced documentation with detailed security risk examples |
QHelp previews: cpp/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.qhelpUse of a broken or risky cryptographic algorithmUsing broken or weak cryptographic algorithms may compromise security guarantees such as confidentiality, integrity, and authenticity. Many cryptographic algorithms are known to be weak or flawed. The security guarantees of a system often rely on the underlying cryptography, so using a weak algorithm can have severe consequences. For example:
RecommendationEnsure that you use a strong, modern cryptographic algorithm. Use at least AES-128 or RSA-2048. ExampleThe following code shows an example of using the void advapi() {
HCRYPTPROV hCryptProv;
HCRYPTKEY hKey;
HCRYPTHASH hHash;
// other preparation goes here
// BAD: use 3DES for key
CryptDeriveKey(hCryptProv, CALG_3DES, hHash, 0, &hKey);
// GOOD: use AES
CryptDeriveKey(hCryptProv, CALG_AES_256, hHash, 0, &hKey);
}
References
java/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.qhelpUse of a broken or risky cryptographic algorithmUsing broken or weak cryptographic algorithms may compromise security guarantees such as confidentiality, integrity, and authenticity. Many cryptographic algorithms are known to be weak or flawed. The security guarantees of a system often rely on the underlying cryptography, so using a weak algorithm can have severe consequences. For example:
RecommendationEnsure that you use a strong, modern cryptographic algorithm. Use at least AES-128 or RSA-2048. Do not use the ECB encryption mode since it is vulnerable to replay and other attacks. ExampleThe following code shows an example of using a java // BAD: DES is a weak algorithm
Cipher des = Cipher.getInstance("DES");
cipher.init(Cipher.ENCRYPT_MODE, secretKeySpec);
byte[] encrypted = cipher.doFinal(input.getBytes("UTF-8"));
// ...
// GOOD: AES is a strong algorithm
Cipher aes = Cipher.getInstance("AES");
// ... References
javascript/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.qhelpUse of a broken or weak cryptographic algorithmUsing broken or weak cryptographic algorithms may compromise security guarantees such as confidentiality, integrity, and authenticity. Many cryptographic algorithms are known to be weak or flawed. The security guarantees of a system often rely on the underlying cryptography, so using a weak algorithm can have severe consequences. For example:
RecommendationEnsure that you use a strong, modern cryptographic algorithm. Use at least AES-128 or RSA-2048 for encryption, and SHA-2 or SHA-3 for secure hashing. ExampleThe following code shows an example of using the builtin cryptographic library of NodeJS to encrypt some secret data. When creating a const crypto = require('crypto');
var secretText = obj.getSecretText();
const desCipher = crypto.createCipher('des', key);
let desEncrypted = desCipher.write(secretText, 'utf8', 'hex'); // BAD: weak encryption
const aesCipher = crypto.createCipher('aes-128', key);
let aesEncrypted = aesCipher.update(secretText, 'utf8', 'hex'); // GOOD: strong encryption References
python/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.qhelpUse of a broken or weak cryptographic algorithmUsing broken or weak cryptographic algorithms may compromise security guarantees such as confidentiality, integrity, and authenticity. Many cryptographic algorithms are known to be weak or flawed. The security guarantees of a system often rely on the underlying cryptography, so using a weak algorithm can have severe consequences. For example:
RecommendationEnsure that you use a strong, modern cryptographic algorithm, such as AES-128 or RSA-2048. ExampleThe following code uses the from Crypto.Cipher import DES, AES
cipher = DES.new(SECRET_KEY)
def send_encrypted(channel, message):
channel.send(cipher.encrypt(message)) # BAD: weak encryption
cipher = AES.new(SECRET_KEY)
def send_encrypted(channel, message):
channel.send(cipher.encrypt(message)) # GOOD: strong encryption
NOTICE: the original References
ruby/ql/src/queries/security/cwe-327/BrokenCryptoAlgorithm.qhelpUse of a broken or weak cryptographic algorithmUsing broken or weak cryptographic algorithms may compromise security guarantees such as confidentiality, integrity, and authenticity. Many cryptographic algorithms are known to be weak or flawed. The security guarantees of a system often rely on the underlying cryptography, so using a weak algorithm can have severe consequences. For example:
RecommendationEnsure that you use a strong, modern cryptographic algorithm, such as AES-128 or RSA-2048. ExampleThe following code uses the require 'openssl'
class Encryptor
attr_accessor :secret_key
def encrypt_message_weak(message)
cipher = OpenSSL::Cipher.new('des') # BAD: weak encryption
cipher.encrypt
cipher.key = secret_key
cipher.update(message)
cipher.final
end
def encrypt_message_strong(message)
cipher = OpenSSL::Cipher::AES128.new # GOOD: strong encryption
cipher.encrypt
cipher.key = secret_key
cipher.update(message)
cipher.final
end
end References
rust/ql/src/queries/security/CWE-327/BrokenCryptoAlgorithm.qhelpUse of a broken or weak cryptographic algorithmUsing broken or weak cryptographic algorithms may compromise security guarantees such as confidentiality, integrity, and authenticity. Many cryptographic algorithms are known to be weak or flawed. The security guarantees of a system often rely on the underlying cryptography, so using a weak algorithm can have severe consequences. For example:
RecommendationEnsure that you use a strong, modern cryptographic algorithm, such as AES-128 or RSA-2048. ExampleThe following code uses the let des_cipher = cbc::Encryptor::<des::Des>::new(key.into(), iv.into()); // BAD: weak encryption
let encryption_result = des_cipher.encrypt_padded_mut::<des::cipher::block_padding::Pkcs7>(data, data_len); Instead, we should use a strong modern algorithm. In this case, we have selected the 256-bit version of the AES algorithm. let aes_cipher = cbc::Encryptor::<aes::Aes256>::new(key.into(), iv.into()); // GOOD: strong encryption
let encryption_result = aes_cipher.encrypt_padded_mut::<aes::cipher::block_padding::Pkcs7>(data, data_len); References
|
That's a nice improvement! Would it be useful/possible to extract the common text in a shared snippet that is imported in the various qhelp files? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C/C++ 👍
Rust 👍 - personally I would delete the bullet point about weak hashing from this file, but I'm also OK with this being merged as is (after a docs review that is) --- update: it's deleted now :)
rust/ql/src/queries/security/CWE-327/BrokenCryptoAlgorithm.qhelp
Outdated
Show resolved
Hide resolved
@aibaars My only issue with that is where it should go. A new shared library with things related to (security) queries? I guess we will want one at some point, if we implement concepts further. Or in the shared concepts library, by the crypto information? I was about to say that most of the languages already depend on that, but I see that it's the library qlpacks that depend on it, and we would be adding this dependency to the source qlpacks. |
Currently it focusses too much on the risk of data being decrypted, and doesn't explain why using weak algorithms can be a problem in other contexts.
Note that many of these queries would only alert for encryption algorithms, so the current text makes sense for them. But
java/potentially-weak-cryptographic-algorithm
, which shares the same help text asjava/weak-cryptographic-algorithm
, alerts for more algorithms, and so doesjs/weak-cryptographic-algorithm
. It seems good to update all the qhelp files at once.