Skip to content

Commit c09b42a

Browse files
committed
Crypto: Adding initial hash tests, and updating how hash alg consumers are tied to the operation to address a bug in the design, propagated the analgous change to ciphers.
1 parent f645fea commit c09b42a

File tree

6 files changed

+26
-8
lines changed

6 files changed

+26
-8
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ private module AlgGetterToAlgConsumerConfig implements DataFlow::ConfigSig {
1010
}
1111

1212
predicate isSink(DataFlow::Node sink) {
13-
exists(EVP_Cipher_Operation c | c.getInitCall().getAlgorithmArg() = sink.asExpr())
13+
exists(EVP_Cipher_Operation c | c.getAlgorithmArg() = sink.asExpr())
1414
}
1515
}
1616

@@ -32,6 +32,8 @@ private module AlgGetterToAlgConsumerFlow = DataFlow::Global<AlgGetterToAlgConsu
3232
abstract class EVP_Cipher_Operation extends OpenSSLOperation, Crypto::KeyOperationInstance {
3333
Expr getContextArg() { result = this.(Call).getArgument(0) }
3434

35+
Expr getAlgorithmArg() { this.getInitCall().getAlgorithmArg() = result }
36+
3537
override Expr getOutputArg() { result = this.(Call).getArgument(1) }
3638

3739
override Crypto::KeyOperationSubtype getKeyOperationSubtype() {

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgor
1212
abstract class EVP_Hash_Operation extends OpenSSLOperation, Crypto::HashOperationInstance {
1313
Expr getContextArg() { result = this.(Call).getArgument(0) }
1414

15+
Expr getAlgorithmArg() { result = this.getInitCall().getAlgorithmArg() }
16+
1517
EVP_Hash_Initializer getInitCall() {
1618
CTXFlow::ctxArgFlowsToCtxArg(result.getContextArg(), this.getContextArg())
1719
}
@@ -23,7 +25,7 @@ abstract class EVP_Hash_Operation extends OpenSSLOperation, Crypto::HashOperatio
2325
*/
2426
override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
2527
AlgGetterToAlgConsumerFlow::flow(result.(OpenSSLAlgorithmValueConsumer).getResultNode(),
26-
DataFlow::exprNode(this.getInitCall().getAlgorithmArg()))
28+
DataFlow::exprNode(this.getAlgorithmArg()))
2729
}
2830
}
2931

@@ -33,7 +35,7 @@ private module AlgGetterToAlgConsumerConfig implements DataFlow::ConfigSig {
3335
}
3436

3537
predicate isSink(DataFlow::Node sink) {
36-
exists(EVP_Hash_Operation c | c.getInitCall().getAlgorithmArg() = sink.asExpr())
38+
exists(EVP_Hash_Operation c | c.getAlgorithmArg() = sink.asExpr())
3739
}
3840
}
3941

@@ -64,6 +66,8 @@ class EVP_Q_Digest_Operation extends EVP_Hash_Operation {
6466
// simply return 'this', see modeled hash algorithm consuers for EVP_Q_Digest
6567
this = result
6668
}
69+
70+
override Expr getAlgorithmArg() { result = this.(Call).getArgument(1) }
6771
}
6872

6973
class EVP_Digest_Operation extends EVP_Hash_Operation {
@@ -72,17 +76,14 @@ class EVP_Digest_Operation extends EVP_Hash_Operation {
7276
// There is no context argument for this function
7377
override Expr getContextArg() { none() }
7478

75-
override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() {
76-
AlgGetterToAlgConsumerFlow::flow(result.(OpenSSLAlgorithmValueConsumer).getResultNode(),
77-
DataFlow::exprNode(this.(Call).getArgument(4)))
78-
}
79-
8079
override EVP_Hash_Initializer getInitCall() {
8180
// This variant of digest does not use an init
8281
// and even if it were used, the init would be ignored/undefined
8382
none()
8483
}
8584

85+
override Expr getAlgorithmArg() { result = this.(Call).getArgument(4) }
86+
8687
override Expr getOutputArg() { result = this.(Call).getArgument(2) }
8788

8889
override Expr getInputArg() { result = this.(Call).getArgument(0) }
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| openssl_basic.c:124:13:124:30 | HashOperation | openssl_basic.c:120:37:120:43 | Message | openssl_basic.c:181:49:181:87 | Constant |
2+
| openssl_basic.c:144:13:144:22 | HashOperation | openssl_basic.c:144:24:144:30 | Message | openssl_basic.c:181:49:181:87 | Constant |
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import cpp
2+
import experimental.quantum.Language
3+
4+
from Crypto::HashOperationNode n, Crypto::MessageArtifactNode m
5+
where n.getInputArtifact() = m
6+
select n, m, m.getSourceNode()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| openssl_basic.c:124:13:124:30 | HashOperation | openssl_basic.c:124:13:124:30 | Digest | openssl_basic.c:116:38:116:47 | HashAlgorithm | openssl_basic.c:120:37:120:43 | Message |
2+
| openssl_basic.c:144:13:144:22 | HashOperation | openssl_basic.c:144:13:144:22 | Digest | openssl_basic.c:144:67:144:73 | HashAlgorithm | openssl_basic.c:144:24:144:30 | Message |
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import cpp
2+
import experimental.quantum.Language
3+
4+
from Crypto::HashOperationNode n
5+
select n, n.getDigest(), n.getAnAlgorithmOrGenericSource(), n.getInputArtifact()

0 commit comments

Comments
 (0)