Skip to content

Commit a241c11

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#5836 from RasmusWL/ec-class-improvement
Approved by tausbn
2 parents 716627c + d50f225 commit a241c11

File tree

6 files changed

+58
-6
lines changed

6 files changed

+58
-6
lines changed

‎python/ql/src/semmle/python/frameworks/Cryptography.qll

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ private module CryptographyModel {
2222
* Gets a predefined curve class from
2323
* `cryptography.hazmat.primitives.asymmetric.ec` with a specific key size (in bits).
2424
*/
25-
private DataFlow::Node curveClassWithKeySize(int keySize) {
25+
private API::Node predefinedCurveClass(int keySize) {
2626
exists(string curveName |
2727
result =
2828
API::moduleImport("cryptography")
@@ -31,7 +31,6 @@ private module CryptographyModel {
3131
.getMember("asymmetric")
3232
.getMember("ec")
3333
.getMember(curveName)
34-
.getAUse()
3534
|
3635
// obtained by manually looking at source code in
3736
// https://github.com/pyca/cryptography/blob/cba69f1922803f4f29a3fde01741890d88b8e217/src/cryptography/hazmat/primitives/asymmetric/ec.py#L208-L300
@@ -75,13 +74,30 @@ private module CryptographyModel {
7574
)
7675
}
7776

77+
/** Gets a reference to a predefined curve class with a specific key size (in bits), as well as the origin of the class. */
78+
private DataFlow::LocalSourceNode curveClassWithKeySize(
79+
DataFlow::TypeTracker t, int keySize, DataFlow::Node origin
80+
) {
81+
t.start() and
82+
result = predefinedCurveClass(keySize).getAnImmediateUse() and
83+
origin = result
84+
or
85+
exists(DataFlow::TypeTracker t2 |
86+
result = curveClassWithKeySize(t2, keySize, origin).track(t2, t)
87+
)
88+
}
89+
90+
/** Gets a reference to a predefined curve class with a specific key size (in bits), as well as the origin of the class. */
91+
DataFlow::Node curveClassWithKeySize(int keySize, DataFlow::Node origin) {
92+
curveClassWithKeySize(DataFlow::TypeTracker::end(), keySize, origin).flowsTo(result)
93+
}
94+
7895
/** Gets a reference to a predefined curve class instance with a specific key size (in bits), as well as the origin of the class. */
7996
private DataFlow::LocalSourceNode curveClassInstanceWithKeySize(
8097
DataFlow::TypeTracker t, int keySize, DataFlow::Node origin
8198
) {
8299
t.start() and
83-
result.(DataFlow::CallCfgNode).getFunction() = curveClassWithKeySize(keySize) and
84-
origin = result
100+
result.(DataFlow::CallCfgNode).getFunction() = curveClassWithKeySize(keySize, origin)
85101
or
86102
exists(DataFlow::TypeTracker t2 |
87103
result = curveClassInstanceWithKeySize(t2, keySize, origin).track(t2, t)
@@ -164,6 +180,8 @@ private module CryptographyModel {
164180

165181
override int getKeySizeWithOrigin(DataFlow::Node origin) {
166182
this.getCurveArg() = Ecc::curveClassInstanceWithKeySize(result, origin)
183+
or
184+
this.getCurveArg() = Ecc::curveClassWithKeySize(result, origin)
167185
}
168186

169187
// Note: There is not really a key-size argument, since it's always specified by the curve.

‎python/ql/test/library-tests/frameworks/cryptography/EcKeygenOrigin.expected

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
| ec_keygen_origin.py:8:1:8:45 | ControlFlowNode for Attribute() | 384 | ec_keygen_origin.py:8:31:8:42 | ControlFlowNode for Attribute |
2+
| ec_keygen_origin.py:9:1:9:43 | ControlFlowNode for Attribute() | 384 | ec_keygen_origin.py:9:31:9:42 | ControlFlowNode for Attribute |
3+
| ec_keygen_origin.py:12:1:12:36 | ControlFlowNode for Attribute() | 384 | ec_keygen_origin.py:11:9:11:20 | ControlFlowNode for Attribute |
4+
| ec_keygen_origin.py:15:1:15:39 | ControlFlowNode for Attribute() | 384 | ec_keygen_origin.py:11:9:11:20 | ControlFlowNode for Attribute |
5+
| ec_keygen_origin.py:20:1:20:32 | ControlFlowNode for Attribute() | 384 | ec_keygen_origin.py:6:58:6:66 | ControlFlowNode for ImportMember |

‎python/ql/test/library-tests/frameworks/cryptography/EcKeygenOrigin.ql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import semmle.python.dataflow.new.DataFlow
2+
import semmle.python.Concepts
3+
4+
from Cryptography::PublicKey::KeyGeneration keyGen, int keySize, DataFlow::Node origin
5+
where
6+
keyGen.getLocation().getFile().getShortName() = "ec_keygen_origin.py" and
7+
keySize = keyGen.getKeySizeWithOrigin(origin)
8+
select keyGen, keySize, origin

‎python/ql/test/library-tests/frameworks/cryptography/ec_keygen_origin.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Since key-size is not specified explicitly as an integer for the predefined
2+
# classes in the `cryptography.hazmat.primitives.asymmetric.ec` module, we need
3+
# special handling of the origin... this test is simply to show off how we handle this.
4+
5+
from cryptography.hazmat.primitives.asymmetric import ec
6+
from cryptography.hazmat.primitives.asymmetric.ec import SECP384R1
7+
8+
ec.generate_private_key(curve=ec.SECP384R1()) # $ PublicKeyGeneration keySize=384
9+
ec.generate_private_key(curve=ec.SECP384R1) # $ PublicKeyGeneration keySize=384
10+
11+
alias = ec.SECP384R1
12+
ec.generate_private_key(curve=alias) # $ PublicKeyGeneration keySize=384
13+
14+
instance = alias()
15+
ec.generate_private_key(curve=instance) # $ PublicKeyGeneration keySize=384
16+
17+
18+
x = SECP384R1
19+
y = x
20+
ec.generate_private_key(curve=y) # $ PublicKeyGeneration keySize=384

‎python/ql/test/library-tests/frameworks/cryptography/test_ec.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77

88
private_key = ec.generate_private_key(curve=ec.SECP384R1()) # $ PublicKeyGeneration keySize=384
9+
private_key = ec.generate_private_key(curve=ec.SECP384R1) # $ PublicKeyGeneration keySize=384
910
public_key = private_key.public_key()
1011

1112
HASH_ALGORITHM = hashes.SHA256()

‎python/ql/test/query-tests/Security/CWE-326/WeakCryptoKey.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
| weak_crypto.py:68:1:68:21 | ControlFlowNode for dsa_gen_key() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:16:12:16:15 | ControlFlowNode for IntegerLiteral | 1024 |
2-
| weak_crypto.py:69:1:69:19 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 224 and considered breakable. | weak_crypto.py:22:11:22:24 | ControlFlowNode for Attribute() | 163 |
2+
| weak_crypto.py:69:1:69:19 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 224 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 163 |
33
| weak_crypto.py:70:1:70:28 | ControlFlowNode for rsa_gen_key() | Creation of an RSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 |
44
| weak_crypto.py:72:1:72:30 | ControlFlowNode for dsa_gen_key() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:16:12:16:15 | ControlFlowNode for IntegerLiteral | 1024 |
5-
| weak_crypto.py:73:1:73:25 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 224 and considered breakable. | weak_crypto.py:22:11:22:24 | ControlFlowNode for Attribute() | 163 |
5+
| weak_crypto.py:73:1:73:25 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 224 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 163 |
66
| weak_crypto.py:74:1:74:37 | ControlFlowNode for rsa_gen_key() | Creation of an RSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 |
77
| weak_crypto.py:76:1:76:22 | ControlFlowNode for Attribute() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:16:12:16:15 | ControlFlowNode for IntegerLiteral | 1024 |
88
| weak_crypto.py:77:1:77:22 | ControlFlowNode for Attribute() | Creation of an RSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 |

0 commit comments

Comments
 (0)