Go: promote go/weak-crypto-algorithm
#20666
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR promotes the go/weak-crypto-algorithm query from experimental to standard by implementing comprehensive modeling of cryptographic operations in Go. The query now detects when sensitive data is used with broken or weak cryptographic algorithms (DES, 3DES, RC4, MD5, SHA1) across various crypto libraries and block cipher modes.
Key changes:
- Completely rewrote cryptographic operation modeling using the shared Concepts library
- Added support for block cipher modes (CBC, CTR, CFB, OFB, GCM) and stream operations
- Extended coverage to include hash operations with proper initialization tracking
- Added comprehensive test cases covering multiple encryption and hashing scenarios
Reviewed Changes
Copilot reviewed 20 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql | New production query file that replaces the experimental query |
| go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmQuery.qll | New taint-tracking configuration for the query |
| go/ql/lib/semmle/go/security/BrokenCryptoAlgorithmCustomizations.qll | New customizations defining sources, sinks, and sanitizers |
| go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll | Comprehensive modeling of Go crypto libraries with algorithm and block mode tracking |
| go/ql/lib/semmle/go/Concepts.qll | Added cryptography concepts including algorithm types and operation abstractions |
| go/ql/lib/qlpack.yml | Added dependency on codeql/concepts shared library |
| go/ql/lib/go.qll | Imported CryptoLibraries framework |
| shared/concepts/codeql/concepts/internal/CryptoAlgorithmNames.qll | Added SHA512224 and SHA512256 to strong hashing algorithms |
| go/ql/test/query-tests/Security/CWE-327/Crypto.go | Comprehensive test cases for various cryptographic operations |
| go/ql/src/experimental/CWE-327/* | Removed old experimental query files |
| go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.ql | New test helper for validating CryptographicOperation modeling |
| go/ql/src/change-notes/2025-10-21-go-weak-crypto-algorithm.md | Change note documenting the new query |
|
QHelp previews: go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.qhelpUse of a broken or weak cryptographic algorithmUsing weak cryptographic algorithms can leave data vulnerable to being decrypted or forged by an attacker. Many cryptographic algorithms provided by cryptography libraries are known to be weak. Using such an algorithm means that encrypted or hashed data is less secure than it appears to be. 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 uses the different packages to encrypt/hash some secret data. The first few examples uses DES, MD5, RC4, and SHA1, which are older algorithms that are now considered weak. The following examples use AES and SHA256, which are stronger, more modern algorithms. package main
import (
"crypto/aes"
"crypto/des"
"crypto/md5"
"crypto/rc4"
"crypto/sha1"
"crypto/sha256"
)
func main() {
public := []byte("hello")
password := []byte("123456")
buf := password // testing dataflow by passing into different variable
// BAD, des is a weak crypto algorithm and password is sensitive data
des.NewTripleDESCipher(buf)
// BAD, md5 is a weak crypto algorithm and password is sensitive data
md5.Sum(buf)
// BAD, rc4 is a weak crypto algorithm and password is sensitive data
rc4.NewCipher(buf)
// BAD, sha1 is a weak crypto algorithm and password is sensitive data
sha1.Sum(buf)
// GOOD, password is sensitive data but aes is a strong crypto algorithm
aes.NewCipher(buf)
// GOOD, password is sensitive data but sha256 is a strong crypto algorithm
sha256.Sum256(buf)
// GOOD, des is a weak crypto algorithm but public is not sensitive data
des.NewTripleDESCipher(public)
// GOOD, md5 is a weak crypto algorithm but public is not sensitive data
md5.Sum(public)
// GOOD, rc4 is a weak crypto algorithm but public is not sensitive data
rc4.NewCipher(public)
// GOOD, sha1 is a weak crypto algorithm but public is not sensitive data
sha1.Sum(public)
// GOOD, aes is a strong crypto algorithm and public is not sensitive data
aes.NewCipher(public)
// GOOD, sha256 is a strong crypto algorithm and public is not sensitive data
sha256.Sum256(public)
}References
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| private module Rc4 { | ||
| private class CipherXORKeyStream extends CryptographicOperation::Range instanceof DataFlow::CallNode |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
| } | ||
|
|
||
| private module Cipher { | ||
| private class NewCBCEncrypter extends StdLibNewEncrypter { |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
| override BlockMode getMode() { result = "CBC" } | ||
| } | ||
|
|
||
| private class NewCFBEncrypter extends StdLibNewEncrypter { |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
| override BlockMode getMode() { result = "CFB" } | ||
| } | ||
|
|
||
| private class NewCTR extends StdLibNewEncrypter { |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
| override BlockMode getMode() { result = "CTR" } | ||
| } | ||
|
|
||
| private class NewGCM extends StdLibNewEncrypter { |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
| override BlockMode getMode() { result = "GCM" } | ||
| } | ||
|
|
||
| private class NewOFB extends StdLibNewEncrypter { |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
| } | ||
| } | ||
|
|
||
| private class StreamXORKeyStream extends EncryptionMethodCall { |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
| Location getASelectedSinkLocation(DataFlow::Node sink) { | ||
| result = sink.(Sink).getLocation() | ||
| or | ||
| result = sink.(Sink).getInitialization().getLocation() |
There was a problem hiding this comment.
The query at go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql only selects source and sink, not getInitialization(). Try removing this location override (getASelectedSinkLocation) altogether.
There was a problem hiding this comment.
Thanks. This has highlighted some differences from the existing queries for other languages which I wasn't aware of. I'll need to update the PR quite a lot, so I'm putting it back in draft now.
owen-mc
force-pushed
the
go/promote-weak-crypto-algorithm
branch
from
d231a17 to
9e04cf3
Compare
| from Cryptography::CryptographicOperation operation, string msgPrefix, DataFlow::Node init | ||
| where | ||
| init = operation.getInitialization() and | ||
| // `init` may be a `BlockModeInit`, a `EncryptionAlgorithmInit`, or `operation` itself. | ||
| ( | ||
| not init instanceof BlockModeInit and | ||
| exists(Cryptography::CryptographicAlgorithm algorithm | | ||
| algorithm = operation.getAlgorithm() and | ||
| algorithm.isWeak() and | ||
| msgPrefix = "The cryptographic algorithm " + algorithm.getName() and | ||
| not algorithm instanceof Cryptography::HashingAlgorithm | ||
| ) | ||
| or | ||
| not init instanceof EncryptionAlgorithmInit and | ||
| operation.getBlockMode().isWeak() and | ||
| msgPrefix = "The block mode " + operation.getBlockMode() | ||
| ) | ||
| select operation, "$@ is broken or weak, and should not be used.", init, msgPrefix |
Check warning
Code scanning / CodeQL
Consistent alert message Warning
2 participants
Added a new query,
go/weak-crypto-algorithm, to detect the use of a broken or weak cryptographic algorithm. A very simple version of this query was originally contributed as an experimental query by @dilanbhalla.