Crypto: Misc. refactoring and code clean up. #19552
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
Refactors and cleans up the OpenSSL QL library by removing redundant filters and imports, simplifying operation constructors, improving module import paths, and adding documentation for context flow and padding constants.
- Simplified constructors by removing
isPossibleOpenSSLFunctionguards and eliminated unusedLibraryDetectorimports. - Switched to relative module imports and added docstrings for CTX flow classes.
- Introduced
OpenSSLPaddingLiteralfor padding constants and cleaned up algorithm instance mappings.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll | Removed LibraryDetector import, simplified hash operation constructors |
| cpp/ql/lib/experimental/quantum/OpenSSL/Operations/ECKeyGenOperation.qll | Removed LibraryDetector import, simplified keygen constructor |
| cpp/ql/lib/experimental/quantum/OpenSSL/OpenSSL.qll | Collapsed imports to relative module paths |
| cpp/ql/lib/experimental/quantum/OpenSSL/CtxFlow.qll | Added documentation comments for CTX types and flow |
| cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/PaddingAlgorithmValueConsumer.qll | Removed LibraryDetector import, simplified predicate |
| cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/PKeyAlgorithmValueConsumer.qll | Removed LibraryDetector import |
| cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/OpenSSLAlgorithmValueConsumerBase.qll | Removed unused DataFlow import |
| cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/EllipticCurveAlgorithmValueConsumer.qll | Removed LibraryDetector import |
| cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/CipherAlgorithmValueConsumer.qll | Removed LibraryDetector import |
| cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/PaddingAlgorithmInstance.qll | Introduced OpenSSLPaddingLiteral, cleaned up comment and mapping |
| cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/KnownAlgorithmConstants.qll | Removed LibraryDetector import, changed match patterns to uppercase |
| cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/CipherAlgorithmInstance.qll | Fixed typo in comment |
| cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/BlockAlgorithmInstance.qll | Fixed typo in comment |
| cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/AlgToAVCFlow.qll | Added import for PaddingAlgorithmInstance, adjusted flow config |
| cpp/ql/lib/experimental/quantum/Language.qll | Excluded zero values from algorithm constants |
Comments suppressed due to low confidence (3)
cpp/ql/lib/experimental/quantum/OpenSSL/CtxFlow.qll:83
- A
Callnode can never be an instance ofCTXPointerExpr(which extendsExpr), so this predicate will always fail. Remove or adjust this condition to target the correct class.
this instanceof CTXPointerExpr
cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll:44
- The constructor no longer checks
isPossibleOpenSSLFunction, which may lead to false positives for non-OpenSSL calls. Consider reintroducing a guard to verify the target belongs to the OpenSSL library.
EVP_Q_Digest_Operation() { this.(Call).getTarget().getName() = "EVP_Q_digest" }
cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/KnownAlgorithmConstants.qll:22
- Changing to uppercase pattern matching may break case-sensitive matches, since
algTypeis derived from.toLowerCase(). Use a case-insensitive pattern or match against lowercase.
algType.matches("%ENCRYPTION")
cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/AlgToAVCFlow.qll
Show resolved
Hide resolved
nicolaswill
previously approved these changes
May 21, 2025
| * # define RSA_PKCS1_WITH_TLS_PADDING 7 | ||
| * # define RSA_PKCS1_NO_IMPLICIT_REJECT_PADDING 8 | ||
| */ | ||
| class OpenSSLPaddingLiteral extends Literal { |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/PaddingAlgorithmInstance.qll
Fixed
Show resolved
Hide resolved
cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/PaddingAlgorithmInstance.qll
Fixed
Show resolved
Hide resolved
...l/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/PaddingAlgorithmValueConsumer.qll
Fixed
Show fixed
Hide fixed
| /** | ||
| * Flow from any CtxPointerArgument to any other CtxPointerArgument | ||
| */ | ||
| module OpenSSLCtxArgumentFlowConfig implements DataFlow::ConfigSig { |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
| ) | ||
| } | ||
| } | ||
|
|
||
| module OpenSSLCTXArgumentFlow = DataFlow::Global<OpenSSLCTXArgumentFlowConfig>; | ||
| module OpenSSLCtxArgumentFlow = DataFlow::Global<OpenSSLCtxArgumentFlowConfig>; |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase. Warning
nicolaswill
approved these changes
May 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.