Skip to content

Commit 9f0c62b

Browse files
committed
Crypto: Address PR comments.
1 parent 122a004 commit 9f0c62b

File tree

2 files changed

+35
-37
lines changed

2 files changed

+35
-37
lines changed

cpp/ql/lib/experimental/quantum/OpenSSL/Operations/CipherOperation.qll

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ abstract class EvpCipherInitializer extends OperationStep {
1212
or
1313
result.asExpr() = this.getArgument(1) and
1414
type = PrimaryAlgorithmIO() and
15-
// Null for the algorithm indicates the algorithm is not actually set
16-
// This pattern can occur during a multi-step initialization
17-
// TODO/Note: not flowing 0 to the sink, assuming a direct use of NULL for now
15+
// Constants that are not equal to zero or
16+
// non-constants (e.g., variable accesses, which require data-flow to determine the value)
17+
// A zero (null) value typically indicates use of this operation step to initialize
18+
// other out parameters in a multi-step initialization.
1819
(exists(result.asExpr().getValue()) implies result.asExpr().getValue().toInt() != 0)
1920
}
2021

@@ -33,9 +34,10 @@ abstract class EvpEXInitializer extends EvpCipherInitializer {
3334
result = super.getInput(type)
3435
or
3536
(
36-
// Null key or nonce indicates the key/nonce is not actually set
37-
// This pattern can occur during a multi-step initialization
38-
// TODO/Note: not flowing 0 to the sink, assuming a direct use of NULL for now
37+
// Constants that are not equal to zero or
38+
// non-constants (e.g., variable accesses, which require data-flow to determine the value)
39+
// A zero (null) value typically indicates use of this operation step to initialize
40+
// other out parameters in a multi-step initialization.
3941
result.asExpr() = this.getArgument(3) and type = KeyIO()
4042
or
4143
result.asExpr() = this.getArgument(4) and type = IVorNonceIO()

cpp/ql/lib/experimental/quantum/OpenSSL/Operations/HashOperation.qll

Lines changed: 27 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,19 @@ private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgor
1212
* - `EVP_DigestInit_ex`
1313
* - `EVP_DigestInit_ex2`
1414
*/
15-
class EvpDigestInitVariantCalls extends OperationStep {
15+
class EvpDigestInitVariantCalls extends OperationStep instanceof Call {
1616
EvpDigestInitVariantCalls() {
17-
this.(Call).getTarget().getName() in [
18-
"EVP_DigestInit", "EVP_DigestInit_ex", "EVP_DigestInit_ex2"
19-
]
17+
this.getTarget().getName() in ["EVP_DigestInit", "EVP_DigestInit_ex", "EVP_DigestInit_ex2"]
2018
}
2119

2220
override DataFlow::Node getInput(IOType type) {
23-
result.asExpr() = this.(Call).getArgument(0) and type = ContextIO()
21+
result.asExpr() = this.getArgument(0) and type = ContextIO()
2422
or
25-
result.asExpr() = this.(Call).getArgument(1) and type = PrimaryAlgorithmIO()
23+
result.asExpr() = this.getArgument(1) and type = PrimaryAlgorithmIO()
2624
}
2725

2826
override DataFlow::Node getOutput(IOType type) {
29-
result.asExpr() = this.(Call).getArgument(0) and
27+
result.asExpr() = this.getArgument(0) and
3028
type = ContextIO()
3129
}
3230

@@ -36,17 +34,17 @@ class EvpDigestInitVariantCalls extends OperationStep {
3634
/**
3735
* A call to `EVP_DigestUpdate`.
3836
*/
39-
class EvpDigestUpdateCall extends OperationStep {
40-
EvpDigestUpdateCall() { this.(Call).getTarget().getName() = "EVP_DigestUpdate" }
37+
class EvpDigestUpdateCall extends OperationStep instanceof Call {
38+
EvpDigestUpdateCall() { this.getTarget().getName() = "EVP_DigestUpdate" }
4139

4240
override DataFlow::Node getInput(IOType type) {
43-
result.asExpr() = this.(Call).getArgument(0) and type = ContextIO()
41+
result.asExpr() = this.getArgument(0) and type = ContextIO()
4442
or
45-
result.asExpr() = this.(Call).getArgument(1) and type = PlaintextIO()
43+
result.asExpr() = this.getArgument(1) and type = PlaintextIO()
4644
}
4745

4846
override DataFlow::Node getOutput(IOType type) {
49-
result.asExpr() = this.(Call).getArgument(0) and
47+
result.asExpr() = this.getArgument(0) and
5048
type = ContextIO()
5149
}
5250

@@ -64,58 +62,56 @@ abstract class EvpFinalDigestOperationStep extends OperationStep {
6462
* A call to `EVP_Q_digest`
6563
* https://docs.openssl.org/3.0/man3/EVP_DigestInit/#synopsis
6664
*/
67-
class EvpQDigestOperation extends EvpFinalDigestOperationStep {
68-
EvpQDigestOperation() { this.(Call).getTarget().getName() = "EVP_Q_digest" }
65+
class EvpQDigestOperation extends EvpFinalDigestOperationStep instanceof Call {
66+
EvpQDigestOperation() { this.getTarget().getName() = "EVP_Q_digest" }
6967

7068
override DataFlow::Node getInput(IOType type) {
71-
result.asExpr() = this.(Call).getArgument(1) and type = PrimaryAlgorithmIO()
69+
result.asExpr() = this.getArgument(1) and type = PrimaryAlgorithmIO()
7270
or
73-
result.asExpr() = this.(Call).getArgument(0) and type = ContextIO()
71+
result.asExpr() = this.getArgument(0) and type = ContextIO()
7472
or
75-
result.asExpr() = this.(Call).getArgument(3) and type = PlaintextIO()
73+
result.asExpr() = this.getArgument(3) and type = PlaintextIO()
7674
}
7775

7876
override DataFlow::Node getOutput(IOType type) {
79-
result.asExpr() = this.(Call).getArgument(0) and
77+
result.asExpr() = this.getArgument(0) and
8078
type = ContextIO()
8179
or
82-
result.asDefiningArgument() = this.(Call).getArgument(5) and type = DigestIO()
80+
result.asDefiningArgument() = this.getArgument(5) and type = DigestIO()
8381
}
8482
}
8583

86-
class EvpDigestOperation extends EvpFinalDigestOperationStep {
87-
EvpDigestOperation() { this.(Call).getTarget().getName() = "EVP_Digest" }
84+
class EvpDigestOperation extends EvpFinalDigestOperationStep instanceof Call {
85+
EvpDigestOperation() { this.getTarget().getName() = "EVP_Digest" }
8886

8987
override DataFlow::Node getInput(IOType type) {
90-
result.asExpr() = this.(Call).getArgument(4) and type = PrimaryAlgorithmIO()
88+
result.asExpr() = this.getArgument(4) and type = PrimaryAlgorithmIO()
9189
or
92-
result.asExpr() = this.(Call).getArgument(0) and type = PlaintextIO()
90+
result.asExpr() = this.getArgument(0) and type = PlaintextIO()
9391
}
9492

9593
override DataFlow::Node getOutput(IOType type) {
96-
result.asDefiningArgument() = this.(Call).getArgument(2) and type = DigestIO()
94+
result.asDefiningArgument() = this.getArgument(2) and type = DigestIO()
9795
}
9896
}
9997

10098
/**
10199
* A call to EVP_DigestFinal variants
102100
*/
103-
class EvpDigestFinalCall extends EvpFinalDigestOperationStep {
101+
class EvpDigestFinalCall extends EvpFinalDigestOperationStep instanceof Call {
104102
EvpDigestFinalCall() {
105-
this.(Call).getTarget().getName() in [
106-
"EVP_DigestFinal", "EVP_DigestFinal_ex", "EVP_DigestFinalXOF"
107-
]
103+
this.getTarget().getName() in ["EVP_DigestFinal", "EVP_DigestFinal_ex", "EVP_DigestFinalXOF"]
108104
}
109105

110106
override DataFlow::Node getInput(IOType type) {
111-
result.asExpr() = this.(Call).getArgument(0) and type = ContextIO()
107+
result.asExpr() = this.getArgument(0) and type = ContextIO()
112108
}
113109

114110
override DataFlow::Node getOutput(IOType type) {
115-
result.asExpr() = this.(Call).getArgument(0) and
111+
result.asExpr() = this.getArgument(0) and
116112
type = ContextIO()
117113
or
118-
result.asDefiningArgument() = this.(Call).getArgument(1) and type = DigestIO()
114+
result.asDefiningArgument() = this.getArgument(1) and type = DigestIO()
119115
}
120116
}
121117

0 commit comments

Comments
 (0)