Skip to content

Commit 746e994

Browse files
yoffyoff
authored
Merge pull request github#5075 from RasmusWL/crypto
Python: Port py/weak-crypto-key to use type-tracking
2 parents f4dc5b9 + 7b92012 commit 746e994

32 files changed

+999
-46
lines changed

‎docs/codeql/support/reusables/frameworks.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,3 +147,6 @@ Python built-in support
147147
MySQLdb, Database
148148
psycopg2, Database
149149
sqlite3, Database
150+
cryptography, Cryptography library
151+
pycryptodome, Cryptography library
152+
pycryptodomex, Cryptography library

‎python/change-notes/2021-02-02-port-weak-crypto-key-query.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
lgtm,codescanning
2+
* Updated _Use of weak cryptographic key_ (`py/weak-crypto-key`) query to use the new type-tracking approach instead of points-to analysis. You may see differences in the results found by the query, but overall this change should result in a more robust and accurate analysis.
3+
* Renamed the query file for _Use of weak cryptographic key_ (`py/weak-crypto-key`) from `WeakCrypto.ql` to `WeakCryptoKey.ql` (in the `python/ql/src/Security/CWE-326/` folder). This will affect any custom query suites that include or exclude this query using its path.

‎python/ql/src/Security/CWE-326/WeakCrypto.qhelp renamed to ‎python/ql/src/Security/CWE-326/WeakCryptoKey.qhelp

File renamed without changes.

‎python/ql/src/Security/CWE-326/WeakCryptoKey.ql

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @name Use of weak cryptographic key
3+
* @description Use of a cryptographic key that is too small may allow the encryption to be broken.
4+
* @kind problem
5+
* @problem.severity error
6+
* @precision high
7+
* @id py/weak-crypto-key
8+
* @tags security
9+
* external/cwe/cwe-326
10+
*/
11+
12+
import python
13+
import semmle.python.Concepts
14+
import semmle.python.dataflow.new.DataFlow
15+
import semmle.python.filters.Tests
16+
17+
from Cryptography::PublicKey::KeyGeneration keyGen, int keySize, DataFlow::Node origin
18+
where
19+
keySize = keyGen.getKeySizeWithOrigin(origin) and
20+
keySize < keyGen.minimumSecureKeySize() and
21+
not origin.getScope().getScope*() instanceof TestScope
22+
select keyGen,
23+
"Creation of an " + keyGen.getName() + " key uses $@ bits, which is below " +
24+
keyGen.minimumSecureKeySize() + " and considered breakable.", origin, keySize.toString()

‎python/ql/src/Security/CWE-326/WeakCrypto.ql renamed to ‎python/ql/src/experimental/Security-old-dataflow/CWE-326/WeakCrypto.ql

File renamed without changes.

‎python/ql/src/semmle/python/Concepts.qll

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,3 +526,116 @@ module HTTP {
526526
}
527527
}
528528
}
529+
530+
/** Provides models for cryptographic things. */
531+
module Cryptography {
532+
/** Provides models for public-key cryptography, also called asymmetric cryptography. */
533+
module PublicKey {
534+
/**
535+
* A data-flow node that generates a new key-pair for use with public-key cryptography.
536+
*
537+
* Extend this class to refine existing API models. If you want to model new APIs,
538+
* extend `KeyGeneration::Range` instead.
539+
*/
540+
class KeyGeneration extends DataFlow::Node {
541+
KeyGeneration::Range range;
542+
543+
KeyGeneration() { this = range }
544+
545+
/** Gets the name of the cryptographic algorithm (for example `"RSA"` or `"AES"`). */
546+
string getName() { result = range.getName() }
547+
548+
/** Gets the argument that specifies the size of the key in bits, if available. */
549+
DataFlow::Node getKeySizeArg() { result = range.getKeySizeArg() }
550+
551+
/**
552+
* Gets the size of the key generated (in bits), as well as the `origin` that
553+
* explains how we obtained this specific key size.
554+
*/
555+
int getKeySizeWithOrigin(DataFlow::Node origin) {
556+
result = range.getKeySizeWithOrigin(origin)
557+
}
558+
559+
/** Gets the minimum key size (in bits) for this algorithm to be considered secure. */
560+
int minimumSecureKeySize() { result = range.minimumSecureKeySize() }
561+
}
562+
563+
/** Provides classes for modeling new key-pair generation APIs. */
564+
module KeyGeneration {
565+
/** Gets a back-reference to the keysize argument `arg` that was used to generate a new key-pair. */
566+
DataFlow::LocalSourceNode keysizeBacktracker(DataFlow::TypeBackTracker t, DataFlow::Node arg) {
567+
t.start() and
568+
arg = any(KeyGeneration::Range r).getKeySizeArg() and
569+
result = arg.getALocalSource()
570+
or
571+
// Due to bad performance when using normal setup with we have inlined that code and forced a join
572+
exists(DataFlow::TypeBackTracker t2 |
573+
exists(DataFlow::StepSummary summary |
574+
keysizeBacktracker_first_join(t2, arg, result, summary) and
575+
t = t2.prepend(summary)
576+
)
577+
)
578+
}
579+
580+
pragma[nomagic]
581+
private predicate keysizeBacktracker_first_join(
582+
DataFlow::TypeBackTracker t2, DataFlow::Node arg, DataFlow::Node res,
583+
DataFlow::StepSummary summary
584+
) {
585+
DataFlow::StepSummary::step(res, keysizeBacktracker(t2, arg), summary)
586+
}
587+
588+
/** Gets a back-reference to the keysize argument `arg` that was used to generate a new key-pair. */
589+
DataFlow::LocalSourceNode keysizeBacktracker(DataFlow::Node arg) {
590+
result = keysizeBacktracker(DataFlow::TypeBackTracker::end(), arg)
591+
}
592+
593+
/**
594+
* A data-flow node that generates a new key-pair for use with public-key cryptography.
595+
*
596+
* Extend this class to model new APIs. If you want to refine existing API models,
597+
* extend `KeyGeneration` instead.
598+
*/
599+
abstract class Range extends DataFlow::Node {
600+
/** Gets the name of the cryptographic algorithm (for example `"RSA"`). */
601+
abstract string getName();
602+
603+
/** Gets the argument that specifies the size of the key in bits, if available. */
604+
abstract DataFlow::Node getKeySizeArg();
605+
606+
/**
607+
* Gets the size of the key generated (in bits), as well as the `origin` that
608+
* explains how we obtained this specific key size.
609+
*/
610+
int getKeySizeWithOrigin(DataFlow::Node origin) {
611+
origin = keysizeBacktracker(this.getKeySizeArg()) and
612+
result = origin.asExpr().(IntegerLiteral).getValue()
613+
}
614+
615+
/** Gets the minimum key size (in bits) for this algorithm to be considered secure. */
616+
abstract int minimumSecureKeySize();
617+
}
618+
619+
/** A data-flow node that generates a new RSA key-pair. */
620+
abstract class RsaRange extends Range {
621+
final override string getName() { result = "RSA" }
622+
623+
final override int minimumSecureKeySize() { result = 2048 }
624+
}
625+
626+
/** A data-flow node that generates a new DSA key-pair. */
627+
abstract class DsaRange extends Range {
628+
final override string getName() { result = "DSA" }
629+
630+
final override int minimumSecureKeySize() { result = 2048 }
631+
}
632+
633+
/** A data-flow node that generates a new ECC key-pair. */
634+
abstract class EccRange extends Range {
635+
final override string getName() { result = "ECC" }
636+
637+
final override int minimumSecureKeySize() { result = 224 }
638+
}
639+
}
640+
}
641+
}

‎python/ql/src/semmle/python/Frameworks.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
* Helper file that imports all framework modeling.
33
*/
44

5+
private import semmle.python.frameworks.Cryptodome
6+
private import semmle.python.frameworks.Cryptography
57
private import semmle.python.frameworks.Dill
68
private import semmle.python.frameworks.Django
79
private import semmle.python.frameworks.Fabric

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

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/**
2+
* Provides classes modeling security-relevant aspects of
3+
* - the `pycryptodome` PyPI package (imported as `Crypto`)
4+
* - the `pycryptodomex` PyPI package (imported as `Cryptodome`)
5+
* See https://pycryptodome.readthedocs.io/en/latest/.
6+
*/
7+
8+
private import python
9+
private import semmle.python.dataflow.new.DataFlow
10+
private import semmle.python.Concepts
11+
private import semmle.python.ApiGraphs
12+
13+
/**
14+
* Provides models for
15+
* - the `pycryptodome` PyPI package (imported as `Crypto`)
16+
* - the `pycryptodomex` PyPI package (imported as `Cryptodome`)
17+
* See https://pycryptodome.readthedocs.io/en/latest/
18+
*/
19+
private module CryptodomeModel {
20+
// ---------------------------------------------------------------------------
21+
/**
22+
* A call to `Cryptodome.PublicKey.RSA.generate`/`Crypto.PublicKey.RSA.generate`
23+
*
24+
* See https://pycryptodome.readthedocs.io/en/latest/src/public_key/rsa.html#Crypto.PublicKey.RSA.generate
25+
*/
26+
class CryptodomePublicKeyRsaGenerateCall extends Cryptography::PublicKey::KeyGeneration::RsaRange,
27+
DataFlow::CallCfgNode {
28+
CryptodomePublicKeyRsaGenerateCall() {
29+
this =
30+
API::moduleImport(["Crypto", "Cryptodome"])
31+
.getMember("PublicKey")
32+
.getMember("RSA")
33+
.getMember("generate")
34+
.getACall()
35+
}
36+
37+
override DataFlow::Node getKeySizeArg() {
38+
result in [this.getArg(0), this.getArgByName("bits")]
39+
}
40+
}
41+
42+
/**
43+
* A call to `Cryptodome.PublicKey.DSA.generate`/`Crypto.PublicKey.DSA.generate`
44+
*
45+
* See https://pycryptodome.readthedocs.io/en/latest/src/public_key/dsa.html#Crypto.PublicKey.DSA.generate
46+
*/
47+
class CryptodomePublicKeyDsaGenerateCall extends Cryptography::PublicKey::KeyGeneration::DsaRange,
48+
DataFlow::CallCfgNode {
49+
CryptodomePublicKeyDsaGenerateCall() {
50+
this =
51+
API::moduleImport(["Crypto", "Cryptodome"])
52+
.getMember("PublicKey")
53+
.getMember("DSA")
54+
.getMember("generate")
55+
.getACall()
56+
}
57+
58+
override DataFlow::Node getKeySizeArg() {
59+
result in [this.getArg(0), this.getArgByName("bits")]
60+
}
61+
}
62+
63+
/**
64+
* A call to `Cryptodome.PublicKey.ECC.generate`/`Crypto.PublicKey.ECC.generate`
65+
*
66+
* See https://pycryptodome.readthedocs.io/en/latest/src/public_key/ecc.html#Crypto.PublicKey.ECC.generate
67+
*/
68+
class CryptodomePublicKeyEccGenerateCall extends Cryptography::PublicKey::KeyGeneration::EccRange,
69+
DataFlow::CallCfgNode {
70+
CryptodomePublicKeyEccGenerateCall() {
71+
this =
72+
API::moduleImport(["Crypto", "Cryptodome"])
73+
.getMember("PublicKey")
74+
.getMember("ECC")
75+
.getMember("generate")
76+
.getACall()
77+
}
78+
79+
/** Gets the argument that specifies the curve to use (a string). */
80+
DataFlow::Node getCurveArg() { result = this.getArgByName("curve") }
81+
82+
/** Gets the name of the curve to use, as well as the origin that explains how we obtained this name. */
83+
string getCurveWithOrigin(DataFlow::Node origin) {
84+
exists(StrConst str | origin = DataFlow::exprNode(str) |
85+
origin = this.getCurveArg().getALocalSource() and
86+
result = str.getText()
87+
)
88+
}
89+
90+
override int getKeySizeWithOrigin(DataFlow::Node origin) {
91+
exists(string curve | curve = this.getCurveWithOrigin(origin) |
92+
// using list from https://pycryptodome.readthedocs.io/en/latest/src/public_key/ecc.html
93+
curve in ["NIST P-256", "p256", "P-256", "prime256v1", "secp256r1"] and result = 256
94+
or
95+
curve in ["NIST P-384", "p384", "P-384", "prime384v1", "secp384r1"] and result = 384
96+
or
97+
curve in ["NIST P-521", "p521", "P-521", "prime521v1", "secp521r1"] and result = 521
98+
)
99+
}
100+
101+
// Note: There is not really a key-size argument, since it's always specified by the curve.
102+
override DataFlow::Node getKeySizeArg() { none() }
103+
}
104+
}

0 commit comments

Comments
 (0)