Skip to content

Quantum: Add OpenSSL PKEY algorithm value consumers. #19547

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 2 commits into from
May 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 22 additions & 1 deletion cpp/ql/lib/experimental/quantum/Language.qll
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
private import cpp as Language
import semmle.code.cpp.dataflow.new.DataFlow
import semmle.code.cpp.dataflow.new.TaintTracking
import codeql.quantum.experimental.Model

module CryptoInput implements InputSig<Language::Location> {
Expand Down Expand Up @@ -86,6 +86,27 @@ module GenericDataSourceFlowConfig implements DataFlow::ConfigSig {
}
}

module GenericDataSourceFlow = TaintTracking::Global<GenericDataSourceFlowConfig>;

private class ConstantDataSource extends Crypto::GenericConstantSourceInstance instanceof Literal {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you test this with MRVA or DCA before merging? I worry the performance impact will be severe from this.

ConstantDataSource() {
// TODO: this is an API specific workaround for OpenSSL, as 'EC' is a constant that may be used
// where typical algorithms are specified, but EC specifically means set up a
// default curve container, that will later be specified explicitly (or if not a default)
// curve is used.
this.getValue() != "EC"
}

override DataFlow::Node getOutputNode() { result.asExpr() = this }

override predicate flowsTo(Crypto::FlowAwareElement other) {
// TODO: separate config to avoid blowing up data-flow analysis
GenericDataSourceFlow::flow(this.getOutputNode(), other.getInputNode())
}

override string getAdditionalDescription() { result = this.toString() }
}

module ArtifactUniversalFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source = any(Crypto::ArtifactInstance artifact).getOutputNode()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ import DirectAlgorithmValueConsumer
import PaddingAlgorithmValueConsumer
import HashAlgorithmValueConsumer
import EllipticCurveAlgorithmValueConsumer
import PKeyAlgorithmValueConsumer
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import cpp
private import experimental.quantum.Language
private import experimental.quantum.OpenSSL.LibraryDetector
private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmInstances

abstract class PKeyValueConsumer extends OpenSSLAlgorithmValueConsumer { }

class EVPPKeyAlgorithmConsumer extends PKeyValueConsumer {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in EVPPKeyAlgorithmConsumer should be PascalCase/camelCase.
DataFlow::Node valueArgNode;
DataFlow::Node resultNode;

EVPPKeyAlgorithmConsumer() {
resultNode.asExpr() = this.(Call) and // in all cases the result is the return
isPossibleOpenSSLFunction(this.(Call).getTarget()) and
(
// NOTE: some of these consumers are themselves key gen operations,
// in these cases, the operation will be created separately for the same function.
this.(Call).getTarget().getName() in [
"EVP_PKEY_CTX_new_id", "EVP_PKEY_new_raw_private_key", "EVP_PKEY_new_raw_public_key",
"EVP_PKEY_new_mac_key"
] and
valueArgNode.asExpr() = this.(Call).getArgument(0)
or
this.(Call).getTarget().getName() in [
"EVP_PKEY_CTX_new_from_name", "EVP_PKEY_new_raw_private_key_ex",
"EVP_PKEY_new_raw_public_key_ex", "EVP_PKEY_CTX_ctrl", "EVP_PKEY_CTX_set_group_name"
] and
valueArgNode.asExpr() = this.(Call).getArgument(1)
or
// argInd 2 is 'type' which can be RSA, or EC
// if RSA argInd 3 is the key size, else if EC argInd 3 is the curve name
// In all other cases there is no argInd 3, and argInd 2 is the algorithm.
// Since this is a key gen operation, handling the key size should be handled
// when the operation is again modeled as a key gen operation.
this.(Call).getTarget().getName() = "EVP_PKEY_Q_keygen" and
(
// Elliptic curve case
// If the argInd 3 is a derived type (pointer or array) then assume it is a curve name
if this.(Call).getArgument(3).getType().getUnderlyingType() instanceof DerivedType
then valueArgNode.asExpr() = this.(Call).getArgument(3)
else
// All other cases
valueArgNode.asExpr() = this.(Call).getArgument(2)
)
)
}

override Crypto::AlgorithmInstance getAKnownAlgorithmSource() {
exists(OpenSSLAlgorithmInstance i | i.getAVC() = this and result = i)
}

override DataFlow::Node getResultNode() { result = resultNode }

override Crypto::ConsumerInputDataFlowNode getInputNode() { result = valueArgNode }
}