Skip to content

Commit f5a3656

Browse files
nicolaswillnicolaswill
authored
Merge pull request #11 from nicolaswill/brodes/openssl_refactor
Brodes/openssl refactor
2 parents c80588c + 4042081 commit f5a3656

30 files changed

+1639
-1230
lines changed

‎cpp/ql/lib/experimental/Quantum/Language.qll

Lines changed: 37 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
private import codeql.cryptography.Model
22
import semmle.code.cpp.ir.IR
33
import semmle.code.cpp.security.FlowSources as FlowSources
4+
import semmle.code.cpp.dataflow.new.DataFlow
45
private import cpp as Lang
56

67
module CryptoInput implements InputSig<Lang::Location> {
@@ -15,10 +16,44 @@ module CryptoInput implements InputSig<Lang::Location> {
1516
result = node.asParameter() or
1617
result = node.asVariable()
1718
}
19+
20+
string locationToFileBaseNameAndLineNumberString(Location location) {
21+
result = location.getFile().getBaseName() + ":" + location.getStartLine()
22+
}
23+
24+
predicate artifactOutputFlowsToGenericInput(
25+
DataFlow::Node artifactOutput, DataFlow::Node otherInput
26+
) {
27+
ArtifactFlow::flow(artifactOutput, otherInput)
28+
}
1829
}
1930

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

33+
module ArtifactFlowConfig implements DataFlow::ConfigSig {
34+
predicate isSource(DataFlow::Node source) {
35+
source = any(Crypto::ArtifactInstance artifact).getOutputNode()
36+
}
37+
38+
predicate isSink(DataFlow::Node sink) {
39+
sink = any(Crypto::FlowAwareElement other).getInputNode()
40+
}
41+
42+
predicate isBarrierOut(DataFlow::Node node) {
43+
node = any(Crypto::FlowAwareElement element).getInputNode()
44+
}
45+
46+
predicate isBarrierIn(DataFlow::Node node) {
47+
node = any(Crypto::FlowAwareElement element).getOutputNode()
48+
}
49+
50+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
51+
node1.(AdditionalFlowInputStep).getOutput() = node2
52+
}
53+
}
54+
55+
module ArtifactFlow = DataFlow::Global<ArtifactFlowConfig>;
56+
2257
/**
2358
* Artifact output to node input configuration
2459
*/
@@ -31,9 +66,9 @@ abstract class AdditionalFlowInputStep extends DataFlow::Node {
3166
/**
3267
* Generic data source to node input configuration
3368
*/
34-
module GenericDataSourceUniversalFlowConfig implements DataFlow::ConfigSig {
69+
module GenericDataSourceFlowConfig implements DataFlow::ConfigSig {
3570
predicate isSource(DataFlow::Node source) {
36-
source = any(Crypto::GenericDataSourceInstance i).getOutputNode()
71+
source = any(Crypto::GenericSourceInstance i).getOutputNode()
3772
}
3873

3974
predicate isSink(DataFlow::Node sink) {
@@ -53,41 +88,6 @@ module GenericDataSourceUniversalFlowConfig implements DataFlow::ConfigSig {
5388
}
5489
}
5590

56-
// // // TODO: I think this will be inefficient, no?
57-
// // class ConstantDataSource extends Crypto::GenericConstantOrAllocationSource instanceof Literal {
58-
// // override DataFlow::Node getOutputNode() {
59-
// // result.asExpr() = this
60-
// // }
61-
// // override predicate flowsTo(Crypto::FlowAwareElement other) {
62-
// // // TODO: separate config to avoid blowing up data-flow analysis
63-
// // GenericDataSourceUniversalFlow::flow(this.getOutputNode(), other.getInputNode())
64-
// // }
65-
// // override string getAdditionalDescription() { result = this.toString() }
66-
// // }
67-
// /**
68-
// * Definitions of various generic data sources
69-
// */
70-
// // final class DefaultFlowSource = SourceNode;
71-
// // final class DefaultRemoteFlowSource = RemoteFlowSource;
72-
// // class GenericLocalDataSource extends Crypto::GenericLocalDataSource {
73-
// // GenericLocalDataSource() {
74-
// // any(DefaultFlowSource src | not src instanceof DefaultRemoteFlowSource).asExpr() = this
75-
// // }
76-
// // override DataFlow::Node getOutputNode() { result.asExpr() = this }
77-
// // override predicate flowsTo(Crypto::FlowAwareElement other) {
78-
// // GenericDataSourceUniversalFlow::flow(this.getOutputNode(), other.getInputNode())
79-
// // }
80-
// // override string getAdditionalDescription() { result = this.toString() }
81-
// // }
82-
// // class GenericRemoteDataSource extends Crypto::GenericRemoteDataSource {
83-
// // GenericRemoteDataSource() { any(DefaultRemoteFlowSource src).asExpr() = this }
84-
// // override DataFlow::Node getOutputNode() { result.asExpr() = this }
85-
// // override predicate flowsTo(Crypto::FlowAwareElement other) {
86-
// // GenericDataSourceUniversalFlow::flow(this.getOutputNode(), other.getInputNode())
87-
// // }
88-
// // override string getAdditionalDescription() { result = this.toString() }
89-
// // }
90-
// module GenericDataSourceUniversalFlow = DataFlow::Global<GenericDataSourceUniversalFlowConfig>;
9191
module ArtifactUniversalFlowConfig implements DataFlow::ConfigSig {
9292
predicate isSource(DataFlow::Node source) {
9393
source = any(Crypto::ArtifactInstance artifact).getOutputNode()
@@ -112,10 +112,4 @@ module ArtifactUniversalFlowConfig implements DataFlow::ConfigSig {
112112

113113
module ArtifactUniversalFlow = DataFlow::Global<ArtifactUniversalFlowConfig>;
114114

115-
abstract class CipherOutputArtifact extends Crypto::KeyOperationOutputArtifactInstance {
116-
override predicate flowsTo(Crypto::FlowAwareElement other) {
117-
ArtifactUniversalFlow::flow(this.getOutputNode(), other.getInputNode())
118-
}
119-
}
120-
121115
import OpenSSL.OpenSSL

‎cpp/ql/lib/experimental/Quantum/OpenSSL/AlgorithmInstances/AlgToAVCFlow.qll

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
import cpp
2+
import semmle.code.cpp.dataflow.new.DataFlow
3+
import experimental.Quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
4+
import experimental.Quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers // import all known alg value consummers
5+
6+
/**
7+
* Traces 'known algorithms' to AVCs, specifically
8+
* algorithms that are in the set of known algorithm constants.
9+
* Padding-specific consumers exist that have their own values that
10+
* overlap with the known algorithm constants.
11+
* Padding consumers (specific padding consumers) are excluded from the set of sinks.
12+
*/
13+
module KnownOpenSSLAlgorithmToAlgorithmValueConsumerConfig implements DataFlow::ConfigSig {
14+
predicate isSource(DataFlow::Node source) {
15+
source.asExpr() instanceof KnownOpenSSLAlgorithmConstant
16+
}
17+
18+
predicate isSink(DataFlow::Node sink) {
19+
exists(OpenSSLAlgorithmValueConsumer c |
20+
c.getInputNode() = sink and
21+
not c instanceof PaddingAlgorithmValueConsumer
22+
)
23+
}
24+
25+
predicate isBarrier(DataFlow::Node node) {
26+
// False positive reducer, don't flow out through argv
27+
exists(VariableAccess va, Variable v |
28+
v.getAnAccess() = va and va = node.asExpr()
29+
or
30+
va = node.asIndirectExpr()
31+
|
32+
v.getName().matches("%argv")
33+
)
34+
}
35+
36+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
37+
knownPassThroughStep(node1, node2)
38+
}
39+
}
40+
41+
module KnownOpenSSLAlgorithmToAlgorithmValueConsumerFlow =
42+
DataFlow::Global<KnownOpenSSLAlgorithmToAlgorithmValueConsumerConfig>;
43+
44+
module RSAPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig implements DataFlow::ConfigSig {
45+
predicate isSource(DataFlow::Node source) {
46+
source.asExpr() instanceof KnownOpenSSLAlgorithmConstant
47+
}
48+
49+
predicate isSink(DataFlow::Node sink) {
50+
exists(PaddingAlgorithmValueConsumer c | c.getInputNode() = sink)
51+
}
52+
53+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
54+
knownPassThroughStep(node1, node2)
55+
}
56+
}
57+
58+
module RSAPaddingAlgorithmToPaddingAlgorithmValueConsumerFlow =
59+
DataFlow::Global<RSAPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig>;
60+
61+
class OpenSSLAlgorithmAdditionalFlowStep extends AdditionalFlowInputStep {
62+
OpenSSLAlgorithmAdditionalFlowStep() { exists(AlgorithmPassthroughCall c | c.getInNode() = this) }
63+
64+
override DataFlow::Node getOutput() {
65+
exists(AlgorithmPassthroughCall c | c.getInNode() = this and c.getOutNode() = result)
66+
}
67+
}
68+
69+
abstract class AlgorithmPassthroughCall extends Call {
70+
abstract DataFlow::Node getInNode();
71+
72+
abstract DataFlow::Node getOutNode();
73+
}
74+
75+
class CopyAndDupAlgorithmPassthroughCall extends AlgorithmPassthroughCall {
76+
DataFlow::Node inNode;
77+
DataFlow::Node outNode;
78+
79+
CopyAndDupAlgorithmPassthroughCall() {
80+
// Flow out through any return or other argument of the same type
81+
// Assume flow in and out is asIndirectExpr or asDefinitingArgument since a pointer is assumed
82+
// to be involved
83+
// NOTE: not attempting to detect openssl specific copy/dup functions, but anything suspected to be copy/dup
84+
this.getTarget().getName().toLowerCase().matches(["%_dup%", "%_copy%"]) and
85+
exists(Expr inArg, Type t |
86+
inArg = this.getAnArgument() and t = inArg.getUnspecifiedType().stripType()
87+
|
88+
inNode.asIndirectExpr() = inArg and
89+
(
90+
// Case 1: flow through another argument as an out arg of the same type
91+
exists(Expr outArg |
92+
outArg = this.getAnArgument() and
93+
outArg != inArg and
94+
outArg.getUnspecifiedType().stripType() = t
95+
|
96+
outNode.asDefiningArgument() = outArg
97+
)
98+
or
99+
// Case 2: flow through the return value if the result is the same as the intput type
100+
exists(Expr outArg | outArg = this and outArg.getUnspecifiedType().stripType() = t |
101+
outNode.asIndirectExpr() = outArg
102+
)
103+
)
104+
)
105+
}
106+
107+
override DataFlow::Node getInNode() { result = inNode }
108+
109+
override DataFlow::Node getOutNode() { result = outNode }
110+
}
111+
112+
class NIDToPointerPassthroughCall extends AlgorithmPassthroughCall {
113+
DataFlow::Node inNode;
114+
DataFlow::Node outNode;
115+
116+
NIDToPointerPassthroughCall() {
117+
this.getTarget().getName() in ["OBJ_nid2obj", "OBJ_nid2ln", "OBJ_nid2sn"] and
118+
inNode.asExpr() = this.getArgument(0) and
119+
outNode.asExpr() = this
120+
//outNode.asIndirectExpr() = this
121+
}
122+
123+
override DataFlow::Node getInNode() { result = inNode }
124+
125+
override DataFlow::Node getOutNode() { result = outNode }
126+
}
127+
128+
class PointerToPointerPassthroughCall extends AlgorithmPassthroughCall {
129+
DataFlow::Node inNode;
130+
DataFlow::Node outNode;
131+
132+
PointerToPointerPassthroughCall() {
133+
this.getTarget().getName() = "OBJ_txt2obj" and
134+
inNode.asIndirectExpr() = this.getArgument(0) and
135+
outNode.asIndirectExpr() = this
136+
or
137+
//outNode.asExpr() = this
138+
this.getTarget().getName() in ["OBJ_obj2txt", "i2t_ASN1_OBJECT"] and
139+
inNode.asIndirectExpr() = this.getArgument(2) and
140+
outNode.asDefiningArgument() = this.getArgument(0)
141+
}
142+
143+
override DataFlow::Node getInNode() { result = inNode }
144+
145+
override DataFlow::Node getOutNode() { result = outNode }
146+
}
147+
148+
class PointerToNIDPassthroughCall extends AlgorithmPassthroughCall {
149+
DataFlow::Node inNode;
150+
DataFlow::Node outNode;
151+
152+
PointerToNIDPassthroughCall() {
153+
this.getTarget().getName() in ["OBJ_obj2nid", "OBJ_ln2nid", "OBJ_sn2nid", "OBJ_txt2nid"] and
154+
(
155+
inNode.asIndirectExpr() = this.getArgument(0)
156+
or
157+
inNode.asExpr() = this.getArgument(0)
158+
) and
159+
outNode.asExpr() = this
160+
}
161+
162+
override DataFlow::Node getInNode() { result = inNode }
163+
164+
override DataFlow::Node getOutNode() { result = outNode }
165+
}
166+
167+
// TODO: pkeys pass through EVP_PKEY_CTX_new and any similar variant
168+
predicate knownPassThroughStep(DataFlow::Node node1, DataFlow::Node node2) {
169+
exists(AlgorithmPassthroughCall c | c.getInNode() = node1 and c.getOutNode() = node2)
170+
}

‎cpp/ql/lib/experimental/Quantum/OpenSSL/AlgorithmInstances/BlockAlgorithmInstance.qll

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import cpp
2+
import experimental.Quantum.Language
3+
import OpenSSLAlgorithmInstanceBase
4+
import experimental.Quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
5+
import experimental.Quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
6+
import AlgToAVCFlow
7+
8+
/**
9+
* Given a `KnownOpenSSLBlockModeAlgorithmConstant`, converts this to a block family type.
10+
* Does not bind if there is know mapping (no mapping to 'unknown' or 'other').
11+
*/
12+
predicate knownOpenSSLConstantToBlockModeFamilyType(
13+
KnownOpenSSLBlockModeAlgorithmConstant e, Crypto::TBlockCipherModeOfOperationType type
14+
) {
15+
exists(string name |
16+
name = e.getNormalizedName() and
17+
(
18+
name.matches("CBC") and type instanceof Crypto::CBC
19+
or
20+
name.matches("CFB%") and type instanceof Crypto::CFB
21+
or
22+
name.matches("CTR") and type instanceof Crypto::CTR
23+
or
24+
name.matches("GCM") and type instanceof Crypto::GCM
25+
or
26+
name.matches("OFB") and type instanceof Crypto::OFB
27+
or
28+
name.matches("XTS") and type instanceof Crypto::XTS
29+
or
30+
name.matches("CCM") and type instanceof Crypto::CCM
31+
or
32+
name.matches("GCM") and type instanceof Crypto::GCM
33+
or
34+
name.matches("CCM") and type instanceof Crypto::CCM
35+
or
36+
name.matches("ECB") and type instanceof Crypto::ECB
37+
)
38+
)
39+
}
40+
41+
class KnownOpenSSLBlockModeConstantAlgorithmInstance extends OpenSSLAlgorithmInstance,
42+
Crypto::ModeOfOperationAlgorithmInstance instanceof KnownOpenSSLBlockModeAlgorithmConstant
43+
{
44+
OpenSSLAlgorithmValueConsumer getterCall;
45+
46+
KnownOpenSSLBlockModeConstantAlgorithmInstance() {
47+
// Two possibilities:
48+
// 1) The source is a literal and flows to a getter, then we know we have an instance
49+
// 2) The source is a KnownOpenSSLAlgorithm is call, and we know we have an instance immediately from that
50+
// Possibility 1:
51+
this instanceof Literal and
52+
exists(DataFlow::Node src, DataFlow::Node sink |
53+
// Sink is an argument to a CipherGetterCall
54+
sink = getterCall.(OpenSSLAlgorithmValueConsumer).getInputNode() and
55+
// Source is `this`
56+
src.asExpr() = this and
57+
// This traces to a getter
58+
KnownOpenSSLAlgorithmToAlgorithmValueConsumerFlow::flow(src, sink)
59+
)
60+
or
61+
// Possibility 2:
62+
this instanceof DirectAlgorithmValueConsumer and getterCall = this
63+
}
64+
65+
override Crypto::TBlockCipherModeOfOperationType getModeType() {
66+
knownOpenSSLConstantToBlockModeFamilyType(this, result)
67+
or
68+
not knownOpenSSLConstantToBlockModeFamilyType(this, _) and result = Crypto::OtherMode()
69+
}
70+
71+
// NOTE: I'm not going to attempt to parse out the mode specific part, so returning
72+
// the same as the raw name for now.
73+
override string getRawModeAlgorithmName() { result = this.(Literal).getValue().toString() }
74+
75+
override OpenSSLAlgorithmValueConsumer getAVC() { result = getterCall }
76+
}

0 commit comments

Comments
 (0)