Skip to content

Quantum: Added signature input nodes to signature verify operation nodes #19623

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
Jun 2, 2025
Merged
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
74 changes: 69 additions & 5 deletions shared/quantum/codeql/quantum/experimental/Model.qll
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,17 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
final override ConsumerInputDataFlowNode getInputNode() { result = inputNode }
}

final private class SignatureArtifactConsumer extends ArtifactConsumerAndInstance {
ConsumerInputDataFlowNode inputNode;

SignatureArtifactConsumer() {
exists(SignatureOperationInstance op | inputNode = op.getSignatureConsumer()) and
this = Input::dfn_to_element(inputNode)
}

final override ConsumerInputDataFlowNode getInputNode() { result = inputNode }
}

/**
* An artifact that is produced by an operation, representing a concrete artifact instance rather than a synthetic consumer artifact.
*/
Expand Down Expand Up @@ -458,6 +469,8 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
}

override DataFlowNode getOutputNode() { result = creator.getOutputArtifact() }

KeyOperationInstance getCreator() { result = creator }
}

/**
Expand Down Expand Up @@ -782,6 +795,17 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
abstract ArtifactOutputDataFlowNode getOutputArtifact();
}

/**
* A key operation instance representing a signature being generated or verified.
*/
abstract class SignatureOperationInstance extends KeyOperationInstance {
/**
* Gets the consumer of the signature that is being verified in case of a
* verification operation.
*/
abstract ConsumerInputDataFlowNode getSignatureConsumer();
}

/**
* A key-based algorithm instance used in cryptographic operations such as encryption, decryption,
* signing, verification, and key wrapping.
Expand Down Expand Up @@ -1264,6 +1288,7 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
TNonceInput(NonceArtifactConsumer e) or
TMessageInput(MessageArtifactConsumer e) or
TSaltInput(SaltArtifactConsumer e) or
TSignatureInput(SignatureArtifactConsumer e) or
TRandomNumberGeneration(RandomNumberGenerationInstance e) { e.flowsTo(_) } or
// Key Creation Operation union type (e.g., key generation, key load)
TKeyCreationOperation(KeyCreationOperationInstance e) or
Expand Down Expand Up @@ -1325,14 +1350,14 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
/**
* Returns the child of this node with the given edge name.
*
* This predicate is overriden by derived classes to construct the graph of cryptographic operations.
* This predicate is overridden by derived classes to construct the graph of cryptographic operations.
*/
NodeBase getChild(string edgeName) { none() }

/**
* Defines properties of this node by name and either a value or location or both.
*
* This predicate is overriden by derived classes to construct the graph of cryptographic operations.
* This predicate is overridden by derived classes to construct the graph of cryptographic operations.
*/
predicate properties(string key, string value, Location location) { none() }

Expand Down Expand Up @@ -1505,6 +1530,20 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
override LocatableElement asElement() { result = instance }
}

/**
* A signature input. This may represent a signature, or a signature component
* such as the scalar values r and s in ECDSA.
*/
final class SignatureArtifactNode extends ArtifactNode, TSignatureInput {
SignatureArtifactConsumer instance;

SignatureArtifactNode() { this = TSignatureInput(instance) }

final override string getInternalType() { result = "SignatureInput" }

override LocatableElement asElement() { result = instance }
}

/**
* A salt input.
*/
Expand All @@ -1528,13 +1567,22 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {

KeyOperationOutputNode() { this = TKeyOperationOutput(instance) }

final override string getInternalType() { result = "KeyOperationOutput" }
override string getInternalType() { result = "KeyOperationOutput" }

override LocatableElement asElement() { result = instance }

override string getSourceNodeRelationship() { none() }
}

class SignOperationOutputNode extends KeyOperationOutputNode {
SignOperationOutputNode() {
this.asElement().(KeyOperationOutputArtifactInstance).getCreator().getKeyOperationSubtype() =
TSignMode()
}

override string getInternalType() { result = "SignatureOutput" }
}

/**
* A source of random number generation.
*/
Expand Down Expand Up @@ -2107,6 +2155,7 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
}

class SignatureOperationNode extends KeyOperationNode {
override SignatureOperationInstance instance;
string nodeName;

SignatureOperationNode() {
Expand All @@ -2116,6 +2165,21 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
}

override string getInternalType() { result = nodeName }

SignatureArtifactNode getASignatureArtifact() {
result.asElement() = instance.getSignatureConsumer().getConsumer()
}

override NodeBase getChild(string key) {
result = super.getChild(key)
or
// [KNOWN_OR_UNKNOWN] - only if we know the type is verify
this.getKeyOperationSubtype() = TVerifyMode() and
key = "Signature" and
if exists(this.getASignatureArtifact())
then result = this.getASignatureArtifact()
else result = this
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a line here, displaying a signature edge if and only if it's a verify operation. As-is, we will report an unknown edge even for signing operations, which do not take in a signature input.

}
}

/**
Expand Down Expand Up @@ -2563,15 +2627,15 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
or
curveName = "CURVE25519" and keySize = 255 and curveFamily = CURVE25519()
or
curveName = "CURVE448" and keySize = 448 and curveFamily = CURVE448()
or
// TODO: separate these into key agreement logic or sign/verify (ECDSA / ECDH)
// or
// curveName = "X25519" and keySize = 255 and curveFamily = CURVE25519()
// or
// curveName = "ED25519" and keySize = 255 and curveFamily = CURVE25519()
// or
// curveName = "ED448" and keySize = 448 and curveFamily = CURVE448()
// curveName = "CURVE448" and keySize = 448 and curveFamily = CURVE448()
// or
// or
// curveName = "X448" and keySize = 448 and curveFamily = CURVE448()
curveName = "SM2" and keySize in [256, 512] and curveFamily = SM2()
Expand Down
Loading