Skip to content

Commit 41f008d

Browse files
committed
Crypto: Adding initial openssl tests, fixing a bug in hash modeling found through tests, and updating CODEOWNERS for quantum tests
1 parent 007683f commit 41f008d

20 files changed

+9028
-10
lines changed

CODEOWNERS

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
/java/ql/test-kotlin2/ @github/codeql-kotlin
1717

1818
# Experimental CodeQL cryptography
19-
**/experimental/quantum/ @github/ps-codeql
19+
**/experimental/**/quantum/ @github/ps-codeql
2020
/shared/quantum/ @github/ps-codeql
2121

2222
# CodeQL tools and associated docs

cpp/ql/lib/experimental/quantum/OpenSSL/CtxFlow.qll

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,19 @@ import semmle.code.cpp.dataflow.new.DataFlow
2929
* - EVP_PKEY_CTX
3030
*/
3131
private class CtxType extends Type {
32-
CtxType() { this.getUnspecifiedType().stripType().getName().matches("evp_%ctx_%st") }
32+
CtxType() {
33+
// It is possible for users to use the underlying type of the CTX variables
34+
// these have a name matching 'evp_%ctx_%st
35+
this.getUnspecifiedType().stripType().getName().matches("evp_%ctx_%st")
36+
or
37+
// In principal the above check should be sufficient, but in case of build mode none issues
38+
// i.e., if a typedef cannot be resolved,
39+
// or issues with properly stubbing test cases, we also explicitly check for the wrapping type defs
40+
// i.e., patterns matching 'EVP_%_CTX'
41+
exists(Type base | base = this or base = this.(DerivedType).getBaseType() |
42+
base.getName().matches("EVP_%_CTX")
43+
)
44+
}
3345
}
3446

3547
/**

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:40:13:40:31 | EncryptOperation | openssl_basic.c:31:49:31:51 | Key | openssl_basic.c:179:43:179:76 | Constant |
2+
| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:77:45:77:47 | Key | openssl_basic.c:179:43:179:76 | 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::CipherOperationNode op, Crypto::KeyArtifactNode k
5+
where op.getAKey() = k
6+
select op, k, k.getSourceNode()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:31:54:31:55 | Nonce | openssl_basic.c:180:42:180:59 | Constant |
2+
| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:77:50:77:51 | Nonce | openssl_basic.c:180:42:180:59 | 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::CipherOperationNode op, Crypto::NonceArtifactNode n
5+
where op.getANonce() = n
6+
select op, n, n.getSourceNode()
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:13:40:31 | KeyOperationOutput | openssl_basic.c:23:62:23:65 | Key | openssl_basic.c:23:68:23:71 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt |
2+
| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:13:40:31 | KeyOperationOutput | openssl_basic.c:23:62:23:65 | Key | openssl_basic.c:31:54:31:55 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt |
3+
| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:13:40:31 | KeyOperationOutput | openssl_basic.c:31:49:31:51 | Key | openssl_basic.c:23:68:23:71 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt |
4+
| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | Message | openssl_basic.c:40:13:40:31 | KeyOperationOutput | openssl_basic.c:31:49:31:51 | Key | openssl_basic.c:31:54:31:55 | Nonce | openssl_basic.c:23:37:23:51 | KeyOperationAlgorithm | Encrypt |
5+
| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:11:90:29 | KeyOperationOutput | openssl_basic.c:69:58:69:61 | Key | openssl_basic.c:69:64:69:67 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt |
6+
| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:11:90:29 | KeyOperationOutput | openssl_basic.c:69:58:69:61 | Key | openssl_basic.c:77:50:77:51 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt |
7+
| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:11:90:29 | KeyOperationOutput | openssl_basic.c:77:45:77:47 | Key | openssl_basic.c:69:64:69:67 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt |
8+
| openssl_basic.c:90:11:90:29 | DecryptOperation | openssl_basic.c:81:49:81:58 | Message | openssl_basic.c:90:11:90:29 | KeyOperationOutput | openssl_basic.c:77:45:77:47 | Key | openssl_basic.c:77:50:77:51 | Nonce | openssl_basic.c:69:33:69:47 | KeyOperationAlgorithm | Decrypt |
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::CipherOperationNode n
5+
select n, n.getAnInputArtifact(), n.getAnOutputArtifact(), n.getAKey(), n.getANonce(),
6+
n.getAnAlgorithmOrGenericSource(), n.getKeyOperationSubtype()
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
| openssl_basic.c:40:13:40:31 | EncryptOperation | openssl_basic.c:35:54:35:62 | 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::CipherOperationNode n, Crypto::MessageArtifactNode m
5+
where n.getAnInputArtifact() = 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: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)