-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: Weak encryption algorithm query. #18226
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
Merged
Merged
Changes from 14 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
bdb2f3d
Rust: Add placeholder query and tests for 'cipher' module.
geoffw0 6c4e0a9
Rust: A few more test cases.
geoffw0 07e3421
Rust: Add shared ConceptsShared.qll, CryptoAlgorithms.qll and CryptoA…
geoffw0 eeeb142
Rust: Implement the query.
geoffw0 94dbad7
Rust: Model for cipher traits.
geoffw0 6eb850c
Rust: Improve the model.
geoffw0 dd0fa79
Rust: Add qhelp.
geoffw0 de042ea
Merge branch 'main' into badcrypto
geoffw0 4e418d3
Rust: Update for latest main, and autoformat.
geoffw0 129f21a
Rust: Make a predicate private.
geoffw0 f637b3b
Apply suggestions from code review
geoffw0 4b93325
Merge branch 'main' into badcrypto
geoffw0 ad75906
Apply suggestions from code review
geoffw0 591db05
Rust: Formatting.
geoffw0 d2cfcb4
Update rust/ql/lib/codeql/rust/internal/ConceptsShared.qll
geoffw0 1d72b75
Rust: data-flow -> data flow.
geoffw0 611d04e
Rust: Revert stylistic change in shared file.
geoffw0 44a0ad2
Update data-flow -> data flow in all versions of ConceptsShared.qll.
geoffw0 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| /** | ||
| * Provides modeling for the `RustCrypto` family of crates (`cipher`, `digest` etc). | ||
| */ | ||
|
|
||
| private import rust | ||
| private import codeql.rust.Concepts | ||
| private import codeql.rust.dataflow.DataFlow | ||
|
|
||
| bindingset[algorithmName] | ||
| private string simplifyAlgorithmName(string algorithmName) { | ||
| // the cipher library gives triple-DES names like "TdesEee2" and "TdesEde2" | ||
| if algorithmName.matches("Tdes%") then result = "3des" else result = algorithmName | ||
| } | ||
|
|
||
| /** | ||
| * An operation that initializes a cipher through the `cipher::KeyInit` or | ||
| * `cipher::KeyIvInit` trait, for example `Des::new` or `cbc::Encryptor<des::Des>::new`. | ||
| */ | ||
| class StreamCipherInit extends Cryptography::CryptographicOperation::Range { | ||
| string algorithmName; | ||
|
|
||
| StreamCipherInit() { | ||
| // a call to `cipher::KeyInit::new`, `cipher::KeyInit::new_from_slice`, | ||
| // `cipher::KeyIvInit::new`, `cipher::KeyIvInit::new_from_slices` or `rc2::Rc2::new_with_eff_key_len`. | ||
| exists(PathExpr p, string rawAlgorithmName | | ||
| this.asExpr().getExpr().(CallExpr).getFunction() = p and | ||
| p.getResolvedCrateOrigin().matches("%/RustCrypto%") and | ||
| p.getPath().getPart().getNameRef().getText() = | ||
| ["new", "new_from_slice", "new_from_slices", "new_with_eff_key_len"] and | ||
| ( | ||
| rawAlgorithmName = p.getPath().getQualifier().getPart().getNameRef().getText() or | ||
| rawAlgorithmName = | ||
| p.getPath() | ||
| .getQualifier() | ||
| .getPart() | ||
| .getGenericArgList() | ||
| .getGenericArg(0) | ||
| .(TypeArg) | ||
| .getTypeRepr() | ||
| .(PathTypeRepr) | ||
| .getPath() | ||
| .getPart() | ||
| .getNameRef() | ||
| .getText() | ||
| ) and | ||
| algorithmName = simplifyAlgorithmName(rawAlgorithmName) | ||
| ) | ||
| } | ||
|
|
||
| override DataFlow::Node getInitialization() { result = this } | ||
|
|
||
| override Cryptography::CryptographicAlgorithm getAlgorithm() { result.matchesName(algorithmName) } | ||
|
|
||
| override DataFlow::Node getAnInput() { none() } | ||
|
|
||
| override Cryptography::BlockMode getBlockMode() { result = "" } | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| /** | ||
| * This file contains imports required for the Rust version of `ConceptsShared.qll`. | ||
| * Since they are language-specific, they can't be placed directly in that file, as it is shared between languages. | ||
| */ | ||
|
|
||
| import codeql.rust.dataflow.DataFlow::DataFlow as DataFlow | ||
| import codeql.rust.security.CryptoAlgorithms as CryptoAlgorithms |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,181 @@ | ||
| /** | ||
| * Provides Concepts which are shared across languages. | ||
| * | ||
| * Each language has a language specific `Concepts.qll` file that can import the | ||
| * shared concepts from this file. A language can either re-export the concept directly, | ||
| * or can add additional member-predicates that are needed for that language. | ||
| * | ||
| * Moving forward, `Concepts.qll` will be the staging ground for brand new concepts from | ||
| * each language, but we will maintain a discipline of moving those concepts to | ||
| * `ConceptsShared.qll` ASAP. | ||
| */ | ||
|
|
||
| private import ConceptsImports | ||
|
|
||
| /** | ||
| * Provides models for cryptographic concepts. | ||
| * | ||
| * Note: The `CryptographicAlgorithm` class currently doesn't take weak keys into | ||
| * consideration for the `isWeak` member predicate. So RSA is always considered | ||
| * secure, although using a low number of bits will actually make it insecure. We plan | ||
| * to improve our libraries in the future to more precisely capture this aspect. | ||
| */ | ||
| module Cryptography { | ||
| class CryptographicAlgorithm = CryptoAlgorithms::CryptographicAlgorithm; | ||
|
|
||
| class EncryptionAlgorithm = CryptoAlgorithms::EncryptionAlgorithm; | ||
|
|
||
| class HashingAlgorithm = CryptoAlgorithms::HashingAlgorithm; | ||
|
|
||
| class PasswordHashingAlgorithm = CryptoAlgorithms::PasswordHashingAlgorithm; | ||
|
|
||
| /** | ||
| * A data-flow node that is an application of a cryptographic algorithm. For example, | ||
geoffw0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| * encryption, decryption, signature-validation. | ||
| * | ||
| * Extend this class to refine existing API models. If you want to model new APIs, | ||
| * extend `CryptographicOperation::Range` instead. | ||
| */ | ||
| class CryptographicOperation extends DataFlow::Node instanceof CryptographicOperation::Range { | ||
| /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */ | ||
| CryptographicAlgorithm getAlgorithm() { result = super.getAlgorithm() } | ||
|
|
||
| /** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */ | ||
| DataFlow::Node getInitialization() { result = super.getInitialization() } | ||
|
|
||
| /** Gets an input the algorithm is used on, for example the plain text input to be encrypted. */ | ||
| DataFlow::Node getAnInput() { result = super.getAnInput() } | ||
|
|
||
| /** | ||
| * Gets the block mode used to perform this cryptographic operation. | ||
| * | ||
| * This predicate is only expected to have a result if two conditions hold: | ||
| * 1. The operation is an encryption operation, i.e. the algorithm used is an `EncryptionAlgorithm`, and | ||
| * 2. The algorithm used is a block cipher (not a stream cipher). | ||
| * | ||
| * If either of these conditions do not hold, then this predicate should have no result. | ||
| */ | ||
| BlockMode getBlockMode() { result = super.getBlockMode() } | ||
| } | ||
|
|
||
| /** Provides classes for modeling new applications of a cryptographic algorithms. */ | ||
| module CryptographicOperation { | ||
| /** | ||
| * A data-flow node that is an application of a cryptographic algorithm. For example, | ||
| * encryption, decryption, signature-validation. | ||
| * | ||
| * Extend this class to model new APIs. If you want to refine existing API models, | ||
| * extend `CryptographicOperation` instead. | ||
| */ | ||
| abstract class Range extends DataFlow::Node { | ||
| /** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */ | ||
| abstract DataFlow::Node getInitialization(); | ||
|
|
||
| /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */ | ||
| abstract CryptographicAlgorithm getAlgorithm(); | ||
|
|
||
| /** Gets an input the algorithm is used on, for example the plain text input to be encrypted. */ | ||
| abstract DataFlow::Node getAnInput(); | ||
|
|
||
| /** | ||
| * Gets the block mode used to perform this cryptographic operation. | ||
| * | ||
| * This predicate is only expected to have a result if two conditions hold: | ||
| * 1. The operation is an encryption operation, i.e. the algorithm used is an `EncryptionAlgorithm`, and | ||
| * 2. The algorithm used is a block cipher (not a stream cipher). | ||
| * | ||
| * If either of these conditions do not hold, then this predicate should have no result. | ||
| */ | ||
| abstract BlockMode getBlockMode(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * A cryptographic block cipher mode of operation. This can be used to encrypt | ||
| * data of arbitrary length using a block encryption algorithm. | ||
| */ | ||
| class BlockMode extends string { | ||
| BlockMode() { | ||
| this = | ||
| [ | ||
| "ECB", "CBC", "GCM", "CCM", "CFB", "OFB", "CTR", "OPENPGP", | ||
| "XTS", // https://csrc.nist.gov/publications/detail/sp/800-38e/final | ||
| "EAX" // https://en.wikipedia.org/wiki/EAX_mode | ||
| ] | ||
| } | ||
|
|
||
| /** Holds if this block mode is considered to be insecure. */ | ||
| predicate isWeak() { this = "ECB" } | ||
|
|
||
| /** Holds if the given string appears to match this block mode. */ | ||
| bindingset[s] | ||
| predicate matchesString(string s) { s.toUpperCase().matches("%" + this + "%") } | ||
| } | ||
| } | ||
|
|
||
| /** Provides classes for modeling HTTP-related APIs. */ | ||
| module Http { | ||
| /** Provides classes for modeling HTTP clients. */ | ||
| module Client { | ||
| /** | ||
| * A data-flow node that makes an outgoing HTTP request. | ||
| * | ||
| * Extend this class to refine existing API models. If you want to model new APIs, | ||
| * extend `Http::Client::Request::Range` instead. | ||
| */ | ||
| class Request extends DataFlow::Node instanceof Request::Range { | ||
| /** | ||
| * Gets a data-flow node that contributes to the URL of the request. | ||
| * Depending on the framework, a request may have multiple nodes which contribute to the URL. | ||
| */ | ||
| DataFlow::Node getAUrlPart() { result = super.getAUrlPart() } | ||
|
|
||
| /** Gets a string that identifies the framework used for this request. */ | ||
| string getFramework() { result = super.getFramework() } | ||
|
|
||
| /** | ||
| * Holds if this request is made using a mode that disables SSL/TLS | ||
| * certificate validation, where `disablingNode` represents the point at | ||
| * which the validation was disabled, and `argumentOrigin` represents the origin | ||
| * of the argument that disabled the validation (which could be the same node as | ||
| * `disablingNode`). | ||
| */ | ||
| predicate disablesCertificateValidation( | ||
| DataFlow::Node disablingNode, DataFlow::Node argumentOrigin | ||
| ) { | ||
| super.disablesCertificateValidation(disablingNode, argumentOrigin) | ||
| } | ||
| } | ||
|
|
||
| /** Provides a class for modeling new HTTP requests. */ | ||
| module Request { | ||
| /** | ||
| * A data-flow node that makes an outgoing HTTP request. | ||
| * | ||
| * Extend this class to model new APIs. If you want to refine existing API models, | ||
| * extend `Http::Client::Request` instead. | ||
| */ | ||
| abstract class Range extends DataFlow::Node { | ||
| /** | ||
| * Gets a data-flow node that contributes to the URL of the request. | ||
| * Depending on the framework, a request may have multiple nodes which contribute to the URL. | ||
| */ | ||
| abstract DataFlow::Node getAUrlPart(); | ||
|
|
||
| /** Gets a string that identifies the framework used for this request. */ | ||
| abstract string getFramework(); | ||
|
|
||
| /** | ||
| * Holds if this request is made using a mode that disables SSL/TLS | ||
| * certificate validation, where `disablingNode` represents the point at | ||
| * which the validation was disabled, and `argumentOrigin` represents the origin | ||
| * of the argument that disabled the validation (which could be the same node as | ||
| * `disablingNode`). | ||
| */ | ||
| abstract predicate disablesCertificateValidation( | ||
| DataFlow::Node disablingNode, DataFlow::Node argumentOrigin | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.