Skip to content

Crypto: Fix QL-for-QL alerts and refactor type standardization #19814

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.

Sign up for GitHub

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
nicolaswill merged 8 commits into from
Jun 26, 2025
Merged

Crypto: Fix QL-for-QL alerts and refactor type standardization #19814

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8ee03e4
Crypto: Fix cpp-specific code scanning alert failure
bdrodes Jun 18, 2025
710e080
Crypto: Refactor casing and documentation
nicolaswill Jun 25, 2025
14472bf
Crypto: Refactor type name mapping and fix QL-for-QL alerts
nicolaswill Jun 25, 2025
b809750
Update cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/Kno…
nicolaswill Jun 25, 2025
8e6031d
Crypto: Fix further acronym casing and remove unused field
nicolaswill Jun 25, 2025
ad7358a
Crypto: Deduplicate "GCM" mapping from OpenSSL modeling
nicolaswill Jun 25, 2025
98479ff
Crypto: Update queries to use new type names
nicolaswill Jun 25, 2025
652e7ba
Merge branch 'main' into codescanning_fixes_cpp
nicolaswill Jun 26, 2025
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
7 changes: 3 additions & 4 deletions cpp/ql/lib/experimental/quantum/Language.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ module ArtifactFlowConfig implements DataFlow::ConfigSig {
module ArtifactFlow = DataFlow::Global<ArtifactFlowConfig>;

/**
* Artifact output to node input configuration
* An artifact output to node input configuration
*/
abstract class AdditionalFlowInputStep extends DataFlow::Node {
abstract DataFlow::Node getOutput();
Expand Down Expand Up @@ -91,9 +91,8 @@ module GenericDataSourceFlowConfig implements DataFlow::ConfigSig {

module GenericDataSourceFlow = TaintTracking::Global<GenericDataSourceFlowConfig>;

private class ConstantDataSource extends Crypto::GenericConstantSourceInstance instanceof Literal {
ConstantDataSource() { this instanceof OpenSslGenericSourceCandidateLiteral }

private class ConstantDataSource extends Crypto::GenericConstantSourceInstance instanceof OpenSslGenericSourceCandidateLiteral
{
override DataFlow::Node getOutputNode() { result.asExpr() = this }

override predicate flowsTo(Crypto::FlowAwareElement other) {
Expand Down
14 changes: 7 additions & 7 deletions cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/AlgToAVCFlow.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ module KnownOpenSslAlgorithmToAlgorithmValueConsumerConfig implements DataFlow::
module KnownOpenSslAlgorithmToAlgorithmValueConsumerFlow =
DataFlow::Global<KnownOpenSslAlgorithmToAlgorithmValueConsumerConfig>;

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

predicate isSink(DataFlow::Node sink) {
Expand All @@ -60,8 +60,8 @@ module RSAPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig implements DataF
}
}

module RSAPaddingAlgorithmToPaddingAlgorithmValueConsumerFlow =
DataFlow::Global<RSAPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig>;
module RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerFlow =
DataFlow::Global<RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig>;

class OpenSslAlgorithmAdditionalFlowStep extends AdditionalFlowInputStep {
OpenSslAlgorithmAdditionalFlowStep() { exists(AlgorithmPassthroughCall c | c.getInNode() = this) }
Expand Down Expand Up @@ -114,11 +114,11 @@ class CopyAndDupAlgorithmPassthroughCall extends AlgorithmPassthroughCall {
override DataFlow::Node getOutNode() { result = outNode }
}

class NIDToPointerPassthroughCall extends AlgorithmPassthroughCall {
class NidToPointerPassthroughCall extends AlgorithmPassthroughCall {
DataFlow::Node inNode;
DataFlow::Node outNode;

NIDToPointerPassthroughCall() {
NidToPointerPassthroughCall() {
this.getTarget().getName() in ["OBJ_nid2obj", "OBJ_nid2ln", "OBJ_nid2sn"] and
inNode.asExpr() = this.getArgument(0) and
outNode.asExpr() = this
Expand Down Expand Up @@ -150,11 +150,11 @@ class PointerToPointerPassthroughCall extends AlgorithmPassthroughCall {
override DataFlow::Node getOutNode() { result = outNode }
}

class PointerToNIDPassthroughCall extends AlgorithmPassthroughCall {
class PointerToNidPassthroughCall extends AlgorithmPassthroughCall {
DataFlow::Node inNode;
DataFlow::Node outNode;

PointerToNIDPassthroughCall() {
PointerToNidPassthroughCall() {
this.getTarget().getName() in ["OBJ_obj2nid", "OBJ_ln2nid", "OBJ_sn2nid", "OBJ_txt2nid"] and
(
inNode.asIndirectExpr() = this.getArgument(0)
Expand Down
20 changes: 10 additions & 10 deletions cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/BlockAlgorithmInstance.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,25 @@ predicate knownOpenSslConstantToBlockModeFamilyType(
exists(string name |
name = e.(KnownOpenSslAlgorithmExpr).getNormalizedName() and
(
name.matches("CBC") and type instanceof Crypto::CBC
name = "CBC" and type instanceof Crypto::CBC
or
name.matches("CFB%") and type instanceof Crypto::CFB
name = "CFB%" and type instanceof Crypto::CFB
or
name.matches("CTR") and type instanceof Crypto::CTR
name = "CTR" and type instanceof Crypto::CTR
or
name.matches("GCM") and type instanceof Crypto::GCM
name = "GCM" and type instanceof Crypto::GCM
or
name.matches("OFB") and type instanceof Crypto::OFB
name = "OFB" and type instanceof Crypto::OFB
or
name.matches("XTS") and type instanceof Crypto::XTS
name = "XTS" and type instanceof Crypto::XTS
or
name.matches("CCM") and type instanceof Crypto::CCM
name = "CCM" and type instanceof Crypto::CCM
or
name.matches("GCM") and type instanceof Crypto::GCM
name = "GCM" and type instanceof Crypto::GCM
or
Copy link

Copilot AI Jun 18, 2025

Choose a reason for hiding this comment

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

This entry for GCM is duplicated later in the OR chain; consider removing the redundant clause to reduce code duplication and simplify maintenance.

Suggested change
name = "GCM" and type instanceof Crypto::GCM
or

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@bdrodes bdrodes Jun 26, 2025

Choose a reason for hiding this comment

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

@nicolaswill was this resolved?

name.matches("CCM") and type instanceof Crypto::CCM
name = "CCM" and type instanceof Crypto::CCM
or
name.matches("ECB") and type instanceof Crypto::ECB
name = "ECB" and type instanceof Crypto::ECB
)
)
}
Expand Down
22 changes: 11 additions & 11 deletions cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/HashAlgorithmInstance.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,35 +11,35 @@ predicate knownOpenSslConstantToHashFamilyType(
exists(string name |
name = e.(KnownOpenSslAlgorithmExpr).getNormalizedName() and
(
name.matches("BLAKE2B") and type instanceof Crypto::BLAKE2B
name = "BLAKE2B" and type instanceof Crypto::BLAKE2B
or
name.matches("BLAKE2S") and type instanceof Crypto::BLAKE2S
name = "BLAKE2S" and type instanceof Crypto::BLAKE2S
or
name.matches("GOST%") and type instanceof Crypto::GOSTHash
or
name.matches("MD2") and type instanceof Crypto::MD2
name = "MD2" and type instanceof Crypto::MD2
or
name.matches("MD4") and type instanceof Crypto::MD4
name = "MD4" and type instanceof Crypto::MD4
or
name.matches("MD5") and type instanceof Crypto::MD5
name = "MD5" and type instanceof Crypto::MD5
or
name.matches("MDC2") and type instanceof Crypto::MDC2
name = "MDC2" and type instanceof Crypto::MDC2
or
name.matches("POLY1305") and type instanceof Crypto::POLY1305
name = "POLY1305" and type instanceof Crypto::POLY1305
or
name.matches(["SHA", "SHA1"]) and type instanceof Crypto::SHA1
or
name.matches("SHA_%") and not name.matches(["SHA1", "SHA3-"]) and type instanceof Crypto::SHA2
or
name.matches("SHA3-%") and type instanceof Crypto::SHA3
or
name.matches(["SHAKE"]) and type instanceof Crypto::SHAKE
name = "SHAKE" and type instanceof Crypto::SHAKE
or
name.matches("SM3") and type instanceof Crypto::SM3
name = "SM3" and type instanceof Crypto::SM3
or
name.matches("RIPEMD160") and type instanceof Crypto::RIPEMD160
name = "RIPEMD160" and type instanceof Crypto::RIPEMD160
or
name.matches("WHIRLPOOL") and type instanceof Crypto::WHIRLPOOL
name = "WHIRLPOOL" and type instanceof Crypto::WHIRLPOOL
)
)
}
Expand Down
5 changes: 3 additions & 2 deletions cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/KnownAlgorithmConstants.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ string getAlgorithmAlias(string alias) {
}

/**
* Finds aliases of known alagorithms defined by users (through obj_name_add and various macros pointing to this function)
* Holds for aliases of known alagorithms defined by users
* (through obj_name_add and various macros pointing to this function).
*
* The `target` and `alias` are converted to lowercase to be of a standard form.
*/
Expand All @@ -222,7 +223,7 @@ predicate customAliases(string target, string alias) {
}

/**
* A hard-coded mapping of known algorithm aliases in OpenSsl.
* Holds for a hard-coded mapping of known algorithm aliases in OpenSsl.
* This was derived by applying the same kind of logic foun din `customAliases` to the
* OpenSsl code base directly.
*
Expand Down
16 changes: 8 additions & 8 deletions cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/PaddingAlgorithmInstance.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@
exists(string name |
name = e.(KnownOpenSslAlgorithmExpr).getNormalizedName() and
(
name.matches("OAEP") and type = Crypto::OAEP()
name = "OAEP" and type = Crypto::OAEP()
or
name.matches("PSS") and type = Crypto::PSS()
name = "PSS" and type = Crypto::PSS()
or
name.matches("PKCS7") and type = Crypto::PKCS7()
name = "PKCS7" and type = Crypto::PKCS7()
or
name.matches("PKCS1V15") and type = Crypto::PKCS1_v1_5()
name = "PKCS1V15" and type = Crypto::PKCS1_v1_5()
)
)
}
Expand Down Expand Up @@ -85,7 +85,7 @@
// Source is `this`
src.asExpr() = this and
// This traces to a padding-specific consumer
RSAPaddingAlgorithmToPaddingAlgorithmValueConsumerFlow::flow(src, sink)
RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerFlow::flow(src, sink)
) and
isPaddingSpecificConsumer = true
}
Expand Down Expand Up @@ -143,7 +143,7 @@
// this instanceof Literal and
// this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8]
// // TODO: trace to padding-specific consumers
// RSAPaddingAlgorithmToPaddingAlgorithmValueConsumerFlow
// RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerFlow
// }
// override string getRawPaddingAlgorithmName() { result = this.(Literal).getValue().toString() }
// override Crypto::TPaddingType getPaddingType() {
Expand All @@ -161,10 +161,10 @@
// else result = Crypto::OtherPadding()
// }
// }
class OAEPPaddingAlgorithmInstance extends Crypto::OAEPPaddingAlgorithmInstance,
class OaepPaddingAlgorithmInstance extends Crypto::OAEPPaddingAlgorithmInstance,
KnownOpenSslPaddingConstantAlgorithmInstance
{
OAEPPaddingAlgorithmInstance() {
OaepPaddingAlgorithmInstance() {
this.(Crypto::PaddingAlgorithmInstance).getPaddingType() = Crypto::OAEP()
}

Expand Down
8 changes: 4 additions & 4 deletions ...lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/DirectAlgorithmValueConsumer.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmCon
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase

/**
* Cases like EVP_MD5(),
* there is no input, rather it directly gets an algorithm
* and returns it.
* Also includes operations directly using an algorithm
* A call that is considered to inherently 'consume' an algorithm value.
* E.g., cases like EVP_MD5(),
* where there is no input, rather it directly gets an algorithm
* and returns it. Also includes operations directly using an algorithm
* like AES_encrypt().
*/
class DirectAlgorithmValueConsumer extends OpenSslAlgorithmValueConsumer instanceof OpenSslAlgorithmCall
Expand Down
2 changes: 1 addition & 1 deletion ...l/lib/experimental/quantum/OpenSSL/AlgorithmValueConsumers/HashAlgorithmValueConsumer.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ private import experimental.quantum.OpenSSL.AlgorithmInstances.OpenSSLAlgorithmI
abstract class HashAlgorithmValueConsumer extends OpenSslAlgorithmValueConsumer { }

/**
* EVP_Q_Digest directly consumes algorithm constant values
* An EVP_Q_Digest directly consumes algorithm constant values
*/
class Evp_Q_Digest_Algorithm_Consumer extends HashAlgorithmValueConsumer {
Evp_Q_Digest_Algorithm_Consumer() { this.(Call).getTarget().getName() = "EVP_Q_digest" }
Expand Down
4 changes: 3 additions & 1 deletion cpp/ql/lib/experimental/quantum/OpenSSL/Operations/EVPCipherOperation.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ class Evp_Cipher_Update_Call extends EvpUpdate {
}

/**
* see: https://docs.openssl.org/master/man3/EVP_EncryptInit/#synopsis
* The EVP Cipher operations.
* See: https://docs.openssl.org/master/man3/EVP_EncryptInit/#synopsis
* Base configuration for all EVP cipher operations.
*/
abstract class Evp_Cipher_Operation extends EvpOperation, Crypto::KeyOperationInstance {
Expand Down Expand Up @@ -163,6 +164,7 @@ class Evp_Cipher_Final_Call extends EvpFinal, Evp_Cipher_Operation {
}

/**
* The EVP encryption/decryption operations.
* https://docs.openssl.org/3.2/man3/EVP_PKEY_decrypt/
* https://docs.openssl.org/3.2/man3/EVP_PKEY_encrypt
*/
Expand Down