Crypto: Code review cleanup.

This commit is contained in:
REDMOND\brodes
2025-06-16 09:22:11 -04:00
parent 1882db7d86
commit 45fa2c94da
10 changed files with 15 additions and 25 deletions

View File

@@ -22,12 +22,12 @@ predicate knownOpenSSLConstantToKeyAgreementFamilyType(
)
}
class KnownOpenSSLHashConstantAlgorithmInstance extends OpenSSLAlgorithmInstance,
class KnownOpenSSLKeyAgreementConstantAlgorithmInstance extends OpenSSLAlgorithmInstance,
Crypto::KeyAgreementAlgorithmInstance instanceof KnownOpenSSLKeyAgreementAlgorithmExpr
{
OpenSSLAlgorithmValueConsumer getterCall;
KnownOpenSSLHashConstantAlgorithmInstance() {
KnownOpenSSLKeyAgreementConstantAlgorithmInstance() {
// Two possibilities:
// 1) The source is a literal and flows to a getter, then we know we have an instance
// 2) The source is a KnownOpenSSLAlgorithm is call, and we know we have an instance immediately from that
@@ -44,7 +44,6 @@ class KnownOpenSSLHashConstantAlgorithmInstance extends OpenSSLAlgorithmInstance
or
// Possibility 2:
this instanceof OpenSSLAlgorithmCall and
this instanceof DirectAlgorithmValueConsumer and
getterCall = this
}

View File

@@ -19,7 +19,7 @@ class KnownOpenSSLMACConstantAlgorithmInstance extends OpenSSLAlgorithmInstance,
this instanceof OpenSSLAlgorithmLiteral and
exists(DataFlow::Node src, DataFlow::Node sink |
// Sink is an argument to a CipherGetterCall
sink = getterCall.(OpenSSLAlgorithmValueConsumer).getInputNode() and
sink = getterCall.getInputNode() and
// Source is `this`
src.asExpr() = this and
// This traces to a getter

View File

@@ -12,9 +12,6 @@ private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgor
*/
class DirectAlgorithmValueConsumer extends OpenSSLAlgorithmValueConsumer instanceof OpenSSLAlgorithmCall
{
DataFlow::Node resultNode;
Expr resultExpr;
/**
* These cases take in no explicit value (the value is implicit)
*/

View File

@@ -28,7 +28,7 @@ class EVP_Q_Digest_Algorithm_Consumer extends HashAlgorithmValueConsumer {
}
/**
* Instances from https://docs.openssl.org/3.0/man3/EVP_PKEY_CTX_ctrl/
* An instance from https://docs.openssl.org/3.0/man3/EVP_PKEY_CTX_ctrl/
* where the digest is directly consumed by name.
* In these cases, the operation is not yet performed, but there is
* these functions are treated as 'initializers' and track the algorithm through
@@ -71,7 +71,7 @@ class EVPDigestAlgorithmValueConsumer extends HashAlgorithmValueConsumer {
] and
valueArgNode.asExpr() = this.(Call).getArgument(0)
or
this.(Call).getTarget().getName() in ["EVP_MD_fetch"] and
this.(Call).getTarget().getName() = "EVP_MD_fetch" and
valueArgNode.asExpr() = this.(Call).getArgument(1)
or
this.(Call).getTarget().getName() = "EVP_DigestSignInit_ex" and

View File

@@ -1,5 +1,5 @@
import semmle.code.cpp.dataflow.new.DataFlow
import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
/**
* Flows from algorithm values to operations, specific to OpenSSL

View File

@@ -66,10 +66,8 @@ class CtxPointerArgument extends CtxPointerExpr {
/**
* A call returning a CtxPointerExpr.
*/
private class CtxPointerReturn extends CtxPointerExpr {
CtxPointerReturn() { exists(Call c | c = this) }
Call getCall() { result = this.(Call) }
private class CtxPointerReturn extends CtxPointerExpr instanceof Call {
Call getCall() { result = this }
}
/**

View File

@@ -12,9 +12,7 @@ module OpenSSLKeyFlowConfig implements DataFlow::ConfigSig {
exists(Crypto::KeyCreationOperationInstance keygen | keygen.getOutputKeyArtifact() = source)
}
predicate isSink(DataFlow::Node sink) {
exists(Call call | call.(Call).getAnArgument() = sink.asExpr())
}
predicate isSink(DataFlow::Node sink) { exists(Call call | call.getAnArgument() = sink.asExpr()) }
//TODO: consideration for additional flow steps? Can a key be copied for example?
}

View File

@@ -2,7 +2,6 @@ private import experimental.quantum.Language
private import experimental.quantum.OpenSSL.CtxFlow
private import OpenSSLOperationBase
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
private import semmle.code.cpp.dataflow.new.DataFlow
class EVPKeyGenInitialize extends EvpPrimaryAlgorithmInitializer {
EVPKeyGenInitialize() {
@@ -13,7 +12,8 @@ class EVPKeyGenInitialize extends EvpPrimaryAlgorithmInitializer {
}
/**
* The algorithm is encoded through the context argument.
* Gets the algorithm argument.
* In this case the algorithm is encoded through the context argument.
* The context may be directly created from an algorithm consumer,
* or from a new operation off of a prior key. Either way,
* we will treat this argument as the algorithm argument.
@@ -67,8 +67,8 @@ class EVPKeyGenOperation extends EvpOperation, Crypto::KeyGenerationOperationIns
}
/**
* Calls to `EVP_PKEY_new_mac_key` create a new MAC key.
* EVP_PKEY *EVP_PKEY_new_mac_key(int type, ENGINE *e, const unsigned char *key, int keylen);
* A call to `EVP_PKEY_new_mac_key` that creatse a new generic MAC key.
* Signature: EVP_PKEY *EVP_PKEY_new_mac_key(int type, ENGINE *e, const unsigned char *key, int keylen);
*/
class EvpNewMacKey extends EvpOperation, Crypto::KeyGenerationOperationInstance {
DataFlow::Node keyResultNode;

View File

@@ -8,7 +8,6 @@
import cpp
private import experimental.quantum.OpenSSL.CtxFlow
private import OpenSSLOperations
private import OpenSSLOperationBase
/**
* A call to `EVP_PKEY_CTX_new` or `EVP_PKEY_CTX_new_from_pkey`.
@@ -17,7 +16,7 @@ private import OpenSSLOperationBase
* parameters set (e.g., `EVP_PKEY_paramgen`).
* NOTE: for the case of `EVP_PKEY_paramgen`, these calls
* are encoded as context passthroughs, and any operation
* will get all associated initializers for teh paramgen
* will get all associated initializers for the paramgen
* at the final keygen operation automatically.
*/
class EVPNewKeyCtx extends EvpKeyInitializer {

View File

@@ -3,7 +3,6 @@
*/
private import experimental.quantum.Language
private import OpenSSLOperationBase
private import experimental.quantum.OpenSSL.AvcFlow
private import experimental.quantum.OpenSSL.CtxFlow
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
@@ -87,7 +86,7 @@ private Expr signatureOperationOutputArg(Call call) {
}
/**
* Base configuration for all EVP signature operations.
* The base configuration for all EVP signature operations.
*/
abstract class EvpSignatureOperation extends EvpOperation, Crypto::SignatureOperationInstance {
EvpSignatureOperation() {