From 6d1b1d1a6e09fd53712728a7a49805b0850262ac Mon Sep 17 00:00:00 2001 From: GrosQuildu Date: Fri, 23 May 2025 18:09:39 +0200 Subject: [PATCH 1/7] refactor EVP common classes add initial work for openssl signatures add basic C test files for ciphers and signatures more signature classes, comments for evp base classes more signature tests fix super calls for input consumers fix getOutputArtifact for tests formatting delete redundant test files move algorithm methods to OpenSSLOperation refactor ECKeyGenOperation for new EVP classes formatting fix getOutputArtifact fix cipher and digest operation test results mv openssl signature to another PR --- .../OpenSSL/Operations/ECKeyGenOperation.qll | 31 +--- .../Operations/EVPCipherInitializer.qll | 14 +- .../OpenSSL/Operations/EVPCipherOperation.qll | 89 ++-------- .../OpenSSL/Operations/EVPHashInitializer.qll | 7 +- .../OpenSSL/Operations/EVPHashOperation.qll | 100 ++++-------- .../Operations/OpenSSLOperationBase.qll | 152 +++++++++++++++++- .../OpenSSL/Operations/OpenSSLOperations.qll | 1 + .../openssl/cipher_operations.expected | 16 +- .../quantum/openssl/hash_operations.expected | 4 +- 9 files changed, 209 insertions(+), 205 deletions(-) diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/ECKeyGenOperation.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/ECKeyGenOperation.qll index 4f07ecc0f9e3..40103569cac0 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/ECKeyGenOperation.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/ECKeyGenOperation.qll @@ -4,42 +4,15 @@ private import OpenSSLOperationBase private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers private import semmle.code.cpp.dataflow.new.DataFlow -private module AlgGetterToAlgConsumerConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { - exists(OpenSSLAlgorithmValueConsumer c | c.getResultNode() = source) - } - - predicate isSink(DataFlow::Node sink) { - exists(ECKeyGenOperation c | c.getAlgorithmArg() = sink.asExpr()) - } -} - -private module AlgGetterToAlgConsumerFlow = DataFlow::Global; - class ECKeyGenOperation extends OpenSSLOperation, Crypto::KeyGenerationOperationInstance { ECKeyGenOperation() { this.(Call).getTarget().getName() = "EC_KEY_generate_key" } - override Expr getOutputArg() { - result = this.(Call) // return value of call - } - - Expr getAlgorithmArg() { result = this.(Call).getArgument(0) } - - override Expr getInputArg() { - // there is no 'input', in the sense that no data is being manipulated by the operation. - // There is an input of an algorithm, but that is not the intention of the operation input arg. - none() - } + override Expr getAlgorithmArg() { result = this.(Call).getArgument(0) } override Crypto::KeyArtifactType getOutputKeyType() { result = Crypto::TAsymmetricKeyType() } override Crypto::ArtifactOutputDataFlowNode getOutputKeyArtifact() { - result = this.getOutputNode() - } - - override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() { - AlgGetterToAlgConsumerFlow::flow(result.(OpenSSLAlgorithmValueConsumer).getResultNode(), - DataFlow::exprNode(this.getAlgorithmArg())) + result.asExpr() = this.(Call).getArgument(0) } override Crypto::ConsumerInputDataFlowNode getKeySizeConsumer() { diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherInitializer.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherInitializer.qll index 353a89645ec0..e6e9954a3332 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherInitializer.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherInitializer.qll @@ -5,6 +5,7 @@ private import experimental.quantum.Language private import experimental.quantum.OpenSSL.CtxFlow as CTXFlow +private import OpenSSLOperationBase module EncValToInitEncArgConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source.asExpr().getValue().toInt() in [0, 1] } @@ -34,19 +35,12 @@ Crypto::KeyOperationSubtype intToCipherOperationSubtype(int i) { } // TODO: need to add key consumer -abstract class EVP_Cipher_Initializer extends Call { - Expr getContextArg() { result = this.(Call).getArgument(0) } +abstract class EVP_Cipher_Initializer extends EVPInitialize { + override Expr getAlgorithmArg() { result = this.(Call).getArgument(1) } - Expr getAlgorithmArg() { result = this.(Call).getArgument(1) } - - abstract Expr getKeyArg(); - - abstract Expr getIVArg(); - - // abstract Crypto::CipherOperationSubtype getCipherOperationSubtype(); abstract Expr getOperationSubtypeArg(); - Crypto::KeyOperationSubtype getCipherOperationSubtype() { + override Crypto::KeyOperationSubtype getKeyOperationSubtype() { if this.(Call).getTarget().getName().toLowerCase().matches("%encrypt%") then result instanceof Crypto::TEncryptMode else diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll index 233bfd504338..fb06543ee5bb 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll @@ -4,36 +4,21 @@ private import EVPCipherInitializer private import OpenSSLOperationBase private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers -private module AlgGetterToAlgConsumerConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { - exists(OpenSSLAlgorithmValueConsumer c | c.getResultNode() = source) +class EVP_Cipher_Update_Call extends EVPUpdate { + EVP_Cipher_Update_Call() { + this.(Call).getTarget().getName() in [ + "EVP_EncryptUpdate", "EVP_DecryptUpdate", "EVP_CipherUpdate" + ] } - predicate isSink(DataFlow::Node sink) { - exists(EVP_Cipher_Operation c | c.getAlgorithmArg() = sink.asExpr()) - } + override Expr getInputArg() { result = this.(Call).getArgument(3) } } -private module AlgGetterToAlgConsumerFlow = DataFlow::Global; - -// import experimental.quantum.OpenSSL.AlgorithmValueConsumers.AlgorithmValueConsumers -// import OpenSSLOperation -// class EVPCipherOutput extends CipherOutputArtifact { -// EVPCipherOutput() { exists(EVP_Cipher_Operation op | op.getOutputArg() = this) } -// override DataFlow::Node getOutputNode() { result.asDefiningArgument() = this } -// } -// /** * see: https://docs.openssl.org/master/man3/EVP_EncryptInit/#synopsis * Base configuration for all EVP cipher operations. - * NOTE: cannot extend instance of OpenSSLOperation, as we need to override - * elements of OpenSSLOperation (i.e., we are creating an instance) */ -abstract class EVP_Cipher_Operation extends OpenSSLOperation, Crypto::KeyOperationInstance { - Expr getContextArg() { result = this.(Call).getArgument(0) } - - Expr getAlgorithmArg() { this.getInitCall().getAlgorithmArg() = result } - +abstract class EVP_Cipher_Operation extends EVPOperation, Crypto::KeyOperationInstance { override Expr getOutputArg() { result = this.(Call).getArgument(1) } override Crypto::KeyOperationSubtype getKeyOperationSubtype() { @@ -43,81 +28,39 @@ abstract class EVP_Cipher_Operation extends OpenSSLOperation, Crypto::KeyOperati result instanceof Crypto::TDecryptMode and this.(Call).getTarget().getName().toLowerCase().matches("%decrypt%") or - result = this.getInitCall().getCipherOperationSubtype() and + result = this.getInitCall().getKeyOperationSubtype() and this.(Call).getTarget().getName().toLowerCase().matches("%cipher%") } - EVP_Cipher_Initializer getInitCall() { - CTXFlow::ctxArgFlowsToCtxArg(result.getContextArg(), this.getContextArg()) - } - override Crypto::ConsumerInputDataFlowNode getNonceConsumer() { this.getInitCall().getIVArg() = result.asExpr() } - override Crypto::ConsumerInputDataFlowNode getInputConsumer() { result = this.getInputNode() } - override Crypto::ConsumerInputDataFlowNode getKeyConsumer() { this.getInitCall().getKeyArg() = result.asExpr() + // todo: or track to the EVP_PKEY_CTX_new } - override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { result = this.getOutputNode() } + override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { + result = EVPOperation.super.getOutputArtifact() + } - override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() { - AlgGetterToAlgConsumerFlow::flow(result.(OpenSSLAlgorithmValueConsumer).getResultNode(), - DataFlow::exprNode(this.getInitCall().getAlgorithmArg())) + override Crypto::ConsumerInputDataFlowNode getInputConsumer() { + result = EVPOperation.super.getInputConsumer() } } -class EVP_Cipher_Call extends EVP_Cipher_Operation { +class EVP_Cipher_Call extends EVPOneShot, EVP_Cipher_Operation { EVP_Cipher_Call() { this.(Call).getTarget().getName() = "EVP_Cipher" } override Expr getInputArg() { result = this.(Call).getArgument(2) } } -// NOTE: not modeled as cipher operations, these are intermediate calls -class EVP_Cipher_Update_Call extends Call { - EVP_Cipher_Update_Call() { - this.(Call).getTarget().getName() in [ - "EVP_EncryptUpdate", "EVP_DecryptUpdate", "EVP_CipherUpdate" - ] - } - - Expr getInputArg() { result = this.(Call).getArgument(3) } - - DataFlow::Node getInputNode() { result.asExpr() = this.getInputArg() } - - Expr getContextArg() { result = this.(Call).getArgument(0) } -} - -class EVP_Cipher_Final_Call extends EVP_Cipher_Operation { +class EVP_Cipher_Final_Call extends EVPFinal, EVP_Cipher_Operation { EVP_Cipher_Final_Call() { this.(Call).getTarget().getName() in [ "EVP_EncryptFinal_ex", "EVP_DecryptFinal_ex", "EVP_CipherFinal_ex", "EVP_EncryptFinal", "EVP_DecryptFinal", "EVP_CipherFinal" ] } - - EVP_Cipher_Update_Call getUpdateCalls() { - CTXFlow::ctxArgFlowsToCtxArg(result.getContextArg(), this.getContextArg()) - } - - override Expr getInputArg() { result = this.getUpdateCalls().getInputArg() } - - override Crypto::ConsumerInputDataFlowNode getInputConsumer() { result = this.getInputNode() } -} - -class EVP_PKEY_Operation extends EVP_Cipher_Operation { - EVP_PKEY_Operation() { - this.(Call).getTarget().getName() in ["EVP_PKEY_decrypt", "EVP_PKEY_encrypt"] - } - - override Expr getInputArg() { result = this.(Call).getArgument(3) } - // TODO: how PKEY is initialized is different that symmetric cipher - // Consider making an entirely new class for this and specializing - // the get init call -} - -class EVPCipherInputArgument extends Expr { - EVPCipherInputArgument() { exists(EVP_Cipher_Operation op | op.getInputArg() = this) } } diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashInitializer.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashInitializer.qll index 46d414ece6ce..7309242f198b 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashInitializer.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashInitializer.qll @@ -1,10 +1,7 @@ import cpp +private import OpenSSLOperationBase -abstract class EVP_Hash_Initializer extends Call { - Expr getContextArg() { result = this.(Call).getArgument(0) } - - abstract Expr getAlgorithmArg(); -} +abstract class EVP_Hash_Initializer extends EVPInitialize { } class EVP_DigestInit_Variant_Calls extends EVP_Hash_Initializer { EVP_DigestInit_Variant_Calls() { diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll index c68ccd960845..2b798a96b7e2 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll @@ -8,118 +8,78 @@ private import OpenSSLOperationBase private import EVPHashInitializer private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers -// import EVPHashConsumers -abstract class EVP_Hash_Operation extends OpenSSLOperation, Crypto::HashOperationInstance { - Expr getContextArg() { result = this.(Call).getArgument(0) } - - Expr getAlgorithmArg() { result = this.getInitCall().getAlgorithmArg() } - - EVP_Hash_Initializer getInitCall() { - CTXFlow::ctxArgFlowsToCtxArg(result.getContextArg(), this.getContextArg()) - } - - /** - * By default, the algorithm value comes from the init call. - * There are variants where this isn't true, in which case the - * subclass should override this method. - */ - override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() { - AlgGetterToAlgConsumerFlow::flow(result.(OpenSSLAlgorithmValueConsumer).getResultNode(), - DataFlow::exprNode(this.getAlgorithmArg())) - } -} - -private module AlgGetterToAlgConsumerConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { - exists(OpenSSLAlgorithmValueConsumer c | c.getResultNode() = source) - } +class EVP_Digest_Update_Call extends EVPUpdate { + EVP_Digest_Update_Call() { this.(Call).getTarget().getName() in ["EVP_DigestUpdate"] } - predicate isSink(DataFlow::Node sink) { - exists(EVP_Hash_Operation c | c.getAlgorithmArg() = sink.asExpr()) - } + override Expr getInputArg() { result = this.(Call).getArgument(1) } } -private module AlgGetterToAlgConsumerFlow = DataFlow::Global; - //https://docs.openssl.org/3.0/man3/EVP_DigestInit/#synopsis -class EVP_Q_Digest_Operation extends EVP_Hash_Operation { +class EVP_Q_Digest_Operation extends EVPOneShot, Crypto::HashOperationInstance { EVP_Q_Digest_Operation() { this.(Call).getTarget().getName() = "EVP_Q_digest" } - //override Crypto::AlgorithmConsumer getAlgorithmConsumer() { } + override Expr getAlgorithmArg() { result = this.(Call).getArgument(1) } + override EVP_Hash_Initializer getInitCall() { // This variant of digest does not use an init // and even if it were used, the init would be ignored/undefined none() } - override Expr getOutputArg() { result = this.(Call).getArgument(5) } - override Expr getInputArg() { result = this.(Call).getArgument(3) } - override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { result = this.getOutputNode() } - - override Crypto::ConsumerInputDataFlowNode getInputConsumer() { result = this.getInputNode() } + override Expr getOutputArg() { result = this.(Call).getArgument(5) } - override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() { - // The operation is a direct algorithm consumer - // NOTE: the operation itself is already modeld as a value consumer, so we can - // simply return 'this', see modeled hash algorithm consuers for EVP_Q_Digest - this = result + override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { + result = EVPOneShot.super.getOutputArtifact() } - override Expr getAlgorithmArg() { result = this.(Call).getArgument(1) } + override Crypto::ConsumerInputDataFlowNode getInputConsumer() { + result = EVPOneShot.super.getInputConsumer() + } } -class EVP_Digest_Operation extends EVP_Hash_Operation { +class EVP_Digest_Operation extends EVPOneShot, Crypto::HashOperationInstance { EVP_Digest_Operation() { this.(Call).getTarget().getName() = "EVP_Digest" } // There is no context argument for this function override Expr getContextArg() { none() } + override Expr getAlgorithmArg() { result = this.(Call).getArgument(4) } + override EVP_Hash_Initializer getInitCall() { // This variant of digest does not use an init // and even if it were used, the init would be ignored/undefined none() } - override Expr getAlgorithmArg() { result = this.(Call).getArgument(4) } - - override Expr getOutputArg() { result = this.(Call).getArgument(2) } - override Expr getInputArg() { result = this.(Call).getArgument(0) } - override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { result = this.getOutputNode() } - - override Crypto::ConsumerInputDataFlowNode getInputConsumer() { result = this.getInputNode() } -} - -// NOTE: not modeled as hash operations, these are intermediate calls -class EVP_Digest_Update_Call extends Call { - EVP_Digest_Update_Call() { this.(Call).getTarget().getName() in ["EVP_DigestUpdate"] } - - Expr getInputArg() { result = this.(Call).getArgument(1) } + override Expr getOutputArg() { result = this.(Call).getArgument(2) } - DataFlow::Node getInputNode() { result.asExpr() = this.getInputArg() } + override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { + result = EVPOneShot.super.getOutputArtifact() + } - Expr getContextArg() { result = this.(Call).getArgument(0) } + override Crypto::ConsumerInputDataFlowNode getInputConsumer() { + result = EVPOneShot.super.getInputConsumer() + } } -class EVP_Digest_Final_Call extends EVP_Hash_Operation { +class EVP_Digest_Final_Call extends EVPFinal, Crypto::HashOperationInstance { EVP_Digest_Final_Call() { this.(Call).getTarget().getName() in [ "EVP_DigestFinal", "EVP_DigestFinal_ex", "EVP_DigestFinalXOF" ] } - EVP_Digest_Update_Call getUpdateCalls() { - CTXFlow::ctxArgFlowsToCtxArg(result.getContextArg(), this.getContextArg()) - } - - override Expr getInputArg() { result = this.getUpdateCalls().getInputArg() } - - override Crypto::ConsumerInputDataFlowNode getInputConsumer() { result = this.getInputNode() } - override Expr getOutputArg() { result = this.(Call).getArgument(1) } - override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { result = this.getOutputNode() } + override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { + result = EVPFinal.super.getOutputArtifact() + } + + override Crypto::ConsumerInputDataFlowNode getInputConsumer() { + result = EVPFinal.super.getInputConsumer() + } } diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll index f9753e92c5d2..9cccd6395a1c 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll @@ -1,21 +1,157 @@ private import experimental.quantum.Language +private import experimental.quantum.OpenSSL.CtxFlow as CTXFlow +private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers +/** + * All OpenSSL operations. + */ abstract class OpenSSLOperation extends Crypto::OperationInstance instanceof Call { + /** + * Expression that specifies the algorithm for the operation. + * Will be an argument of the operation in the simplest case. + */ + abstract Expr getAlgorithmArg(); + + /** + * Algorithm is specified in initialization call or is implicitly established by the key. + */ + override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() { + AlgGetterToAlgConsumerFlow::flow(result.(OpenSSLAlgorithmValueConsumer).getResultNode(), + DataFlow::exprNode(this.getAlgorithmArg())) + } +} + +/** + * Calls to initialization functions of EVP API. + * These are not operations in the sense of Crypto::OperationInstance, + * but they are used to initialize the context for the operation. + */ +abstract class EVPInitialize extends Call { + /** + * The context argument that ties together initialization, updates and/or final calls. + */ + Expr getContextArg() { result = this.(Call).getArgument(0) } + + /** + * The type of key operation, none if not applicable. + */ + Crypto::KeyOperationSubtype getKeyOperationSubtype() { none() } + + /** + * Explicitly specified algorithm or none if implicit (e.g., established by the key). + * None if not applicable. + */ + Expr getAlgorithmArg() { none() } + + /** + * The key for the operation, none if not applicable. + */ + Expr getKeyArg() { none() } + + /** + * The IV/nonce, none if not applicable. + */ + Expr getIVArg() { none() } +} + +/** + * Calls to update functions of EVP API. + * These are not operations in the sense of Crypto::OperationInstance, + * but they are used to update the context for the operation. + */ +abstract class EVPUpdate extends Call { + /** + * The context argument that ties together initialization, updates and/or final calls. + */ + Expr getContextArg() { result = this.(Call).getArgument(0) } + + /** + * Update calls always have some input data like plaintext or message digest. + */ abstract Expr getInputArg(); +} + +/** + * Flows from algorithm values to operations, specific to OpenSSL + */ +private module AlgGetterToAlgConsumerConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + exists(OpenSSLAlgorithmValueConsumer c | c.getResultNode() = source) + } + + predicate isSink(DataFlow::Node sink) { + exists(EVPOperation c | c.getAlgorithmArg() = sink.asExpr()) + } +} + +private module AlgGetterToAlgConsumerFlow = DataFlow::Global; + +/** + * Base class for all operations of the EVP API. + * Currently final calls and one-shot calls are implemented. + * Provides some default methods for Crypto::KeyOperationInstance class + */ +abstract class EVPOperation extends OpenSSLOperation { + /** + * The context argument that ties together initialization, updates and/or final calls. + */ + Expr getContextArg() { result = this.(Call).getArgument(0) } /** - * Can be an argument of a call or a return value of a function. + * Some input data like plaintext or message digest. + * Either argument provided direcly in the call or all arguments that were provided in update calls. + */ + abstract Expr getInputArg(); + + /** + * Some output data like ciphertext or signature. + * Always produced directly by this operation. + * Assumption: output is provided as an argument to the call, never as return value. */ abstract Expr getOutputArg(); - DataFlow::Node getInputNode() { - // Assumed to be default to asExpr - result.asExpr() = this.getInputArg() + /** + * Overwrite with an explicitly specified algorithm or leave base implementation to find it in the initialization call. + */ + override Expr getAlgorithmArg() { + if exists(this.getInitCall()) then result = this.getInitCall().getAlgorithmArg() else none() + } + + /** + * Finds the initialization call, may be none. + */ + EVPInitialize getInitCall() { + CTXFlow::ctxArgFlowsToCtxArg(result.getContextArg(), this.getContextArg()) } - DataFlow::Node getOutputNode() { - if exists(Call c | c.getAnArgument() = this) - then result.asDefiningArgument() = this - else result.asExpr() = this + Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { + result.asExpr() = this.getOutputArg() } + + /** + * Input consumer is the input argument of the call. + */ + Crypto::ConsumerInputDataFlowNode getInputConsumer() { result.asExpr() = this.getInputArg() } } + +/** + * Final calls of EVP API. + */ +abstract class EVPFinal extends EVPOperation { + /** + * All update calls that were executed before this final call. + */ + EVPUpdate getUpdateCalls() { + CTXFlow::ctxArgFlowsToCtxArg(result.getContextArg(), this.getContextArg()) + } + + /** + * The input data was provided to all update calls. + */ + override Expr getInputArg() { result = this.getUpdateCalls().getInputArg() } +} + +/** + * One-shot calls of EVP API. + */ +abstract class EVPOneShot extends EVPOperation { } diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll index f6ff0dd1f077..54ac977ead0d 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll @@ -2,3 +2,4 @@ import OpenSSLOperationBase import EVPCipherOperation import EVPHashOperation import ECKeyGenOperation +import EVPSignatureOperation diff --git a/cpp/ql/test/experimental/library-tests/quantum/openssl/cipher_operations.expected b/cpp/ql/test/experimental/library-tests/quantum/openssl/cipher_operations.expected index 074f86fd4494..d566397eacad 100644 --- a/cpp/ql/test/experimental/library-tests/quantum/openssl/cipher_operations.expected +++ b/cpp/ql/test/experimental/library-tests/quantum/openssl/cipher_operations.expected @@ -1,8 +1,8 @@ -| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:13:40:31 | KeyOperationOutput | openssl_basic.c:23:62:23:65 | Key | openssl_basic.c:23:68:23:71 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | -| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:13:40:31 | KeyOperationOutput | openssl_basic.c:23:62:23:65 | Key | openssl_basic.c:31:54:31:55 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | -| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:13:40:31 | KeyOperationOutput | openssl_basic.c:31:49:31:51 | Key | openssl_basic.c:23:68:23:71 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | -| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:13:40:31 | KeyOperationOutput | openssl_basic.c:31:49:31:51 | Key | openssl_basic.c:31:54:31:55 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | -| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:11:90:29 | KeyOperationOutput | openssl_basic.c:69:58:69:61 | Key | openssl_basic.c:69:64:69:67 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | -| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:11:90:29 | KeyOperationOutput | openssl_basic.c:69:58:69:61 | Key | openssl_basic.c:77:50:77:51 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | -| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:11:90:29 | KeyOperationOutput | openssl_basic.c:77:45:77:47 | Key | openssl_basic.c:69:64:69:67 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | -| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:11:90:29 | KeyOperationOutput | openssl_basic.c:77:45:77:47 | Key | openssl_basic.c:77:50:77:51 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | +| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:38:40:53 | KeyOperationOutput | openssl_basic.c:23:62:23:65 | Key | openssl_basic.c:23:68:23:71 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | +| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:38:40:53 | KeyOperationOutput | openssl_basic.c:23:62:23:65 | Key | openssl_basic.c:31:54:31:55 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | +| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:38:40:53 | KeyOperationOutput | openssl_basic.c:31:49:31:51 | Key | openssl_basic.c:23:68:23:71 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | +| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:38:40:53 | KeyOperationOutput | openssl_basic.c:31:49:31:51 | Key | openssl_basic.c:31:54:31:55 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | +| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:36:90:50 | KeyOperationOutput | openssl_basic.c:69:58:69:61 | Key | openssl_basic.c:69:64:69:67 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | +| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:36:90:50 | KeyOperationOutput | openssl_basic.c:69:58:69:61 | Key | openssl_basic.c:77:50:77:51 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | +| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:36:90:50 | KeyOperationOutput | openssl_basic.c:77:45:77:47 | Key | openssl_basic.c:69:64:69:67 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | +| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:36:90:50 | KeyOperationOutput | openssl_basic.c:77:45:77:47 | Key | openssl_basic.c:77:50:77:51 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | diff --git a/cpp/ql/test/experimental/library-tests/quantum/openssl/hash_operations.expected b/cpp/ql/test/experimental/library-tests/quantum/openssl/hash_operations.expected index 9e52ea448856..247c4389bc1a 100644 --- a/cpp/ql/test/experimental/library-tests/quantum/openssl/hash_operations.expected +++ b/cpp/ql/test/experimental/library-tests/quantum/openssl/hash_operations.expected @@ -1,2 +1,2 @@ -| openssl_basic.c:124:13:124:30 | HashOperation | openssl_basic.c:124:13:124:30 | Digest | openssl_basic.c:116:38:116:47 | HashAlgorithm | openssl_basic.c:120:37:120:43 | Message | -| openssl_basic.c:144:13:144:22 | HashOperation | openssl_basic.c:144:13:144:22 | Digest | openssl_basic.c:144:67:144:73 | HashAlgorithm | openssl_basic.c:144:24:144:30 | Message | +| openssl_basic.c:124:13:124:30 | HashOperation | openssl_basic.c:124:39:124:44 | Digest | openssl_basic.c:116:38:116:47 | HashAlgorithm | openssl_basic.c:120:37:120:43 | Message | +| openssl_basic.c:144:13:144:22 | HashOperation | openssl_basic.c:144:46:144:51 | Digest | openssl_basic.c:144:67:144:73 | HashAlgorithm | openssl_basic.c:144:24:144:30 | Message | From af8702d6a8a08ef4bb58380e01631ddea1fdc0da Mon Sep 17 00:00:00 2001 From: GrosQuildu Date: Wed, 28 May 2025 18:39:15 +0200 Subject: [PATCH 2/7] fix openssl outputs --- .../OpenSSL/Operations/EVPCipherOperation.qll | 11 ++++++++++ .../Operations/OpenSSLOperationBase.qll | 20 +++++++++++++++---- .../OpenSSL/Operations/OpenSSLOperations.qll | 1 - .../openssl/cipher_operations.expected | 8 ++++++++ 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll index fb06543ee5bb..993b5ad60a92 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll @@ -12,6 +12,8 @@ class EVP_Cipher_Update_Call extends EVPUpdate { } override Expr getInputArg() { result = this.(Call).getArgument(3) } + + override Expr getOutputArg() { result = this.(Call).getArgument(1) } } /** @@ -63,4 +65,13 @@ class EVP_Cipher_Final_Call extends EVPFinal, EVP_Cipher_Operation { "EVP_DecryptFinal", "EVP_CipherFinal" ] } + + /** + * Output is both from update calls and from the final call. + */ + override Expr getOutputArg() { + result = EVPFinal.super.getOutputArg() + or + result = EVP_Cipher_Operation.super.getOutputArg() + } } diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll index 9cccd6395a1c..d7dc2483c84a 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll @@ -69,6 +69,11 @@ abstract class EVPUpdate extends Call { * Update calls always have some input data like plaintext or message digest. */ abstract Expr getInputArg(); + + /** + * Update calls sometimes have some output data like a plaintext. + */ + Expr getOutputArg() { none() } } /** @@ -105,8 +110,6 @@ abstract class EVPOperation extends OpenSSLOperation { /** * Some output data like ciphertext or signature. - * Always produced directly by this operation. - * Assumption: output is provided as an argument to the call, never as return value. */ abstract Expr getOutputArg(); @@ -125,13 +128,15 @@ abstract class EVPOperation extends OpenSSLOperation { } Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { - result.asExpr() = this.getOutputArg() + result = DataFlow::exprNode(this.getOutputArg()) } /** * Input consumer is the input argument of the call. */ - Crypto::ConsumerInputDataFlowNode getInputConsumer() { result.asExpr() = this.getInputArg() } + Crypto::ConsumerInputDataFlowNode getInputConsumer() { + result = DataFlow::exprNode(this.getInputArg()) + } } /** @@ -147,8 +152,15 @@ abstract class EVPFinal extends EVPOperation { /** * The input data was provided to all update calls. + * If more input data was provided in the final call, override the method. */ override Expr getInputArg() { result = this.getUpdateCalls().getInputArg() } + + /** + * The output data was provided to all update calls. + * If more output data was provided in the final call, override the method. + */ + override Expr getOutputArg() { result = this.getUpdateCalls().getOutputArg() } } /** diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll index 54ac977ead0d..f6ff0dd1f077 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll @@ -2,4 +2,3 @@ import OpenSSLOperationBase import EVPCipherOperation import EVPHashOperation import ECKeyGenOperation -import EVPSignatureOperation diff --git a/cpp/ql/test/experimental/library-tests/quantum/openssl/cipher_operations.expected b/cpp/ql/test/experimental/library-tests/quantum/openssl/cipher_operations.expected index d566397eacad..73b0af3ad5f4 100644 --- a/cpp/ql/test/experimental/library-tests/quantum/openssl/cipher_operations.expected +++ b/cpp/ql/test/experimental/library-tests/quantum/openssl/cipher_operations.expected @@ -1,7 +1,15 @@ +| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:35:36:35:45 | KeyOperationOutput | openssl_basic.c:23:62:23:65 | Key | openssl_basic.c:23:68:23:71 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | +| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:35:36:35:45 | KeyOperationOutput | openssl_basic.c:23:62:23:65 | Key | openssl_basic.c:31:54:31:55 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | +| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:35:36:35:45 | KeyOperationOutput | openssl_basic.c:31:49:31:51 | Key | openssl_basic.c:23:68:23:71 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | +| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:35:36:35:45 | KeyOperationOutput | openssl_basic.c:31:49:31:51 | Key | openssl_basic.c:31:54:31:55 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | | openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:38:40:53 | KeyOperationOutput | openssl_basic.c:23:62:23:65 | Key | openssl_basic.c:23:68:23:71 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | | openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:38:40:53 | KeyOperationOutput | openssl_basic.c:23:62:23:65 | Key | openssl_basic.c:31:54:31:55 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | | openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:38:40:53 | KeyOperationOutput | openssl_basic.c:31:49:31:51 | Key | openssl_basic.c:23:68:23:71 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | | openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:38:40:53 | KeyOperationOutput | openssl_basic.c:31:49:31:51 | Key | openssl_basic.c:31:54:31:55 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt | +| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:81:32:81:40 | KeyOperationOutput | openssl_basic.c:69:58:69:61 | Key | openssl_basic.c:69:64:69:67 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | +| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:81:32:81:40 | KeyOperationOutput | openssl_basic.c:69:58:69:61 | Key | openssl_basic.c:77:50:77:51 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | +| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:81:32:81:40 | KeyOperationOutput | openssl_basic.c:77:45:77:47 | Key | openssl_basic.c:69:64:69:67 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | +| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:81:32:81:40 | KeyOperationOutput | openssl_basic.c:77:45:77:47 | Key | openssl_basic.c:77:50:77:51 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | | openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:36:90:50 | KeyOperationOutput | openssl_basic.c:69:58:69:61 | Key | openssl_basic.c:69:64:69:67 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | | openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:36:90:50 | KeyOperationOutput | openssl_basic.c:69:58:69:61 | Key | openssl_basic.c:77:50:77:51 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | | openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:36:90:50 | KeyOperationOutput | openssl_basic.c:77:45:77:47 | Key | openssl_basic.c:69:64:69:67 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt | From f103e8be96a261ff6fbdb0b118096c77d59c94e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20P=C5=82atek?= Date: Thu, 29 May 2025 11:12:31 +0200 Subject: [PATCH 3/7] Update cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll Co-authored-by: Ben Rodes --- .../quantum/OpenSSL/Operations/OpenSSLOperationBase.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll index d7dc2483c84a..e97181c744f4 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll @@ -22,7 +22,7 @@ abstract class OpenSSLOperation extends Crypto::OperationInstance instanceof Cal } /** - * Calls to initialization functions of EVP API. + * A Call to initialization functions from the EVP API. * These are not operations in the sense of Crypto::OperationInstance, * but they are used to initialize the context for the operation. */ From 328cf798bf330ed58502be13f0ff5f0d6ef14126 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20P=C5=82atek?= Date: Thu, 29 May 2025 11:14:16 +0200 Subject: [PATCH 4/7] Apply docs suggestions Co-authored-by: Ben Rodes --- .../quantum/OpenSSL/Operations/OpenSSLOperationBase.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll index e97181c744f4..dcd2d83cdc5b 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll @@ -55,7 +55,7 @@ abstract class EVPInitialize extends Call { } /** - * Calls to update functions of EVP API. + * A Call to update functions from the EVP API. * These are not operations in the sense of Crypto::OperationInstance, * but they are used to update the context for the operation. */ @@ -92,7 +92,7 @@ private module AlgGetterToAlgConsumerConfig implements DataFlow::ConfigSig { private module AlgGetterToAlgConsumerFlow = DataFlow::Global; /** - * Base class for all operations of the EVP API. + * The base class for all operations of the EVP API. * Currently final calls and one-shot calls are implemented. * Provides some default methods for Crypto::KeyOperationInstance class */ From f04fa58c8b5d8e06e76e13fbef03bc8b4286c83e Mon Sep 17 00:00:00 2001 From: GrosQuildu Date: Thu, 29 May 2025 11:33:52 +0200 Subject: [PATCH 5/7] rm one-shot class --- .../OpenSSL/Operations/EVPCipherOperation.qll | 2 +- .../quantum/OpenSSL/Operations/EVPHashOperation.qll | 12 ++++++------ .../OpenSSL/Operations/OpenSSLOperationBase.qll | 7 +------ 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll index 993b5ad60a92..5f24d840ff88 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll @@ -52,7 +52,7 @@ abstract class EVP_Cipher_Operation extends EVPOperation, Crypto::KeyOperationIn } } -class EVP_Cipher_Call extends EVPOneShot, EVP_Cipher_Operation { +class EVP_Cipher_Call extends EVPOperation, EVP_Cipher_Operation { EVP_Cipher_Call() { this.(Call).getTarget().getName() = "EVP_Cipher" } override Expr getInputArg() { result = this.(Call).getArgument(2) } diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll index 2b798a96b7e2..f2907db56781 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll @@ -15,7 +15,7 @@ class EVP_Digest_Update_Call extends EVPUpdate { } //https://docs.openssl.org/3.0/man3/EVP_DigestInit/#synopsis -class EVP_Q_Digest_Operation extends EVPOneShot, Crypto::HashOperationInstance { +class EVP_Q_Digest_Operation extends EVPOperation, Crypto::HashOperationInstance { EVP_Q_Digest_Operation() { this.(Call).getTarget().getName() = "EVP_Q_digest" } override Expr getAlgorithmArg() { result = this.(Call).getArgument(1) } @@ -31,15 +31,15 @@ class EVP_Q_Digest_Operation extends EVPOneShot, Crypto::HashOperationInstance { override Expr getOutputArg() { result = this.(Call).getArgument(5) } override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { - result = EVPOneShot.super.getOutputArtifact() + result = EVPOperation.super.getOutputArtifact() } override Crypto::ConsumerInputDataFlowNode getInputConsumer() { - result = EVPOneShot.super.getInputConsumer() + result = EVPOperation.super.getInputConsumer() } } -class EVP_Digest_Operation extends EVPOneShot, Crypto::HashOperationInstance { +class EVP_Digest_Operation extends EVPOperation, Crypto::HashOperationInstance { EVP_Digest_Operation() { this.(Call).getTarget().getName() = "EVP_Digest" } // There is no context argument for this function @@ -58,11 +58,11 @@ class EVP_Digest_Operation extends EVPOneShot, Crypto::HashOperationInstance { override Expr getOutputArg() { result = this.(Call).getArgument(2) } override Crypto::ArtifactOutputDataFlowNode getOutputArtifact() { - result = EVPOneShot.super.getOutputArtifact() + result = EVPOperation.super.getOutputArtifact() } override Crypto::ConsumerInputDataFlowNode getInputConsumer() { - result = EVPOneShot.super.getInputConsumer() + result = EVPOperation.super.getInputConsumer() } } diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll index dcd2d83cdc5b..7e9e781c8971 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll @@ -93,7 +93,7 @@ private module AlgGetterToAlgConsumerFlow = DataFlow::Global Date: Tue, 3 Jun 2025 15:22:25 +0200 Subject: [PATCH 6/7] remove redundant if/none --- .../quantum/OpenSSL/Operations/OpenSSLOperationBase.qll | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll index 7e9e781c8971..cbd24f19904e 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll @@ -116,9 +116,7 @@ abstract class EVPOperation extends OpenSSLOperation { /** * Overwrite with an explicitly specified algorithm or leave base implementation to find it in the initialization call. */ - override Expr getAlgorithmArg() { - if exists(this.getInitCall()) then result = this.getInitCall().getAlgorithmArg() else none() - } + override Expr getAlgorithmArg() { result = this.getInitCall().getAlgorithmArg() } /** * Finds the initialization call, may be none. From 60d9b6e338842c95b1fb62346e603d754f6a3254 Mon Sep 17 00:00:00 2001 From: GrosQuildu Date: Tue, 3 Jun 2025 16:19:55 +0200 Subject: [PATCH 7/7] update docs --- .../OpenSSL/Operations/EVPHashOperation.qll | 2 +- .../Operations/OpenSSLOperationBase.qll | 20 +++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll index f2907db56781..796f71398385 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPHashOperation.qll @@ -9,7 +9,7 @@ private import EVPHashInitializer private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers class EVP_Digest_Update_Call extends EVPUpdate { - EVP_Digest_Update_Call() { this.(Call).getTarget().getName() in ["EVP_DigestUpdate"] } + EVP_Digest_Update_Call() { this.(Call).getTarget().getName() = "EVP_DigestUpdate" } override Expr getInputArg() { result = this.(Call).getArgument(1) } } diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll index cbd24f19904e..6ada6cb4665d 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperationBase.qll @@ -3,7 +3,7 @@ private import experimental.quantum.OpenSSL.CtxFlow as CTXFlow private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers /** - * All OpenSSL operations. + * A class for all OpenSSL operations. */ abstract class OpenSSLOperation extends Crypto::OperationInstance instanceof Call { /** @@ -28,12 +28,12 @@ abstract class OpenSSLOperation extends Crypto::OperationInstance instanceof Cal */ abstract class EVPInitialize extends Call { /** - * The context argument that ties together initialization, updates and/or final calls. + * Gets the context argument that ties together initialization, updates and/or final calls. */ Expr getContextArg() { result = this.(Call).getArgument(0) } /** - * The type of key operation, none if not applicable. + * Gets the type of key operation, none if not applicable. */ Crypto::KeyOperationSubtype getKeyOperationSubtype() { none() } @@ -44,12 +44,12 @@ abstract class EVPInitialize extends Call { Expr getAlgorithmArg() { none() } /** - * The key for the operation, none if not applicable. + * Gets the key for the operation, none if not applicable. */ Expr getKeyArg() { none() } /** - * The IV/nonce, none if not applicable. + * Gets the IV/nonce, none if not applicable. */ Expr getIVArg() { none() } } @@ -61,7 +61,7 @@ abstract class EVPInitialize extends Call { */ abstract class EVPUpdate extends Call { /** - * The context argument that ties together initialization, updates and/or final calls. + * Gets the context argument that ties together initialization, updates and/or final calls. */ Expr getContextArg() { result = this.(Call).getArgument(0) } @@ -98,7 +98,7 @@ private module AlgGetterToAlgConsumerFlow = DataFlow::Global