Skip to content

Quantum: Expand OpenSSL cipher modeling and fix JCA false reporting of intermediate calls #19509

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 16, 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
Original file line number Diff line number Diff line change
Expand Up @@ -67,37 +67,42 @@ abstract class EVP_Cipher_Operation extends OpenSSLOperation, Crypto::KeyOperati
}
}

// abstract class EVP_Update_Call extends EVP_Cipher_Operation { }
abstract class EVP_Final_Call extends EVP_Cipher_Operation {
override Expr getInputArg() { none() }
}

// TODO: only model Final (model final as operation and model update but not as an operation)
// Updates are multiple input consumers (most important)
// TODO: assuming update doesn't ouput, otherwise it outputs artifacts, but is not an operation
class EVP_Cipher_Call extends EVP_Cipher_Operation {
EVP_Cipher_Call() { this.(Call).getTarget().getName() = "EVP_Cipher" }

override Expr getInputArg() { result = this.(Call).getArgument(2) }
}

// ******* TODO: model UPDATE but not as the core operation, rather a step towards final
// see the JCA
// class EVP_Encrypt_Decrypt_or_Cipher_Update_Call extends EVP_Update_Call {
// EVP_Encrypt_Decrypt_or_Cipher_Update_Call() {
// this.(Call).getTarget().getName() in [
// "EVP_EncryptUpdate", "EVP_DecryptUpdate", "EVP_CipherUpdate"
// ]
// }
// override Expr getInputArg() { result = this.(Call).getArgument(3) }
// }
class EVP_Encrypt_Decrypt_or_Cipher_Final_Call extends EVP_Final_Call {
EVP_Encrypt_Decrypt_or_Cipher_Final_Call() {
// NOTE: not modeled as cipher operations, these are intermediate calls
class EVP_Update_Call extends Call {
EVP_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_Final_Call extends EVP_Cipher_Operation {
EVP_Final_Call() {
this.(Call).getTarget().getName() in [
"EVP_EncryptFinal_ex", "EVP_DecryptFinal_ex", "EVP_CipherFinal_ex", "EVP_EncryptFinal",
"EVP_DecryptFinal", "EVP_CipherFinal"
]
}

EVP_Update_Call getUpdateCalls() {
CTXFlow::ctxArgFlowsToCtxArg(result.getContextArg(), this.getContextArg())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest adding a flow configuration as in the JCA for properly modelling inputs and distinguishing between intermediate and final calls.

}

override Expr getInputArg() { result = this.getUpdateCalls().getInputArg() }

override Crypto::ConsumerInputDataFlowNode getInputConsumer() { result = this.getInputNode() }
}

class EVP_PKEY_Operation extends EVP_Cipher_Operation {
Expand Down
2 changes: 2 additions & 0 deletions java/ql/lib/experimental/quantum/JCA.qll
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,8 @@ module JCAModel {
}

class CipherOperationInstance extends Crypto::KeyOperationInstance instanceof CipherOperationCall {
CipherOperationInstance() { not this.isIntermediate() }

override Crypto::KeyOperationSubtype getKeyOperationSubtype() {
if CipherFlowAnalysisImpl::hasInit(this)
then result = CipherFlowAnalysisImpl::getInitFromUse(this, _, _).getCipherOperationModeType()
Expand Down