Skip to content
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
80 changes: 37 additions & 43 deletions cpp/ql/lib/experimental/Quantum/Language.qll
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
private import codeql.cryptography.Model
import semmle.code.cpp.ir.IR
import semmle.code.cpp.security.FlowSources as FlowSources
import semmle.code.cpp.dataflow.new.DataFlow
private import cpp as Lang

module CryptoInput implements InputSig<Lang::Location> {
Expand All @@ -15,10 +16,44 @@ module CryptoInput implements InputSig<Lang::Location> {
result = node.asParameter() or
result = node.asVariable()
}

string locationToFileBaseNameAndLineNumberString(Location location) {
result = location.getFile().getBaseName() + ":" + location.getStartLine()
}

predicate artifactOutputFlowsToGenericInput(
DataFlow::Node artifactOutput, DataFlow::Node otherInput
) {
ArtifactFlow::flow(artifactOutput, otherInput)
}
}

module Crypto = CryptographyBase<Lang::Location, CryptoInput>;

module ArtifactFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source = any(Crypto::ArtifactInstance artifact).getOutputNode()
}

predicate isSink(DataFlow::Node sink) {
sink = any(Crypto::FlowAwareElement other).getInputNode()
}

predicate isBarrierOut(DataFlow::Node node) {
node = any(Crypto::FlowAwareElement element).getInputNode()
}

predicate isBarrierIn(DataFlow::Node node) {
node = any(Crypto::FlowAwareElement element).getOutputNode()
}

predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
node1.(AdditionalFlowInputStep).getOutput() = node2
}
}

module ArtifactFlow = DataFlow::Global<ArtifactFlowConfig>;

/**
* Artifact output to node input configuration
*/
Expand All @@ -31,9 +66,9 @@ abstract class AdditionalFlowInputStep extends DataFlow::Node {
/**
* Generic data source to node input configuration
*/
module GenericDataSourceUniversalFlowConfig implements DataFlow::ConfigSig {
module GenericDataSourceFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source = any(Crypto::GenericDataSourceInstance i).getOutputNode()
source = any(Crypto::GenericSourceInstance i).getOutputNode()
}

predicate isSink(DataFlow::Node sink) {
Expand All @@ -53,41 +88,6 @@ module GenericDataSourceUniversalFlowConfig implements DataFlow::ConfigSig {
}
}

// // // TODO: I think this will be inefficient, no?
// // class ConstantDataSource extends Crypto::GenericConstantOrAllocationSource instanceof Literal {
// // override DataFlow::Node getOutputNode() {
// // result.asExpr() = this
// // }
// // override predicate flowsTo(Crypto::FlowAwareElement other) {
// // // TODO: separate config to avoid blowing up data-flow analysis
// // GenericDataSourceUniversalFlow::flow(this.getOutputNode(), other.getInputNode())
// // }
// // override string getAdditionalDescription() { result = this.toString() }
// // }
// /**
// * Definitions of various generic data sources
// */
// // final class DefaultFlowSource = SourceNode;
// // final class DefaultRemoteFlowSource = RemoteFlowSource;
// // class GenericLocalDataSource extends Crypto::GenericLocalDataSource {
// // GenericLocalDataSource() {
// // any(DefaultFlowSource src | not src instanceof DefaultRemoteFlowSource).asExpr() = this
// // }
// // override DataFlow::Node getOutputNode() { result.asExpr() = this }
// // override predicate flowsTo(Crypto::FlowAwareElement other) {
// // GenericDataSourceUniversalFlow::flow(this.getOutputNode(), other.getInputNode())
// // }
// // override string getAdditionalDescription() { result = this.toString() }
// // }
// // class GenericRemoteDataSource extends Crypto::GenericRemoteDataSource {
// // GenericRemoteDataSource() { any(DefaultRemoteFlowSource src).asExpr() = this }
// // override DataFlow::Node getOutputNode() { result.asExpr() = this }
// // override predicate flowsTo(Crypto::FlowAwareElement other) {
// // GenericDataSourceUniversalFlow::flow(this.getOutputNode(), other.getInputNode())
// // }
// // override string getAdditionalDescription() { result = this.toString() }
// // }
// module GenericDataSourceUniversalFlow = DataFlow::Global<GenericDataSourceUniversalFlowConfig>;
module ArtifactUniversalFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source = any(Crypto::ArtifactInstance artifact).getOutputNode()
Expand All @@ -112,10 +112,4 @@ module ArtifactUniversalFlowConfig implements DataFlow::ConfigSig {

module ArtifactUniversalFlow = DataFlow::Global<ArtifactUniversalFlowConfig>;

abstract class CipherOutputArtifact extends Crypto::KeyOperationOutputArtifactInstance {
override predicate flowsTo(Crypto::FlowAwareElement other) {
ArtifactUniversalFlow::flow(this.getOutputNode(), other.getInputNode())
}
}

import OpenSSL.OpenSSL
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
import cpp
import semmle.code.cpp.dataflow.new.DataFlow
import experimental.Quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
import experimental.Quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers // import all known alg value consummers

/**
* Traces 'known algorithms' to AVCs, specifically
* algorithms that are in the set of known algorithm constants.
* Padding-specific consumers exist that have their own values that
* overlap with the known algorithm constants.
* Padding consumers (specific padding consumers) are excluded from the set of sinks.
*/
module KnownOpenSSLAlgorithmToAlgorithmValueConsumerConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source.asExpr() instanceof KnownOpenSSLAlgorithmConstant
}

predicate isSink(DataFlow::Node sink) {
exists(OpenSSLAlgorithmValueConsumer c |
c.getInputNode() = sink and
not c instanceof PaddingAlgorithmValueConsumer
)
}

predicate isBarrier(DataFlow::Node node) {
// False positive reducer, don't flow out through argv
exists(VariableAccess va, Variable v |
v.getAnAccess() = va and va = node.asExpr()
or
va = node.asIndirectExpr()
|
v.getName().matches("%argv")
)
}

predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
knownPassThroughStep(node1, node2)
}
}

module KnownOpenSSLAlgorithmToAlgorithmValueConsumerFlow =
DataFlow::Global<KnownOpenSSLAlgorithmToAlgorithmValueConsumerConfig>;

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

predicate isSink(DataFlow::Node sink) {
exists(PaddingAlgorithmValueConsumer c | c.getInputNode() = sink)
}

predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
knownPassThroughStep(node1, node2)
}
}

module RSAPaddingAlgorithmToPaddingAlgorithmValueConsumerFlow =
DataFlow::Global<RSAPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig>;

class OpenSSLAlgorithmAdditionalFlowStep extends AdditionalFlowInputStep {
OpenSSLAlgorithmAdditionalFlowStep() { exists(AlgorithmPassthroughCall c | c.getInNode() = this) }

override DataFlow::Node getOutput() {
exists(AlgorithmPassthroughCall c | c.getInNode() = this and c.getOutNode() = result)
}
}

abstract class AlgorithmPassthroughCall extends Call {
abstract DataFlow::Node getInNode();

abstract DataFlow::Node getOutNode();
}

class CopyAndDupAlgorithmPassthroughCall extends AlgorithmPassthroughCall {
DataFlow::Node inNode;
DataFlow::Node outNode;

CopyAndDupAlgorithmPassthroughCall() {
// Flow out through any return or other argument of the same type
// Assume flow in and out is asIndirectExpr or asDefinitingArgument since a pointer is assumed
// to be involved
// NOTE: not attempting to detect openssl specific copy/dup functions, but anything suspected to be copy/dup
Copy link
Owner

Choose a reason for hiding this comment

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

Since this is supposed to be a heuristic, can you include examples?

Suggested change:

// Flow may propagate through return values or other arguments of the same type.
// Assume both incoming and outgoing flow use `asIndirectExpr` or `asDefiningArgument`,
// since pointers are likely involved.
// NOTE: This does not attempt to detect OpenSSL-specific copy/dup functions;
// it targets any function suspected to perform copying or duplication.

Also, what about type conversions, such as byte arrays to strings? Perhaps that's a different bit of logic and one handled anyway via taint-tracking, but I want to make clear what this case (vs the generic catch-all taint tracking flow steps) are for.

this.getTarget().getName().toLowerCase().matches(["%_dup%", "%_copy%"]) and
exists(Expr inArg, Type t |
inArg = this.getAnArgument() and t = inArg.getUnspecifiedType().stripType()
|
inNode.asIndirectExpr() = inArg and
(
// Case 1: flow through another argument as an out arg of the same type
exists(Expr outArg |
outArg = this.getAnArgument() and
outArg != inArg and
outArg.getUnspecifiedType().stripType() = t
|
outNode.asDefiningArgument() = outArg
)
or
// Case 2: flow through the return value if the result is the same as the intput type
exists(Expr outArg | outArg = this and outArg.getUnspecifiedType().stripType() = t |
outNode.asIndirectExpr() = outArg
)
)
)
}

override DataFlow::Node getInNode() { result = inNode }

override DataFlow::Node getOutNode() { result = outNode }
}

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

NIDToPointerPassthroughCall() {
this.getTarget().getName() in ["OBJ_nid2obj", "OBJ_nid2ln", "OBJ_nid2sn"] and
inNode.asExpr() = this.getArgument(0) and
outNode.asExpr() = this
//outNode.asIndirectExpr() = this
}

override DataFlow::Node getInNode() { result = inNode }

override DataFlow::Node getOutNode() { result = outNode }
}

class PointerToPointerPassthroughCall extends AlgorithmPassthroughCall {
DataFlow::Node inNode;
DataFlow::Node outNode;

PointerToPointerPassthroughCall() {
this.getTarget().getName() = "OBJ_txt2obj" and
inNode.asIndirectExpr() = this.getArgument(0) and
outNode.asIndirectExpr() = this
or
//outNode.asExpr() = this
this.getTarget().getName() in ["OBJ_obj2txt", "i2t_ASN1_OBJECT"] and
inNode.asIndirectExpr() = this.getArgument(2) and
outNode.asDefiningArgument() = this.getArgument(0)
}

override DataFlow::Node getInNode() { result = inNode }

override DataFlow::Node getOutNode() { result = outNode }
}

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

PointerToNIDPassthroughCall() {
this.getTarget().getName() in ["OBJ_obj2nid", "OBJ_ln2nid", "OBJ_sn2nid", "OBJ_txt2nid"] and
(
inNode.asIndirectExpr() = this.getArgument(0)
or
Copy link
Owner

Choose a reason for hiding this comment

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

Only asExpr or asIndirectExpr are going to be correct in any one given instance. If it depends on the function, then we need to specialize that per-function to avoid issues, particularly in the indirect case.

inNode.asExpr() = this.getArgument(0)
) and
outNode.asExpr() = this
}

override DataFlow::Node getInNode() { result = inNode }

override DataFlow::Node getOutNode() { result = outNode }
}

// TODO: pkeys pass through EVP_PKEY_CTX_new and any similar variant
predicate knownPassThroughStep(DataFlow::Node node1, DataFlow::Node node2) {
exists(AlgorithmPassthroughCall c | c.getInNode() = node1 and c.getOutNode() = node2)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import cpp
import experimental.Quantum.Language
import OpenSSLAlgorithmInstanceBase
import experimental.Quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
import experimental.Quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
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').
*/
predicate knownOpenSSLConstantToBlockModeFamilyType(
KnownOpenSSLBlockModeAlgorithmConstant e, Crypto::TBlockCipherModeOfOperationType type
) {
exists(string name |
name = e.getNormalizedName() and
(
name.matches("CBC") and type instanceof Crypto::CBC
or
name.matches("CFB%") and type instanceof Crypto::CFB
or
name.matches("CTR") and type instanceof Crypto::CTR
or
name.matches("GCM") and type instanceof Crypto::GCM
or
name.matches("OFB") and type instanceof Crypto::OFB
or
name.matches("XTS") and type instanceof Crypto::XTS
or
name.matches("CCM") and type instanceof Crypto::CCM
or
name.matches("GCM") and type instanceof Crypto::GCM
or
name.matches("CCM") and type instanceof Crypto::CCM
or
name.matches("ECB") and type instanceof Crypto::ECB
)
)
}

class KnownOpenSSLBlockModeConstantAlgorithmInstance extends OpenSSLAlgorithmInstance,
Crypto::ModeOfOperationAlgorithmInstance instanceof KnownOpenSSLBlockModeAlgorithmConstant
{
OpenSSLAlgorithmValueConsumer getterCall;

KnownOpenSSLBlockModeConstantAlgorithmInstance() {
// Two possibilities:
// 1) The source is a literal and flows to a getter, then we know we have an instance
// 2) The source is a KnownOpenSSLAlgorithm is call, and we know we have an instance immediately from that
// Possibility 1:
this instanceof Literal and
exists(DataFlow::Node src, DataFlow::Node sink |
// Sink is an argument to a CipherGetterCall
sink = getterCall.(OpenSSLAlgorithmValueConsumer).getInputNode() and
// Source is `this`
src.asExpr() = this and
// This traces to a getter
KnownOpenSSLAlgorithmToAlgorithmValueConsumerFlow::flow(src, sink)
)
or
// Possibility 2:
this instanceof DirectAlgorithmValueConsumer and getterCall = this
}

override Crypto::TBlockCipherModeOfOperationType getModeType() {
knownOpenSSLConstantToBlockModeFamilyType(this, result)
or
not knownOpenSSLConstantToBlockModeFamilyType(this, _) and result = Crypto::OtherMode()
}

// NOTE: I'm not going to attempt to parse out the mode specific part, so returning
// the same as the raw name for now.
override string getRawModeAlgorithmName() { result = this.(Literal).getValue().toString() }

override OpenSSLAlgorithmValueConsumer getAVC() { result = getterCall }
}
Loading
Loading