hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-21 11:16:30 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #14603 from github/max-schaefer/broken-crypto-algorithm-link

Browse Source 
JavaScript/Python/Ruby: Improve alert message for `*/weak-cryptographic-algorithm`.
This commit is contained in:
Rasmus Wriedt Larsen
2023-11-08 14:29:24 +01:00
committed by GitHub
parent 4ae35d179e 104700f6d3
commit 43d9d2ceb7
17 changed files with 231 additions and 116 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
python/ql/lib/semmle/python/frameworks/Cryptodome.qll
View File

@@ -128,6 +128,8 @@ private module CryptodomeModel {
this = newCall.getReturn().getMember(methodName).getACall()
}
override DataFlow::Node getInitialization() { result = newCall }
override Cryptography::CryptographicAlgorithm getAlgorithm() { result.matchesName(cipherName) }
override DataFlow::Node getAnInput() {
@@ -181,21 +183,23 @@ private module CryptodomeModel {
class CryptodomeGenericSignatureOperation extends Cryptography::CryptographicOperation::Range,
DataFlow::CallCfgNode
{
API::CallNode newCall;
string methodName;
string signatureName;
CryptodomeGenericSignatureOperation() {
methodName in ["sign", "verify"] and
this =
newCall =
API::moduleImport(["Crypto", "Cryptodome"])
.getMember("Signature")
.getMember(signatureName)
.getMember("new")
.getReturn()
.getMember(methodName)
.getACall()
.getACall() and
this = newCall.getReturn().getMember(methodName).getACall()
}
override DataFlow::Node getInitialization() { result = newCall }
override Cryptography::CryptographicAlgorithm getAlgorithm() {
result.matchesName(signatureName)
}
@@ -221,19 +225,23 @@ private module CryptodomeModel {
class CryptodomeGenericHashOperation extends Cryptography::CryptographicOperation::Range,
DataFlow::CallCfgNode
{
API::CallNode newCall;
string hashName;
CryptodomeGenericHashOperation() {
exists(API::Node hashModule |
hashModule =
API::moduleImport(["Crypto", "Cryptodome"]).getMember("Hash").getMember(hashName)
API::moduleImport(["Crypto", "Cryptodome"]).getMember("Hash").getMember(hashName) and
newCall = hashModule.getMember("new").getACall()
|
this = hashModule.getMember("new").getACall()
this = newCall
or
this = hashModule.getMember("new").getReturn().getMember("update").getACall()
this = newCall.getReturn().getMember("update").getACall()
)
}
override DataFlow::Node getInitialization() { result = newCall }
override Cryptography::CryptographicAlgorithm getAlgorithm() { result.matchesName(hashName) }
override DataFlow::Node getAnInput() { result in [this.getArg(0), this.getArgByName("data")] }

42
python/ql/lib/semmle/python/frameworks/Cryptography.qll
View File

@@ -209,18 +209,18 @@ private module CryptographyModel {
class CryptographyGenericCipherOperation extends Cryptography::CryptographicOperation::Range,
DataFlow::MethodCallNode
{
API::CallNode init;
string algorithmName;
string modeName;
CryptographyGenericCipherOperation() {
this =
cipherInstance(algorithmName, modeName)
.getMember(["decryptor", "encryptor"])
.getReturn()
.getMember(["update", "update_into"])
.getACall()
init =
cipherInstance(algorithmName, modeName).getMember(["decryptor", "encryptor"]).getACall() and
this = init.getReturn().getMember(["update", "update_into"]).getACall()
}
override DataFlow::Node getInitialization() { result = init }
override Cryptography::CryptographicAlgorithm getAlgorithm() {
result.matchesName(algorithmName)
}
@@ -247,19 +247,17 @@ private module CryptographyModel {
}
/** Gets a reference to a Hash instance using algorithm with `algorithmName`. */
private API::Node hashInstance(string algorithmName) {
exists(API::CallNode call | result = call.getReturn() |
call =
API::moduleImport("cryptography")
.getMember("hazmat")
.getMember("primitives")
.getMember("hashes")
.getMember("Hash")
.getACall() and
algorithmClassRef(algorithmName).getReturn().getAValueReachableFromSource() in [
call.getArg(0), call.getArgByName("algorithm")
]
)
private API::CallNode hashInstance(string algorithmName) {
result =
API::moduleImport("cryptography")
.getMember("hazmat")
.getMember("primitives")
.getMember("hashes")
.getMember("Hash")
.getACall() and
algorithmClassRef(algorithmName).getReturn().getAValueReachableFromSource() in [
result.getArg(0), result.getArgByName("algorithm")
]
}
/**
@@ -268,12 +266,16 @@ private module CryptographyModel {
class CryptographyGenericHashOperation extends Cryptography::CryptographicOperation::Range,
DataFlow::MethodCallNode
{
API::CallNode init;
string algorithmName;
CryptographyGenericHashOperation() {
this = hashInstance(algorithmName).getMember("update").getACall()
init = hashInstance(algorithmName) and
this = init.getReturn().getMember("update").getACall()
}
override DataFlow::Node getInitialization() { result = init }
override Cryptography::CryptographicAlgorithm getAlgorithm() {
result.matchesName(algorithmName)
}

12
python/ql/lib/semmle/python/frameworks/Rsa.qll
View File

@@ -37,6 +37,8 @@ private module Rsa {
class RsaEncryptCall extends Cryptography::CryptographicOperation::Range, DataFlow::CallCfgNode {
RsaEncryptCall() { this = API::moduleImport("rsa").getMember("encrypt").getACall() }
override DataFlow::Node getInitialization() { result = this }
override Cryptography::CryptographicAlgorithm getAlgorithm() { result.getName() = "RSA" }
override DataFlow::Node getAnInput() {
@@ -54,6 +56,8 @@ private module Rsa {
class RsaDecryptCall extends Cryptography::CryptographicOperation::Range, DataFlow::CallCfgNode {
RsaDecryptCall() { this = API::moduleImport("rsa").getMember("decrypt").getACall() }
override DataFlow::Node getInitialization() { result = this }
override Cryptography::CryptographicAlgorithm getAlgorithm() { result.getName() = "RSA" }
override DataFlow::Node getAnInput() { result in [this.getArg(0), this.getArgByName("crypto")] }
@@ -69,6 +73,8 @@ private module Rsa {
class RsaSignCall extends Cryptography::CryptographicOperation::Range, DataFlow::CallCfgNode {
RsaSignCall() { this = API::moduleImport("rsa").getMember("sign").getACall() }
override DataFlow::Node getInitialization() { result = this }
override Cryptography::CryptographicAlgorithm getAlgorithm() {
// signature part
result.getName() = "RSA"
@@ -96,6 +102,8 @@ private module Rsa {
class RsaVerifyCall extends Cryptography::CryptographicOperation::Range, DataFlow::CallCfgNode {
RsaVerifyCall() { this = API::moduleImport("rsa").getMember("verify").getACall() }
override DataFlow::Node getInitialization() { result = this }
override Cryptography::CryptographicAlgorithm getAlgorithm() {
// note that technically there is also a hashing operation going on but we don't
// know what algorithm is used up front, since it is encoded in the signature
@@ -121,6 +129,8 @@ private module Rsa {
{
RsaComputeHashCall() { this = API::moduleImport("rsa").getMember("compute_hash").getACall() }
override DataFlow::Node getInitialization() { result = this }
override Cryptography::CryptographicAlgorithm getAlgorithm() {
exists(StrConst str, DataFlow::Node hashNameArg |
hashNameArg in [this.getArg(1), this.getArgByName("method_name")] and
@@ -144,6 +154,8 @@ private module Rsa {
class RsaSignHashCall extends Cryptography::CryptographicOperation::Range, DataFlow::CallCfgNode {
RsaSignHashCall() { this = API::moduleImport("rsa").getMember("sign_hash").getACall() }
override DataFlow::Node getInitialization() { result = this }
override Cryptography::CryptographicAlgorithm getAlgorithm() { result.getName() = "RSA" }
override DataFlow::Node getAnInput() {

29
python/ql/lib/semmle/python/frameworks/Stdlib.qll
View File

@@ -2747,6 +2747,8 @@ private module StdlibPrivate {
exists(this.getParameter(1, "data"))
}
override DataFlow::Node getInitialization() { result = this }
override Cryptography::CryptographicAlgorithm getAlgorithm() { result.matchesName(hashName) }
override DataFlow::Node getAnInput() { result = this.getParameter(1, "data").asSink() }
@@ -2758,12 +2760,16 @@ private module StdlibPrivate {
* A hashing operation by using the `update` method on the result of calling the `hashlib.new` function.
*/
class HashlibNewUpdateCall extends Cryptography::CryptographicOperation::Range, API::CallNode {
API::CallNode init;
string hashName;
HashlibNewUpdateCall() {
this = hashlibNewCall(hashName).getReturn().getMember("update").getACall()
init = hashlibNewCall(hashName) and
this = init.getReturn().getMember("update").getACall()
}
override DataFlow::Node getInitialization() { result = init }
override Cryptography::CryptographicAlgorithm getAlgorithm() { result.matchesName(hashName) }
override DataFlow::Node getAnInput() { result = this.getArg(0) }
@@ -2802,7 +2808,14 @@ private module StdlibPrivate {
* (such as `hashlib.md5`), by calling its' `update` method.
*/
class HashlibHashClassUpdateCall extends HashlibGenericHashOperation {
HashlibHashClassUpdateCall() { this = hashClass.getReturn().getMember("update").getACall() }
API::CallNode init;
HashlibHashClassUpdateCall() {
init = hashClass.getACall() and
this = hashClass.getReturn().getMember("update").getACall()
}
override DataFlow::Node getInitialization() { result = init }
override DataFlow::Node getAnInput() { result = this.getArg(0) }
}
@@ -2819,6 +2832,8 @@ private module StdlibPrivate {
exists([this.getArg(0), this.getArgByName("string")])
}
override DataFlow::Node getInitialization() { result = this }
override DataFlow::Node getAnInput() {
result = this.getArg(0)
or
@@ -2865,6 +2880,8 @@ private module StdlibPrivate {
exists(this.getParameter(1, "msg").asSink())
}
override DataFlow::Node getInitialization() { result = this }
override API::Node getDigestArg() { result = digestArg }
override DataFlow::Node getAnInput() { result = this.getParameter(1, "msg").asSink() }
@@ -2876,12 +2893,16 @@ private module StdlibPrivate {
* See https://docs.python.org/3.11/library/hmac.html#hmac.HMAC.update
*/
class HmacUpdateCall extends HmacCryptographicOperation {
API::CallNode init;
API::Node digestArg;
HmacUpdateCall() {
this = getHmacConstructorCall(digestArg).getReturn().getMember("update").getACall()
init = getHmacConstructorCall(digestArg) and
this = init.getReturn().getMember("update").getACall()
}
override DataFlow::Node getInitialization() { result = init }
override API::Node getDigestArg() { result = digestArg }
override DataFlow::Node getAnInput() { result = this.getParameter(0, "msg").asSink() }
@@ -2895,6 +2916,8 @@ private module StdlibPrivate {
class HmacDigestCall extends HmacCryptographicOperation {
HmacDigestCall() { this = API::moduleImport("hmac").getMember("digest").getACall() }
override DataFlow::Node getInitialization() { result = this }
override API::Node getDigestArg() { result = this.getParameter(2, "digest") }
override DataFlow::Node getAnInput() { result = this.getParameter(1, "msg").asSink() }

6
python/ql/lib/semmle/python/internal/ConceptsShared.qll
View File

@@ -40,6 +40,9 @@ module Cryptography {
/** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
CryptographicAlgorithm getAlgorithm() { result = super.getAlgorithm() }
/** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
DataFlow::Node getInitialization() { result = super.getInitialization() }
/** Gets an input the algorithm is used on, for example the plain text input to be encrypted. */
DataFlow::Node getAnInput() { result = super.getAnInput() }
@@ -65,6 +68,9 @@ module Cryptography {
* extend `CryptographicOperation` instead.
*/
abstract class Range extends DataFlow::Node {
/** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
abstract DataFlow::Node getInitialization();
/** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
abstract CryptographicAlgorithm getAlgorithm();

13
python/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql
View File

@@ -13,18 +13,15 @@
import python
import semmle.python.Concepts
from
Cryptography::CryptographicOperation operation, Cryptography::CryptographicAlgorithm algorithm,
string msgPrefix
from Cryptography::CryptographicOperation operation, string msgPrefix
where
algorithm = operation.getAlgorithm() and
// `Cryptography::HashingAlgorithm` and `Cryptography::PasswordHashingAlgorithm` are
// handled by `py/weak-sensitive-data-hashing`
algorithm instanceof Cryptography::EncryptionAlgorithm and
(
exists(Cryptography::EncryptionAlgorithm algorithm | algorithm = operation.getAlgorithm() |
algorithm.isWeak() and
msgPrefix = "The cryptographic algorithm " + operation.getAlgorithm().getName()
msgPrefix = "The cryptographic algorithm " + algorithm.getName()
)
or
operation.getBlockMode().isWeak() and msgPrefix = "The block mode " + operation.getBlockMode()
select operation, msgPrefix + " is broken or weak, and should not be used."
select operation, "$@ is broken or weak, and should not be used.", operation.getInitialization(),
msgPrefix

8
python/ql/test/query-tests/Security/CWE-327-BrokenCryptoAlgorithm/BrokenCryptoAlgorithm.expected
View File

@@ -1,4 +1,4 @@
| test_cryptodome.py:11:13:11:42 | ControlFlowNode for Attribute() | The cryptographic algorithm ARC4 is broken or weak, and should not be used. |
| test_cryptodome.py:16:13:16:42 | ControlFlowNode for Attribute() | The block mode ECB is broken or weak, and should not be used. |
| test_cryptography.py:13:13:13:44 | ControlFlowNode for Attribute() | The cryptographic algorithm ARC4 is broken or weak, and should not be used. |
| test_cryptography.py:22:13:22:58 | ControlFlowNode for Attribute() | The block mode ECB is broken or weak, and should not be used. |
| test_cryptodome.py:11:13:11:42 | ControlFlowNode for Attribute() | $@ is broken or weak, and should not be used. | test_cryptodome.py:10:10:10:22 | ControlFlowNode for Attribute() | The cryptographic algorithm ARC4 |
| test_cryptodome.py:16:13:16:42 | ControlFlowNode for Attribute() | $@ is broken or weak, and should not be used. | test_cryptodome.py:15:10:15:35 | ControlFlowNode for Attribute() | The block mode ECB |
| test_cryptography.py:13:13:13:44 | ControlFlowNode for Attribute() | $@ is broken or weak, and should not be used. | test_cryptography.py:12:13:12:30 | ControlFlowNode for Attribute() | The cryptographic algorithm ARC4 |
| test_cryptography.py:22:13:22:58 | ControlFlowNode for Attribute() | $@ is broken or weak, and should not be used. | test_cryptography.py:21:13:21:30 | ControlFlowNode for Attribute() | The block mode ECB |