Skip to content

Crypto: Misc. refactoring and code clean up. #19552

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 3 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
5 changes: 4 additions & 1 deletion cpp/ql/lib/experimental/quantum/Language.qll
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ private class ConstantDataSource extends Crypto::GenericConstantSourceInstance i
// 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"
this.getValue() != "EC" and
// Exclude all 0's as algorithms. Currently we know of no algorithm defined as 0, and
// the typical case is 0 is assigned to represent null.
this.getValue().toInt() != 0
}

override DataFlow::Node getOutputNode() { result.asExpr() = this }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ private import experimental.quantum.Language
private import semmle.code.cpp.dataflow.new.DataFlow
private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
private import PaddingAlgorithmInstance

/**
* Traces 'known algorithms' to AVCs, specifically
Expand All @@ -19,6 +20,9 @@ module KnownOpenSSLAlgorithmToAlgorithmValueConsumerConfig implements DataFlow::
predicate isSink(DataFlow::Node sink) {
exists(OpenSSLAlgorithmValueConsumer c |
c.getInputNode() = sink and
// exclude padding algorithm consumers, since
// these consumers take in different constant values
// not in the typical "known algorithm" set
not c instanceof PaddingAlgorithmValueConsumer
)
}
Expand All @@ -43,9 +47,7 @@ module KnownOpenSSLAlgorithmToAlgorithmValueConsumerFlow =
DataFlow::Global<KnownOpenSSLAlgorithmToAlgorithmValueConsumerConfig>;

module RSAPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source.asExpr() instanceof KnownOpenSSLAlgorithmConstant
}
predicate isSource(DataFlow::Node source) { source.asExpr() instanceof OpenSSLPaddingLiteral }

predicate isSink(DataFlow::Node sink) {
exists(PaddingAlgorithmValueConsumer c | c.getInputNode() = sink)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ private import AlgToAVCFlow

/**
* Given a `KnownOpenSSLBlockModeAlgorithmConstant`, converts this to a block family type.
* Does not bind if there is know mapping (no mapping to 'unknown' or 'other').
* Does not bind if there is no mapping (no mapping to 'unknown' or 'other').
*/
predicate knownOpenSSLConstantToBlockModeFamilyType(
KnownOpenSSLBlockModeAlgorithmConstant e, Crypto::TBlockCipherModeOfOperationType type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ private import BlockAlgorithmInstance

/**
* Given a `KnownOpenSSLCipherAlgorithmConstant`, converts this to a cipher family type.
* Does not bind if there is know mapping (no mapping to 'unknown' or 'other').
* Does not bind if there is no mapping (no mapping to 'unknown' or 'other').
*/
predicate knownOpenSSLConstantToCipherFamilyType(
KnownOpenSSLCipherAlgorithmConstant e, Crypto::KeyOpAlg::TAlgorithm type
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import cpp
private import experimental.quantum.OpenSSL.LibraryDetector

predicate resolveAlgorithmFromExpr(Expr e, string normalizedName, string algType) {
resolveAlgorithmFromCall(e, normalizedName, algType)
Expand All @@ -20,7 +19,7 @@ class KnownOpenSSLCipherAlgorithmConstant extends KnownOpenSSLAlgorithmConstant

KnownOpenSSLCipherAlgorithmConstant() {
resolveAlgorithmFromExpr(this, _, algType) and
algType.toLowerCase().matches("%encryption")
algType.matches("%ENCRYPTION")
}

int getExplicitKeySize() {
Expand All @@ -37,7 +36,7 @@ class KnownOpenSSLPaddingAlgorithmConstant extends KnownOpenSSLAlgorithmConstant

KnownOpenSSLPaddingAlgorithmConstant() {
resolveAlgorithmFromExpr(this, _, algType) and
algType.toLowerCase().matches("%padding")
algType.matches("%PADDING")
}
}

Expand All @@ -46,7 +45,7 @@ class KnownOpenSSLBlockModeAlgorithmConstant extends KnownOpenSSLAlgorithmConsta

KnownOpenSSLBlockModeAlgorithmConstant() {
resolveAlgorithmFromExpr(this, _, algType) and
algType.toLowerCase().matches("%block_mode")
algType.matches("%BLOCK_MODE")
}
}

Expand All @@ -55,7 +54,7 @@ class KnownOpenSSLHashAlgorithmConstant extends KnownOpenSSLAlgorithmConstant {

KnownOpenSSLHashAlgorithmConstant() {
resolveAlgorithmFromExpr(this, _, algType) and
algType.toLowerCase().matches("%hash")
algType.matches("%HASH")
}

int getExplicitDigestLength() {
Expand All @@ -71,7 +70,7 @@ class KnownOpenSSLEllipticCurveAlgorithmConstant extends KnownOpenSSLAlgorithmCo
KnownOpenSSLEllipticCurveAlgorithmConstant() {
exists(string algType |
resolveAlgorithmFromExpr(this, _, algType) and
algType.toLowerCase().matches("elliptic_curve")
algType.matches("ELLIPTIC_CURVE")
)
}
}
Expand All @@ -89,7 +88,6 @@ class KnownOpenSSLEllipticCurveAlgorithmConstant extends KnownOpenSSLAlgorithmCo
* alias = "dss1" and target = "dsaWithSHA1"
*/
predicate resolveAlgorithmFromCall(Call c, string normalized, string algType) {
isPossibleOpenSSLFunction(c.getTarget()) and
exists(string name, string parsedTargetName |
parsedTargetName =
c.getTarget().getName().replaceAll("EVP_", "").toLowerCase().replaceAll("_", "-") and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,26 @@
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase

/**
* A class to define padding specific integer values.
* from rsa.h in openssl:
* # define RSA_PKCS1_PADDING 1
* # define RSA_NO_PADDING 3
* # define RSA_PKCS1_OAEP_PADDING 4
* # define RSA_X931_PADDING 5
* # define RSA_PKCS1_PSS_PADDING 6
* # define RSA_PKCS1_WITH_TLS_PADDING 7
* # define RSA_PKCS1_NO_IMPLICIT_REJECT_PADDING 8
*/
class OpenSSLPaddingLiteral extends Literal {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in OpenSSLPaddingLiteral should be PascalCase/camelCase.
// TODO: we can be more specific about where the literal is in a larger expression
// to avoid literals that are clealy not representing an algorithm, e.g., array indices.
OpenSSLPaddingLiteral() { this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8] }
}

/**
* Given a `KnownOpenSSLPaddingAlgorithmConstant`, converts this to a padding family type.
* Does not bind if there is know mapping (no mapping to 'unknown' or 'other').
* Does not bind if there is no mapping (no mapping to 'unknown' or 'other').
*/
predicate knownOpenSSLConstantToPaddingFamilyType(
KnownOpenSSLPaddingAlgorithmConstant e, Crypto::TPaddingType type
Expand Down Expand Up @@ -60,19 +77,8 @@
this instanceof KnownOpenSSLPaddingAlgorithmConstant and
isPaddingSpecificConsumer = false
or
// Possibility 3:
// from rsa.h in openssl:
// # define RSA_PKCS1_PADDING 1
// # define RSA_NO_PADDING 3
// # define RSA_PKCS1_OAEP_PADDING 4
// # define RSA_X931_PADDING 5
// /* EVP_PKEY_ only */
// # define RSA_PKCS1_PSS_PADDING 6
// # define RSA_PKCS1_WITH_TLS_PADDING 7
// /* internal RSA_ only */
// # define RSA_PKCS1_NO_IMPLICIT_REJECT_PADDING 8
this instanceof Literal and
this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8] and
// Possibility 3: padding-specific literal
this instanceof OpenSSLPaddingLiteral and
exists(DataFlow::Node src, DataFlow::Node sink |
// Sink is an argument to a CipherGetterCall
sink = getterCall.(OpenSSLAlgorithmValueConsumer).getInputNode() and
Expand All @@ -88,24 +94,24 @@

override OpenSSLAlgorithmValueConsumer getAVC() { result = getterCall }

Crypto::TPaddingType getKnownPaddingType() {
this.(Literal).getValue().toInt() in [1, 7, 8] and result = Crypto::PKCS1_v1_5()
or
this.(Literal).getValue().toInt() = 3 and result = Crypto::NoPadding()
or
this.(Literal).getValue().toInt() = 4 and result = Crypto::OAEP()
or
this.(Literal).getValue().toInt() = 5 and result = Crypto::ANSI_X9_23()
or
this.(Literal).getValue().toInt() = 6 and result = Crypto::PSS()
}

override Crypto::TPaddingType getPaddingType() {
isPaddingSpecificConsumer = true and
(
if this.(Literal).getValue().toInt() in [1, 7, 8]
then result = Crypto::PKCS1_v1_5()
else
if this.(Literal).getValue().toInt() = 3
then result = Crypto::NoPadding()
else
if this.(Literal).getValue().toInt() = 4
then result = Crypto::OAEP()
else
if this.(Literal).getValue().toInt() = 5
then result = Crypto::ANSI_X9_23()
else
if this.(Literal).getValue().toInt() = 6
then result = Crypto::PSS()
else result = Crypto::OtherPadding()
result = this.getKnownPaddingType()
or
not exists(this.getKnownPaddingType()) and result = Crypto::OtherPadding()
)
or
isPaddingSpecificConsumer = false and
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
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.AlgorithmInstances.OpenSSLAlgorithmInstanceBase
private import OpenSSLAlgorithmValueConsumerBase
Expand All @@ -14,7 +13,6 @@ class EVPCipherAlgorithmValueConsumer extends CipherAlgorithmValueConsumer {

EVPCipherAlgorithmValueConsumer() {
resultNode.asExpr() = this and
isPossibleOpenSSLFunction(this.(Call).getTarget()) and
(
this.(Call).getTarget().getName() in [
"EVP_get_cipherbyname", "EVP_get_cipherbyobj", "EVP_get_cipherbynid"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
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
Expand All @@ -14,7 +13,6 @@ class EVPEllipticCurveAlgorithmConsumer extends EllipticCurveValueConsumer {

EVPEllipticCurveAlgorithmConsumer() {
resultNode.asExpr() = this.(Call) and // in all cases the result is the return
isPossibleOpenSSLFunction(this.(Call).getTarget()) and
(
this.(Call).getTarget().getName() in ["EVP_EC_gen", "EC_KEY_new_by_curve_name"] and
valueArgNode.asExpr() = this.(Call).getArgument(0)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
private import experimental.quantum.Language
private import semmle.code.cpp.dataflow.new.DataFlow

abstract class OpenSSLAlgorithmValueConsumer extends Crypto::AlgorithmValueConsumer instanceof Call {
/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
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
Expand All @@ -13,7 +12,6 @@ class EVPPKeyAlgorithmConsumer extends PKeyValueConsumer {

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.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
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.AlgorithmInstances.OpenSSLAlgorithmInstanceBase
private import OpenSSLAlgorithmValueConsumerBase
Expand All @@ -16,11 +15,8 @@ class EVP_PKEY_CTX_set_rsa_padding_AlgorithmValueConsumer extends PaddingAlgorit

EVP_PKEY_CTX_set_rsa_padding_AlgorithmValueConsumer() {
resultNode.asExpr() = this and
isPossibleOpenSSLFunction(this.(Call).getTarget()) and
(
this.(Call).getTarget().getName() in ["EVP_PKEY_CTX_set_rsa_padding"] and
valueArgNode.asExpr() = this.(Call).getArgument(1)
)
this.(Call).getTarget().getName() = "EVP_PKEY_CTX_set_rsa_padding" and
valueArgNode.asExpr() = this.(Call).getArgument(1)
}

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