From 094f7bf2b65a85a7a8472acc741b0446e1e894a1 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Wed, 28 Jan 2026 11:50:57 +0000 Subject: [PATCH 01/13] add argon2 KDF algorithms --- .../cryptography/modules/CryptographyModule.qll | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll index 0831d625d803..93c8212f7458 100644 --- a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll +++ b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll @@ -70,10 +70,10 @@ module KDF { .getMember(algName) and result = algModule.asSource() and // https://github.com/pyca/cryptography/tree/main/src/cryptography/hazmat/primitives/kdf - member in ["concatkdf", "hkdf", "kbkdf", "pbkdf2", "scrypt", "x963kdf"] and + member in ["argon2", "concatkdf", "hkdf", "kbkdf", "pbkdf2", "scrypt", "x963kdf"] and algName in [ - "ConcatKDFHash", "ConcatKDFHMAC", "HKDF", "HKDFExpand", "KBKDFCMAC", "KBKDFHMAC", - "PBKDF2HMAC", "Scrypt", "X963KDF" + "Argon2d", "Argon2i", "Argon2id", "ConcatKDFHash", "ConcatKDFHMAC", "HKDF", + "HKDFExpand", "KBKDFCMAC", "KBKDFHMAC", "PBKDF2HMAC", "Scrypt", "X963KDF" ] ) } From 2642af157807522b2f62daea77e596cf31430564 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Wed, 28 Jan 2026 11:55:46 +0000 Subject: [PATCH 02/13] fix: scrypt requires salt --- .../experimental/cryptography/modules/CryptographyModule.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll index 93c8212f7458..2aec98a8c636 100644 --- a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll +++ b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll @@ -111,7 +111,7 @@ module KDF { } override predicate requiresSalt() { - this.getAlgorithm().getKDFName() in ["PBKDF2HMAC", "CONCATKDFHMAC", "HKDF"] + this.getAlgorithm().getKDFName() in ["PBKDF2HMAC", "CONCATKDFHMAC", "HKDF", "SCRYPT"] } override predicate requiresIteration() { this.getAlgorithm().getKDFName() in ["PBKDF2HMAC"] } From 8928a119545c866282a0f5f0c72f0e4cce705bb6 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Wed, 28 Jan 2026 12:10:53 +0000 Subject: [PATCH 03/13] feat: support `getSaltConfigSrc` for Argon2 variants --- .../cryptography/modules/CryptographyModule.qll | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll index 2aec98a8c636..67bf4e969aa7 100644 --- a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll +++ b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll @@ -111,7 +111,7 @@ module KDF { } override predicate requiresSalt() { - this.getAlgorithm().getKDFName() in ["PBKDF2HMAC", "CONCATKDFHMAC", "HKDF", "SCRYPT"] + this.getAlgorithm().getKDFName() in ["PBKDF2HMAC", "CONCATKDFHMAC", "HKDF", "SCRYPT", "ARGON2"] } override predicate requiresIteration() { this.getAlgorithm().getKDFName() in ["PBKDF2HMAC"] } @@ -124,8 +124,11 @@ module KDF { override DataFlow::Node getSaltConfigSrc() { this.requiresSalt() and + // ARGON2 variants have it as a keyword-only parameter + if this.getAlgorithm().getKDFName() = "ARGON2" + then result = Utils::getUltimateSrcFromApiNode(this.getKeywordParameter("salt")) // SCRYPT has it in arg 1 - if this.getAlgorithm().getKDFName() = "SCRYPT" + else if this.getAlgorithm().getKDFName() = "SCRYPT" then result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "salt")) else // EVERYTHING ELSE that uses salt is in arg 2 From f2942ca8454f75be551f04a6839105cbf725ea1b Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Wed, 28 Jan 2026 12:13:09 +0000 Subject: [PATCH 04/13] feat: support `getIterationSizeSrc` for argon2 variants --- .../cryptography/modules/CryptographyModule.qll | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll index 67bf4e969aa7..726e3d8c6188 100644 --- a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll +++ b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll @@ -114,12 +114,15 @@ module KDF { this.getAlgorithm().getKDFName() in ["PBKDF2HMAC", "CONCATKDFHMAC", "HKDF", "SCRYPT", "ARGON2"] } - override predicate requiresIteration() { this.getAlgorithm().getKDFName() in ["PBKDF2HMAC"] } + override predicate requiresIteration() { this.getAlgorithm().getKDFName() in ["PBKDF2HMAC", "ARGON2"] } override DataFlow::Node getIterationSizeSrc() { this.requiresIteration() and - // ASSUMPTION: ONLY EVER in arg 3 in PBKDF2HMAC - result = Utils::getUltimateSrcFromApiNode(this.getParameter(3, "iterations")) + if this.getAlgorithm().getKDFName() = "ARGON2" + then result = Utils::getUltimateSrcFromApiNode(this.getKeywordParameter("iterations")) + else + // ASSUMPTION: ONLY EVER in arg 3 in PBKDF2HMAC + result = Utils::getUltimateSrcFromApiNode(this.getParameter(3, "iterations")) } override DataFlow::Node getSaltConfigSrc() { From 19ed6bb1987e2b0a5a1fbd692067f63a88abd36f Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Wed, 28 Jan 2026 12:14:00 +0000 Subject: [PATCH 05/13] feat: support `getDerivedKeySizeSrc` for argon2 variants --- .../experimental/cryptography/modules/CryptographyModule.qll | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll index 726e3d8c6188..e096d3961be5 100644 --- a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll +++ b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll @@ -146,7 +146,9 @@ module KDF { // TODO: get encryption algorithm for CBC-based KDF? override DataFlow::Node getDerivedKeySizeSrc() { - if this.getAlgorithm().getKDFName() in ["KBKDFHMAC", "KBKDFCMAC"] + if this.getAlgorithm().getKDFName() = "ARGON2" + then result = Utils::getUltimateSrcFromApiNode(this.getKeywordParameter("length")) + else if this.getAlgorithm().getKDFName() in ["KBKDFHMAC", "KBKDFCMAC"] then result = Utils::getUltimateSrcFromApiNode(this.getParameter(2, "length")) else result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "length")) } From f0b7b9ce39d2301c79abc2cc50c6a2202b1e7c0b Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Wed, 28 Jan 2026 12:22:00 +0000 Subject: [PATCH 06/13] feat: add predicates relating to `lanes`, `memory_cost` KDF parameters --- .../experimental/cryptography/CryptoArtifact.qll | 8 ++++++++ .../cryptography/modules/CryptographyModule.qll | 16 ++++++++++++++++ .../modules/stdlib/HashlibModule.qll | 8 ++++++++ 3 files changed, 32 insertions(+) diff --git a/python/ql/lib/experimental/cryptography/CryptoArtifact.qll b/python/ql/lib/experimental/cryptography/CryptoArtifact.qll index e8939c981113..ac34ab27a166 100644 --- a/python/ql/lib/experimental/cryptography/CryptoArtifact.qll +++ b/python/ql/lib/experimental/cryptography/CryptoArtifact.qll @@ -135,6 +135,10 @@ abstract class KeyDerivationOperation extends CryptographicOperation { DataFlow::Node getHashConfigSrc() { none() } + DataFlow::Node getLanesConfigSrc() { none() } + + DataFlow::Node getMemoryCostConfigSrc() { none() } + // TODO: get encryption algorithm for CBC-based KDF? DataFlow::Node getDerivedKeySizeSrc() { none() } @@ -147,6 +151,10 @@ abstract class KeyDerivationOperation extends CryptographicOperation { abstract predicate requiresHash(); + abstract predicate requiresLanes(); + + abstract predicate requiresMemoryCost(); + //abstract predicate requiresKeySize(); // Going to assume all requires a size abstract predicate requiresMode(); } diff --git a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll index e096d3961be5..f3e87da603f8 100644 --- a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll +++ b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll @@ -116,6 +116,10 @@ module KDF { override predicate requiresIteration() { this.getAlgorithm().getKDFName() in ["PBKDF2HMAC", "ARGON2"] } + override predicate requiresLanes() { this.getAlgorithm().getKDFName() in ["ARGON2"] } + + override predicate requiresMemoryCost() { this.getAlgorithm().getKDFName() in ["ARGON2"] } + override DataFlow::Node getIterationSizeSrc() { this.requiresIteration() and if this.getAlgorithm().getKDFName() = "ARGON2" @@ -144,6 +148,18 @@ module KDF { result = Utils::getUltimateSrcFromApiNode(this.getParameter(0, "algorithm")) } + override DataFlow::Node getLanesConfigSrc() { + this.requiresLanes() and + // ASSUMPTION: ONLY EVER in keyword parameter + result = Utils::getUltimateSrcFromApiNode(this.getKeywordParameter("lanes")) + } + + override DataFlow::Node getMemoryCostConfigSrc() { + this.requiresMemoryCost() and + // ASSUMPTION: ONLY EVER in keyword parameter + result = Utils::getUltimateSrcFromApiNode(this.getKeywordParameter("memory_cost")) + } + // TODO: get encryption algorithm for CBC-based KDF? override DataFlow::Node getDerivedKeySizeSrc() { if this.getAlgorithm().getKDFName() = "ARGON2" diff --git a/python/ql/lib/experimental/cryptography/modules/stdlib/HashlibModule.qll b/python/ql/lib/experimental/cryptography/modules/stdlib/HashlibModule.qll index 346512e9a2db..487ed783ff56 100644 --- a/python/ql/lib/experimental/cryptography/modules/stdlib/HashlibModule.qll +++ b/python/ql/lib/experimental/cryptography/modules/stdlib/HashlibModule.qll @@ -197,6 +197,10 @@ module KDF { override predicate requiresSalt() { any() } override predicate requiresIteration() { any() } + + override predicate requiresLanes() { none() } + + override predicate requiresMemoryCost() { none() } } // TODO: better modeling of scrypt @@ -233,5 +237,9 @@ module KDF { override predicate requiresSalt() { any() } override predicate requiresIteration() { none() } + + override predicate requiresLanes() { none() } + + override predicate requiresMemoryCost() { none() } } } From 12a161405f87f05dac41f916c1d7ac9299d64310 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Wed, 28 Jan 2026 14:14:06 +0000 Subject: [PATCH 07/13] add argon2 variants to known KDF names --- .../lib/experimental/cryptography/CryptoAlgorithmNames.qll | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/ql/lib/experimental/cryptography/CryptoAlgorithmNames.qll b/python/ql/lib/experimental/cryptography/CryptoAlgorithmNames.qll index 937b45b7aacc..1a641c0071e6 100644 --- a/python/ql/lib/experimental/cryptography/CryptoAlgorithmNames.qll +++ b/python/ql/lib/experimental/cryptography/CryptoAlgorithmNames.qll @@ -104,9 +104,9 @@ predicate isSymmetricEncryptionAlgorithm(string name) { predicate isKeyDerivationAlgorithm(string name) { name = [ - "ARGON2", "CONCATKDF", "CONCATKDFHASH", "CONCATKDFHMAC", "KBKDFCMAC", "BCRYPT", "HKDF", - "HKDFEXPAND", "KBKDF", "KBKDFHMAC", "PBKDF1", "PBKDF2", "PBKDF2HMAC", "PKCS5", "SCRYPT", - "X963KDF", "EVPKDF" + "ARGON2", "ARGON2D", "ARGON2I", "ARGON2ID", "CONCATKDF", "CONCATKDFHASH", "CONCATKDFHMAC", + "KBKDFCMAC", "BCRYPT", "HKDF", "HKDFEXPAND", "KBKDF", "KBKDFHMAC", "PBKDF1", "PBKDF2", + "PBKDF2HMAC", "PKCS5", "SCRYPT", "X963KDF", "EVPKDF" ] } From 743786073f375cef15fcc9bd1923fce4e186fa05 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Fri, 1 May 2026 15:22:07 +0100 Subject: [PATCH 08/13] fix: multiple reporting of bcrypt initialisation --- .../ql/lib/semmle/javascript/frameworks/CryptoLibraries.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/CryptoLibraries.qll b/javascript/ql/lib/semmle/javascript/frameworks/CryptoLibraries.qll index 9cc76b5f5b8c..999c7034cc4a 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/CryptoLibraries.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/CryptoLibraries.qll @@ -770,7 +770,7 @@ private module Bcrypt { super.getArgument(0) = input } - override DataFlow::Node getInitialization() { result = init.asSource() } + override DataFlow::Node getInitialization() { result = this } override DataFlow::Node getAnInput() { result = input } From 89818227eb18d884fcd1484d4a59f379a9b4eed5 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Fri, 1 May 2026 15:51:54 +0100 Subject: [PATCH 09/13] bring in changes from cryptsweeper --- .../cryptography/CryptoAlgorithmNames.qll | 4 +- .../cryptography/CryptoArtifact.qll | 41 +- .../modules/CryptographyModule.qll | 356 ++++++++++++++---- .../modules/stdlib/HashlibModule.qll | 111 ++++-- 4 files changed, 407 insertions(+), 105 deletions(-) diff --git a/python/ql/lib/experimental/cryptography/CryptoAlgorithmNames.qll b/python/ql/lib/experimental/cryptography/CryptoAlgorithmNames.qll index 1a641c0071e6..8ce9d25c193c 100644 --- a/python/ql/lib/experimental/cryptography/CryptoAlgorithmNames.qll +++ b/python/ql/lib/experimental/cryptography/CryptoAlgorithmNames.qll @@ -104,7 +104,9 @@ predicate isSymmetricEncryptionAlgorithm(string name) { predicate isKeyDerivationAlgorithm(string name) { name = [ - "ARGON2", "ARGON2D", "ARGON2I", "ARGON2ID", "CONCATKDF", "CONCATKDFHASH", "CONCATKDFHMAC", + // 'ARGON2' should only be used in cases where the specific variant in use cannot be discerned reliably + "ARGON2", "ARGON2D", "ARGON2I", "ARGON2ID", + "CONCATKDF", "CONCATKDFHASH", "CONCATKDFHMAC", "KBKDFCMAC", "BCRYPT", "HKDF", "HKDFEXPAND", "KBKDF", "KBKDFHMAC", "PBKDF1", "PBKDF2", "PBKDF2HMAC", "PKCS5", "SCRYPT", "X963KDF", "EVPKDF" ] diff --git a/python/ql/lib/experimental/cryptography/CryptoArtifact.qll b/python/ql/lib/experimental/cryptography/CryptoArtifact.qll index ac34ab27a166..a99e49e71a84 100644 --- a/python/ql/lib/experimental/cryptography/CryptoArtifact.qll +++ b/python/ql/lib/experimental/cryptography/CryptoArtifact.qll @@ -20,7 +20,7 @@ abstract class CryptographicArtifact extends DataFlow::Node { } abstract class SymmetricCipher extends CryptographicArtifact { abstract SymmetricEncryptionAlgorithm getEncryptionAlgorithm(); - abstract BlockMode getBlockMode(); + abstract BlockModeInstance getBlockMode(); final predicate hasBlockMode() { exists(this.getBlockMode()) } } @@ -55,9 +55,14 @@ abstract class CryptographicOperation extends CryptographicArtifact, API::CallNo not this.hasAlgorithm() } + /** Gets the data flow node where the cryptographic algorithm used in this operation is configured. */ + abstract DataFlow::Node getInitialisation(); // TODO: this might have to be parameterized by a configuration source for // situations where an operation is passed an algorithm + /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */ abstract CryptographicAlgorithm getAlgorithm(); + /** Gets an input the algorithm is used on, for example the plain text input to be encrypted. */ + abstract DataFlow::Node getAnInput(); } /** A key generation operation for asymmetric keys */ @@ -129,10 +134,11 @@ abstract class KeyDerivationAlgorithm extends CryptographicAlgorithm { } abstract class KeyDerivationOperation extends CryptographicOperation { - DataFlow::Node getIterationSizeSrc() { none() } - + DataFlow::Node getSaltConfigSink() { none() } DataFlow::Node getSaltConfigSrc() { none() } + DataFlow::Node getIterationSizeSrc() { none() } + DataFlow::Node getHashConfigSrc() { none() } DataFlow::Node getLanesConfigSrc() { none() } @@ -175,6 +181,8 @@ abstract class EncryptionAlgorithm extends CryptographicAlgorithm { // class does not have this common predicate. } +abstract class EncryptionOperation extends CryptographicOperation { } + /** * Algorithms directly or indirectly related to asymmetric encryption, * e.g., RSA, DSA, but also RSA padding algorithms @@ -200,6 +208,8 @@ abstract class SymmetricEncryptionAlgorithm extends EncryptionAlgorithm { // TODO: add a stream cipher predicate? } +abstract class SymmetricEncryptionOperation extends EncryptionOperation { } + // Used only to categorize all padding into a single object, // DO_NOT add predicates here. Only for categorization purposes. abstract class PaddingAlgorithm extends CryptographicAlgorithm { } @@ -230,7 +240,7 @@ abstract class EllipticCurveAlgorithm extends AsymmetricAlgorithm { final int getCurveBitSize() { isEllipticCurveAlgorithm(this.getCurveName(), result) } } -abstract class BlockMode extends CryptographicAlgorithm { +abstract class BlockModeInstance extends CryptographicAlgorithm { final string getBlockModeName() { if exists(string n | n = this.getName() and isCipherBlockModeAlgorithm(n)) then isCipherBlockModeAlgorithm(result) and result = this.getName() @@ -240,21 +250,38 @@ abstract class BlockMode extends CryptographicAlgorithm { /** * Gets the source of the IV configuration. */ - abstract DataFlow::Node getIVorNonce(); + abstract DataFlow::Node getIVOrNonceSrc(); + + /** + * Gets the sink of the IV configuration. + */ + abstract DataFlow::Node getIVOrNonceSink(); - final predicate hasIVorNonce() { exists(this.getIVorNonce()) } + final predicate hasIVorNonce() { exists(this.getIVOrNonceSrc()) } } abstract class KeyWrapOperation extends CryptographicOperation { } abstract class AuthenticatedEncryptionAlgorithm extends SymmetricEncryptionAlgorithm { - final string getAuthticatedEncryptionName() { + final string getAuthenticatedEncryptionName() { if exists(string n | n = this.getName() and isSymmetricEncryptionAlgorithm(n)) then isSymmetricEncryptionAlgorithm(result) and result = this.getName() else result = unknownAlgorithm() } } +abstract class AuthenticatedEncryptionOperation extends SymmetricEncryptionOperation { + /** + * Gets the source of the IV configuration. + */ + abstract DataFlow::Node getIVOrNonceSrc(); + + /** + * Gets the sink of the IV configuration. + */ + abstract DataFlow::Node getIVOrNonceSink(); +} + abstract class KeyExchangeAlgorithm extends AsymmetricAlgorithm { final string getKeyExchangeName() { if exists(string n | n = this.getName() and isKeyExchangeAlgorithm(n)) diff --git a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll index f3e87da603f8..093b35bb1f3e 100644 --- a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll +++ b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll @@ -21,14 +21,14 @@ module Hashes { */ // Copying use of nomagic from similar predicate in codeql/main pragma[nomagic] - DataFlow::Node cryptographyMemberHashAlgorithm(string hashName) { - result = + DataFlow::Node cryptographyMemberHashAlgorithm(API::Node algModule, string hashName) { + algModule = API::moduleImport("cryptography") .getMember("hazmat") .getMember("primitives") .getMember("hashes") - .getMember(hashName) - .asSource() and + .getMember(hashName) and + result = algModule.asSource() and // Don't matches known non-hash members // https://github.com/pyca/cryptography/blob/main/src/cryptography/hazmat/primitives/hashes.py#L69-L111 not hashName in [ @@ -45,21 +45,66 @@ module Hashes { * https://cryptography.io/en/latest/hazmat/primitives/cryptographic-hashes/#cryptography.hazmat.primitives.hashes.Hash */ class CryptographyGenericHashAlgorithm extends HashAlgorithm { - CryptographyGenericHashAlgorithm() { this = cryptographyMemberHashAlgorithm(_) } + CryptographyGenericHashAlgorithm() { this = cryptographyMemberHashAlgorithm(_, _) } override string getName() { - exists(string rawName | this = cryptographyMemberHashAlgorithm(rawName) | + exists(string rawName | this = cryptographyMemberHashAlgorithm(_, rawName) | result = super.normalizeName(rawName) ) } + + API::Node getModule() { this = cryptographyMemberHashAlgorithm(result, _) } } // NOTE: no need to model hashes used for PBKDF2HMAC (and other similar KDF HMAC), the API requires the specified algorithm // is an instance of `HashAlgorithm` handled by `CryptographyGenericHashArtifact` + + API::CallNode getCryptographyHashInitialisation(CryptographyGenericHashAlgorithm hash) { + result = API::moduleImport("cryptography") + .getMember("hazmat") + .getMember("primitives") + .getMember("hashes") + .getMember("Hash") + .getACall() and + hash.getModule().getACall().flowsTo(result.getParameter(0).asSink()) + } + + API::CallNode getCryptographyHashOneShot(CryptographyGenericHashAlgorithm hash) { + result = API::moduleImport("cryptography") + .getMember("hazmat") + .getMember("primitives") + .getMember("hashes") + .getMember("Hash") + .getMember("hash") + .getACall() and + hash.getModule().getACall().flowsTo(result.getParameter(0, "algorithm").asSink()) + } + + API::CallNode getCryptographyHashOperation(CryptographyGenericHashAlgorithm hash, API::CallNode initialisation) { + (initialisation = getCryptographyHashInitialisation(hash) and result = initialisation.getAMethodCall("update")) + or + (initialisation = getCryptographyHashOneShot(hash) and result = initialisation) + } + + class CryptographyHashDigestOperation extends CryptographicOperation { + API::CallNode initialisation; + + CryptographyHashDigestOperation() { this = getCryptographyHashOperation(_, initialisation) } + + override DataFlow::Node getInitialisation() { result = initialisation } + + override DataFlow::Node getAnInput() { + (initialisation = getCryptographyHashInitialisation(_) and (result = this.getArg(0) or result = this.getArgByName("data"))) + or + (initialisation = getCryptographyHashOneShot(_) and (result = this.getArg(1) or result = this.getArgByName("data"))) + } + + override CryptographyGenericHashAlgorithm getAlgorithm() { this = getCryptographyHashOperation(result, _) } + } } // https://cryptography.io/en/latest/hazmat/primitives/key-derivation-functions/#module-cryptography.hazmat.primitives.kdf module KDF { - DataFlow::Node genericKDFArtifact(API::Node algModule, string algName) { + DataFlow::Node genericKDFModule(API::Node algModule, string algName) { exists(string member | algModule = API::moduleImport("cryptography") @@ -70,11 +115,21 @@ module KDF { .getMember(algName) and result = algModule.asSource() and // https://github.com/pyca/cryptography/tree/main/src/cryptography/hazmat/primitives/kdf - member in ["argon2", "concatkdf", "hkdf", "kbkdf", "pbkdf2", "scrypt", "x963kdf"] and - algName in [ - "Argon2d", "Argon2i", "Argon2id", "ConcatKDFHash", "ConcatKDFHMAC", "HKDF", - "HKDFExpand", "KBKDFCMAC", "KBKDFHMAC", "PBKDF2HMAC", "Scrypt", "X963KDF" - ] + ( + member = "argon2" and algName in ["Argon2d", "Argon2i", "Argon2id"] + or + member = "concatkdf" and algName in ["ConcatKDFHash", "ConcatKDFHMAC"] + or + member = "hkdf" and algName in ["HKDF", "HKDFExpand"] + or + member = "kbkdf" and algName in ["KBKDFCMAC", "KBKDFHMAC"] + or + member = "pbkdf2" and algName in ["PBKDF2HMAC"] + or + member = "scrypt" and algName in ["Scrypt"] + or + member = "x963kdf" and algName in ["X963KDF"] + ) ) } @@ -83,96 +138,120 @@ module KDF { * https://cryptography.io/en/latest/hazmat/primitives/key-derivation-functions/#module-cryptography.hazmat.primitives.kdf */ class CryptographyKDFAlgorithm extends KeyDerivationAlgorithm { - CryptographyKDFAlgorithm() { this = genericKDFArtifact(_, _) } + CryptographyKDFAlgorithm() { this = genericKDFModule(_, _) } override string getName() { - exists(string rawName | this = genericKDFArtifact(_, rawName) | + exists(string rawName | this = genericKDFModule(_, rawName) | // TODO: is HKDFExpand ok to categorize as HKDF? result = super.normalizeName(rawName) ) } - API::Node getModule() { this = genericKDFArtifact(result, _) } + API::Node getModule() { this = genericKDFModule(result, _) } } - API::CallNode getCryptographyKDFOperation(CryptographyKDFAlgorithm kdf) { + API::CallNode getCryptographyKDFInitialisation(CryptographyKDFAlgorithm kdf) { result = kdf.getModule().getACall() } + API::CallNode getCryptographyKDFOperation(CryptographyKDFAlgorithm kdf, API::CallNode instance) { + instance = getCryptographyKDFInitialisation(kdf) and + result = instance.getAMethodCall(["derive", "derive_into", "verify"]) + } + class CryptographyKDFOperation extends KeyDerivationOperation { - CryptographyKDFOperation() { this = getCryptographyKDFOperation(_) } + API::CallNode instance; + + CryptographyKDFOperation() { this = getCryptographyKDFOperation(_, instance) } - override KeyDerivationAlgorithm getAlgorithm() { this = getCryptographyKDFOperation(result) } + override KeyDerivationAlgorithm getAlgorithm() { this = getCryptographyKDFOperation(result, instance) } - override predicate requiresHash() { this.getAlgorithm().getKDFName() != "KBKDFCMAC" } + override DataFlow::Node getInitialisation() { result = instance } + override DataFlow::Node getAnInput() { result = this.getArg(0) or result = this.getArgByName("key_material") } + + override predicate requiresHash() { not this.getAlgorithm().getKDFName() in ["KBKDFCMAC", "ARGON2D", "ARGON2I", "ARGON2ID"] } override predicate requiresMode() { this.getAlgorithm().getKDFName() in ["KBKDFCMAC", "KBKDFHMAC"] } override predicate requiresSalt() { - this.getAlgorithm().getKDFName() in ["PBKDF2HMAC", "CONCATKDFHMAC", "HKDF", "SCRYPT", "ARGON2"] + this.getAlgorithm().getKDFName() in ["PBKDF2HMAC", "CONCATKDFHMAC", "HKDF", "SCRYPT", "ARGON2D", "ARGON2I", "ARGON2ID"] } - override predicate requiresIteration() { this.getAlgorithm().getKDFName() in ["PBKDF2HMAC", "ARGON2"] } + override predicate requiresIteration() { this.getAlgorithm().getKDFName() in ["PBKDF2HMAC", "ARGON2D", "ARGON2I", "ARGON2ID"] } - override predicate requiresLanes() { this.getAlgorithm().getKDFName() in ["ARGON2"] } + override predicate requiresLanes() { this.getAlgorithm().getKDFName() in ["ARGON2D", "ARGON2I", "ARGON2ID"] } - override predicate requiresMemoryCost() { this.getAlgorithm().getKDFName() in ["ARGON2"] } + override predicate requiresMemoryCost() { this.getAlgorithm().getKDFName() in ["ARGON2D", "ARGON2I", "ARGON2ID"] } override DataFlow::Node getIterationSizeSrc() { this.requiresIteration() and - if this.getAlgorithm().getKDFName() = "ARGON2" - then result = Utils::getUltimateSrcFromApiNode(this.getKeywordParameter("iterations")) + if this.getAlgorithm().getKDFName() in ["ARGON2D", "ARGON2I", "ARGON2ID"] + then result = Utils::getUltimateSrcFromApiNode(instance.getKeywordParameter("iterations")) else // ASSUMPTION: ONLY EVER in arg 3 in PBKDF2HMAC - result = Utils::getUltimateSrcFromApiNode(this.getParameter(3, "iterations")) + result = Utils::getUltimateSrcFromApiNode(instance.getParameter(3, "iterations")) } - override DataFlow::Node getSaltConfigSrc() { - this.requiresSalt() and + + override DataFlow::Node getSaltConfigSink() { + result = this.getSaltConfigSink(_) + } + + private DataFlow::Node getSaltConfigSink(API::Node apiNode) { + this.requiresSalt() and result = apiNode.asSink() and // ARGON2 variants have it as a keyword-only parameter - if this.getAlgorithm().getKDFName() = "ARGON2" - then result = Utils::getUltimateSrcFromApiNode(this.getKeywordParameter("salt")) + if this.getAlgorithm().getKDFName() in ["ARGON2D", "ARGON2I", "ARGON2ID"] + then apiNode = instance.getKeywordParameter("salt") // SCRYPT has it in arg 1 else if this.getAlgorithm().getKDFName() = "SCRYPT" - then result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "salt")) + then apiNode = instance.getParameter(1, "salt") else // EVERYTHING ELSE that uses salt is in arg 2 - result = Utils::getUltimateSrcFromApiNode(this.getParameter(2, "salt")) + apiNode = instance.getParameter(2, "salt") + } + + override DataFlow::Node getSaltConfigSrc() { + result = this.getSaltConfigSrc(_) + } + + private DataFlow::Node getSaltConfigSrc(API::Node apiNode) { + exists(getSaltConfigSink(apiNode)) and + result = Utils::getUltimateSrcFromApiNode(apiNode) } override DataFlow::Node getHashConfigSrc() { this.requiresHash() and // ASSUMPTION: ONLY EVER in arg 0 - result = Utils::getUltimateSrcFromApiNode(this.getParameter(0, "algorithm")) + result = Utils::getUltimateSrcFromApiNode(instance.getParameter(0, "algorithm")) } override DataFlow::Node getLanesConfigSrc() { - this.requiresLanes() and + this.requiresLanes() and // ASSUMPTION: ONLY EVER in keyword parameter - result = Utils::getUltimateSrcFromApiNode(this.getKeywordParameter("lanes")) + result = Utils::getUltimateSrcFromApiNode(instance.getKeywordParameter("lanes")) } override DataFlow::Node getMemoryCostConfigSrc() { this.requiresMemoryCost() and // ASSUMPTION: ONLY EVER in keyword parameter - result = Utils::getUltimateSrcFromApiNode(this.getKeywordParameter("memory_cost")) + result = Utils::getUltimateSrcFromApiNode(instance.getKeywordParameter("memory_cost")) } // TODO: get encryption algorithm for CBC-based KDF? override DataFlow::Node getDerivedKeySizeSrc() { - if this.getAlgorithm().getKDFName() = "ARGON2" - then result = Utils::getUltimateSrcFromApiNode(this.getKeywordParameter("length")) + if this.getAlgorithm().getKDFName() in ["ARGON2D", "ARGON2I", "ARGON2ID"] + then result = Utils::getUltimateSrcFromApiNode(instance.getKeywordParameter("length")) else if this.getAlgorithm().getKDFName() in ["KBKDFHMAC", "KBKDFCMAC"] - then result = Utils::getUltimateSrcFromApiNode(this.getParameter(2, "length")) - else result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "length")) + then result = Utils::getUltimateSrcFromApiNode(instance.getParameter(2, "length")) + else result = Utils::getUltimateSrcFromApiNode(instance.getParameter(1, "length")) } override DataFlow::Node getModeSrc() { this.requiresMode() and // ASSUMPTION: ONLY EVER in arg 1 - result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "mode")) + result = Utils::getUltimateSrcFromApiNode(instance.getParameter(1, "mode")) } } } @@ -203,11 +282,15 @@ module Encryption { ) } - class CryptographyKeyWrap extends KeyWrapOperation, SymmetricEncryptionAlgorithm, BlockMode, + class CryptographyKeyWrap extends KeyWrapOperation, SymmetricEncryptionAlgorithm, BlockModeInstance, SymmetricCipher { CryptographyKeyWrap() { this = genericKeyWrapArtifact() } + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { none() /* not implemented */ } + override string getName() { //Cryptography Key Wrap Artifact's use ECB block mode by default: // https://github.com/pyca/cryptography/blob/main/src/cryptography/hazmat/primitives/keywrap.py#L14 @@ -222,11 +305,13 @@ module Encryption { /** * ECB mode is effectively no block mode and no IV is associated with this mode. */ - override DataFlow::Node getIVorNonce() { none() } + override DataFlow::Node getIVOrNonceSrc() { none() } + + override DataFlow::Node getIVOrNonceSink() { none() } override SymmetricEncryptionAlgorithm getEncryptionAlgorithm() { result = this } - override BlockMode getBlockMode() { result = this } + override BlockModeInstance getBlockMode() { result = this } override CryptographicAlgorithm getAlgorithm() { result = this } } @@ -253,7 +338,7 @@ module Encryption { result = algModule.asSource() } - class CryptographyAEAD extends BlockMode, AuthenticatedEncryptionAlgorithm, SymmetricCipher { + class CryptographyAEAD extends AuthenticatedEncryptionAlgorithm { CryptographyAEAD() { this = genericAEADArtifact(_, _) } API::Node getMember(string memberName) { result = this.getModule().getMember(memberName) } @@ -261,8 +346,17 @@ module Encryption { API::Node getModule() { this = genericAEADArtifact(result, _) } override string getName() { + this = genericAEADArtifact(_, result) + } + + string getBlockModeName() { + exists(string rawName | genericAEADArtifact(_, rawName) = this | + result = this.normalizedBlockNames(rawName) + ) + } + + string getEncryptionName() { exists(string rawName | genericAEADArtifact(_, rawName) = this | - result = this.normalizedBlockNames(rawName) or result = this.normalizedEncryptionName(rawName) ) } @@ -290,19 +384,6 @@ module Encryption { then result = super.normalizeName("AES") else result = super.normalizeName(rawName) } - - /** - * Since the IV/Nonce is dependent on the API, we could attempt a dataflow to derive - * what it is internal to the library, but instead we take the stance that - * the IV/Nonce is non-existent/unknown to simplify analyses and to be - * safe in case of API changes. Uses of this API must therefore be - * individually assessed for correct IV use. - */ - override DataFlow::Node getIVorNonce() { none() } - - override SymmetricEncryptionAlgorithm getEncryptionAlgorithm() { result = this } - - override BlockMode getBlockMode() { result = this } } DataFlow::Node genericAEADKeyGen(CryptographyAEAD aead) { @@ -314,8 +395,12 @@ module Encryption { override CryptographyAEAD getAlgorithm() { this = genericAEADKeyGen(result) } + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { none() /* not implemented */ } + override int getKeySizeInBits(DataFlow::Node configSrc) { - if this.getAlgorithm().getAuthticatedEncryptionName() = "ChaCha20Poly1305 " + if this.getAlgorithm().getAuthenticatedEncryptionName() = "ChaCha20Poly1305 " then result = 32 * 8 else ( result = configSrc.asExpr().(IntegerLiteral).getValue() and @@ -334,6 +419,87 @@ module Encryption { not exists(this.keyBitLengthSrc()) and result = this } } + + API::CallNode getCryptographyAEADInstance(CryptographyAEAD aead) { + result = aead.getModule().getACall() + } + + Expr getLastElt(List list) { + exists(int size | size = count(list.getAnElt()) and size > 0 and result = list.getElt(size - 1)) + } + + class CryptographyAEADEncrypt extends AuthenticatedEncryptionOperation { + API::CallNode aeadInstance; + string methodName; + + CryptographyAEADEncrypt() { + aeadInstance = getCryptographyAEADInstance(_) and + this = aeadInstance.getAMethodCall(methodName) + and methodName in ["encrypt", "encrypt_into"] + } + + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { none() /* not implemented */ } + + override DataFlow::Node getIVOrNonceSrc() { + result = Utils::getUltimateSrcFromApiNode(this.getIVOrNonceSinkCore()) + } + + override DataFlow::Node getIVOrNonceSink() { + result = this.getIVOrNonceSinkCore().asSink() + } + + private API::Node getIVOrNonceSinkCore() { + if this.getAlgorithmName() = "AESSIV" + then result = any(API::Node candidate, List aad | + aad = this.getParameterSource(1, "associated_data").asExpr() and + candidate.asSink().asExpr() = getLastElt(aad) | + candidate + ) + else result = this.getParameter(0, "nonce") + } + + DataFlow::CallCfgNode getAeadInstance() { result = aeadInstance } + + override CryptographyAEAD getAlgorithm() { aeadInstance = getCryptographyAEADInstance(result) } + } + + class CryptographyAEADDecrypt extends AuthenticatedEncryptionOperation { + CryptographyAEAD aeadInstance; + string methodName; + + CryptographyAEADDecrypt() { + this = getCryptographyAEADInstance(aeadInstance).getAMethodCall(methodName) + and methodName in ["decrypt", "decrypt_into"] + } + + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { none() /* not implemented */ } + + override DataFlow::Node getIVOrNonceSrc() { + result = Utils::getUltimateSrcFromApiNode(this.getIVOrNonceSinkCore()) + } + + override DataFlow::Node getIVOrNonceSink() { + result = this.getIVOrNonceSinkCore().asSink() + } + + private API::Node getIVOrNonceSinkCore() { + if this.getAlgorithmName() = "AESSIV" + then result = any(API::Node candidate, List aad | + aad = this.getParameterSource(1, "associated_data").asExpr() and + candidate.asSink().asExpr() = getLastElt(aad) | + candidate + ) + else result = this.getParameter(0, "nonce") + } + + DataFlow::CallCfgNode getAeadInstance() { result = aeadInstance } + + override CryptographyAEAD getAlgorithm() { aeadInstance = getCryptographyAEADInstance(result) } + } } /** @@ -347,7 +513,7 @@ module Encryption { ) } - class CryptographyFernet extends SymmetricPadding, BlockMode, SymmetricEncryptionAlgorithm, + class CryptographyFernet extends SymmetricPadding, BlockModeInstance, SymmetricEncryptionAlgorithm, SymmetricCipher { CryptographyFernet() { this = fernetConstructor() } @@ -368,11 +534,13 @@ module Encryption { * The current API shows the IV is set via os.urandom: * https://github.com/pyca/cryptography/blob/main/src/cryptography/fernet.py */ - override DataFlow::Node getIVorNonce() { none() } + override DataFlow::Node getIVOrNonceSrc() { none() } + + override DataFlow::Node getIVOrNonceSink() { none() } override SymmetricEncryptionAlgorithm getEncryptionAlgorithm() { result = this } - override BlockMode getBlockMode() { result = this } + override BlockModeInstance getBlockMode() { result = this } } API::CallNode fernetKeyGen(CryptographyFernet fernetCall) { @@ -385,6 +553,10 @@ module Encryption { class FernetKeyGen extends SymmetricKeyGen { FernetKeyGen() { this = fernetKeyGen(_) } + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { none() /* not implemented */ } + override DataFlow::Node getKeyConfigSrc() { result = this } override int getKeySizeInBits(DataFlow::Node configSrc) { @@ -417,8 +589,8 @@ module Encryption { } // https://cryptography.io/en/latest/hazmat/primitives/symmetric-encryption/#module-cryptography.hazmat.primitives.ciphers.modes - class CryptographyGenericBlockMode extends BlockMode { - CryptographyGenericBlockMode() { this = genericBlockMode(_) } + class CryptographyGenericBlockModeInstance extends BlockModeInstance { + CryptographyGenericBlockModeInstance() { this = genericBlockMode(_) } override string getName() { exists(string rawName | this = genericBlockMode(rawName) | @@ -426,10 +598,17 @@ module Encryption { ) } - override DataFlow::Node getIVorNonce() { - exists(string paramName | paramName = ["initialization_vector", "nonce"] | - result = - Utils::getUltimateSrcFromApiNode(this.(API::CallNode).getParameter(0, paramName)) + override DataFlow::Node getIVOrNonceSrc() { + result = Utils::getUltimateSrcFromApiNode(this.getIVOrNonceSinkCore()) + } + + override DataFlow::Node getIVOrNonceSink() { + result = this.getIVOrNonceSinkCore().asSink() + } + + private API::Node getIVOrNonceSinkCore() { + exists(string paramName | paramName in ["initialization_vector", "nonce"] | + result = this.(API::CallNode).getParameter(0, paramName) ) } } @@ -487,7 +666,6 @@ module Encryption { API::moduleImport("cryptography") .getMember("hazmat") .getMember("primitives") - .getMember("ciphers") .getMember("padding") .getMember(name) .getACall() and @@ -508,8 +686,8 @@ module Encryption { /** * https://cryptography.io/en/latest/hazmat/primitives/symmetric-encryption/#cryptography.hazmat.primitives.ciphers.Cipher */ - class CyrptographyGenericCipher extends SymmetricCipher { - CyrptographyGenericCipher() { + class CryptographyGenericCipher extends SymmetricCipher { + CryptographyGenericCipher() { this = API::moduleImport("cryptography") .getMember("hazmat") @@ -524,7 +702,7 @@ module Encryption { Utils::getUltimateSrcFromApiNode(this.(API::CallNode).getParameter(0, "algorithm")) } - override BlockMode getBlockMode() { + override BlockModeInstance getBlockMode() { result = Utils::getUltimateSrcFromApiNode(this.(API::CallNode).getParameter(1, "mode")) } } @@ -663,6 +841,10 @@ module Encryption { class CryptographyRSAKeyGen extends AsymmetricKeyGen, AsymmetricEncryptionAlgorithm { CryptographyRSAKeyGen() { this = getRSAKeyGenCall() } + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { none() /* not implemented */ } + override DataFlow::Node getKeyConfigSrc() { result = Utils::getUltimateSrcFromApiNode(this.(API::CallNode).getParameter(1, "key_size")) @@ -776,6 +958,10 @@ module Encryption { class CryptographyEllipticCurveKeyGen extends AsymmetricKeyGen { CryptographyEllipticCurveKeyGen() { this = getEllipticCurveKeyGenCall() } + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { none() /* not implemented */ } + override DataFlow::Node getKeyConfigSrc() { result = Utils::getUltimateSrcFromApiNode(this.(API::CallNode).getParameter(0, "curve")) } @@ -867,6 +1053,10 @@ module Encryption { .getACall() } + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { none() /* not implemented */ } + override string getName() { result = super.normalizeName("DiffieHellman") } override DataFlow::Node getKeyConfigSrc() { @@ -896,6 +1086,10 @@ module Encryption { .asSource() } + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { none() /* not implemented */ } + override string getName() { result = super.normalizeName("DiffieHellman") or @@ -931,6 +1125,10 @@ module Encryption { .asSource() } + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { none() /* not implemented */ } + override string getName() { result = super.normalizeName("DiffieHellman") or @@ -963,6 +1161,10 @@ module Encryption { ) } + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { none() /* not implemented */ } + override DataFlow::Node getKeyConfigSrc() { result = Utils::getUltimateSrcFromApiNode(this.(API::CallNode).getParameter(0, "key_size")) @@ -997,6 +1199,10 @@ module Encryption { .asSource() } + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { none() /* not implemented */ } + override string getName() { result = super.normalizeName("EDDSA") or @@ -1032,6 +1238,10 @@ module Encryption { .asSource() } + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { none() /* not implemented */ } + override string getName() { result = super.normalizeName("EDDSA") or diff --git a/python/ql/lib/experimental/cryptography/modules/stdlib/HashlibModule.qll b/python/ql/lib/experimental/cryptography/modules/stdlib/HashlibModule.qll index 487ed783ff56..950bee3191f9 100644 --- a/python/ql/lib/experimental/cryptography/modules/stdlib/HashlibModule.qll +++ b/python/ql/lib/experimental/cryptography/modules/stdlib/HashlibModule.qll @@ -17,12 +17,13 @@ module Hashes { * Represents a hash algorithm used by `hashlib.new`, where the hash algorithm is a string in the first argument. */ class HashlibNewHashAlgorithm extends HashAlgorithm { + API::CallNode instance; + HashlibNewHashAlgorithm() { - this = - Utils::getUltimateSrcFromApiNode(API::moduleImport("hashlib") + instance = API::moduleImport("hashlib") .getMember("new") - .getACall() - .getParameter(0, "name")) + .getACall() and + this = Utils::getUltimateSrcFromApiNode(instance.getParameter(0, "name")) } override string getName() { @@ -31,6 +32,8 @@ module Hashes { // if not a known/static string, assume from an outside source and the algorithm is UNKNOWN not this.asExpr() instanceof StringLiteral and result = unknownAlgorithm() } + + API::CallNode getInstance() { result = instance } } /** @@ -71,14 +74,15 @@ module Hashes { * In these cases, the algorithm is considered 'UNKNOWN'. */ class HashlibFileDigestAlgorithm extends HashAlgorithm { + API::CallNode instance; + HashlibFileDigestAlgorithm() { - this = - Utils::getUltimateSrcFromApiNode(API::moduleImport("hashlib") + instance = API::moduleImport("hashlib") .getMember("file_digest") - .getACall() - .getParameter(1, "digest")) and + .getACall() and + this = Utils::getUltimateSrcFromApiNode(instance.getParameter(1, "digest")) and // Ignore sources that are hash constructors, allow `HashlibMemberAlgorithm` to detect these - this != hashlibMemberHashAlgorithm(_) and + this != hashlibMemberHashAlgorithm(_, _) and // Ignore sources that are HMAC objects, to be handled by HmacModule this != API::moduleImport("hmac").getMember("new").getACall() and this != API::moduleImport("hmac").getMember("HMAC").getACall() @@ -93,6 +97,8 @@ module Hashes { not this.asExpr() instanceof StringLiteral and result = unknownAlgorithm() } + + API::CallNode getInstance() { result = instance } } /** @@ -104,8 +110,9 @@ module Hashes { */ // Copying use of nomagic from similar predicate in codeql/main pragma[nomagic] - DataFlow::Node hashlibMemberHashAlgorithm(string hashName) { - result = API::moduleImport("hashlib").getMember(hashName).asSource() and + DataFlow::Node hashlibMemberHashAlgorithm(API::Node hashModule, string hashName) { + hashModule = API::moduleImport("hashlib").getMember(hashName) and + result = hashModule.asSource() and // Don't matches known non-hash members not hashName in [ "new", "pbkdf2_hmac", "algorithms_available", "algorithms_guaranteed", "file_digest" @@ -119,13 +126,40 @@ module Hashes { * e.g., `hashlib.sha512`. */ class HashlibMemberAlgorithm extends HashAlgorithm { - HashlibMemberAlgorithm() { this = hashlibMemberHashAlgorithm(_) } + HashlibMemberAlgorithm() { this = hashlibMemberHashAlgorithm(_, _) } override string getName() { exists(string rawName | - result = super.normalizeName(rawName) and this = hashlibMemberHashAlgorithm(rawName) + result = super.normalizeName(rawName) and this = hashlibMemberHashAlgorithm(_, rawName) ) } + + API::Node getModule() { this = hashlibMemberHashAlgorithm(result, _) } + } + + API::CallNode getHashlibDigestInstance(HashAlgorithm hash) { + exists(HashlibMemberAlgorithm memberAlgorithm | result = memberAlgorithm.getModule().getACall() and hash = memberAlgorithm) + or + exists(HashlibNewHashAlgorithm newHashAlgorithm | result = newHashAlgorithm.getInstance() and hash = newHashAlgorithm) + or + exists(HashlibFileDigestAlgorithm fileDigestAlgorithm | result = fileDigestAlgorithm.getInstance() and hash = fileDigestAlgorithm) + } + + API::CallNode getHashlibDigestOperation(HashAlgorithm hash, API::CallNode instance) { + instance = getHashlibDigestInstance(hash) and + result = instance.getAMethodCall("update") + } + + class HashlibDigestOperation extends CryptographicOperation { + HashlibDigestOperation() { + this = getHashlibDigestOperation(_, _) + } + + override DataFlow::Node getInitialisation() { this = getHashlibDigestOperation(_, result) } + + override HashAlgorithm getAlgorithm() { this = getHashlibDigestOperation(result, _) } + + override DataFlow::Node getAnInput() { result = this.getArg(0) or result = this.getArgByName("data") } } } @@ -160,12 +194,22 @@ module KDF { override string getName() { result = super.normalizeName("pbkdf2_hmac") } + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { result = this.getArg(1) or result = this.getArgByName("password") } + override DataFlow::Node getIterationSizeSrc() { exists(API::Node it | hashlibPBDKF2HMACKDFRequiredParams(this, _, _, it) | result = Utils::getUltimateSrcFromApiNode(it) ) } + override DataFlow::Node getSaltConfigSink() { + exists(API::Node s | hashlibPBDKF2HMACKDFRequiredParams(this, _, s, _) | + result = s.asSink() + ) + } + override DataFlow::Node getSaltConfigSrc() { exists(API::Node s | hashlibPBDKF2HMACKDFRequiredParams(this, _, s, _) | result = Utils::getUltimateSrcFromApiNode(s) @@ -179,9 +223,7 @@ module KDF { } override DataFlow::Node getDerivedKeySizeSrc() { - exists(API::Node dk | hashlibPBDKF2HMACKDFOptionalParams(this, dk) | - result = Utils::getUltimateSrcFromApiNode(dk) - ) + result = Utils::getUltimateSrcFromApiNode(this.getParameter(4, "dklen")) } // TODO: if DK is none, then the length is based on the hash type, if hash length not known, must call this unknown @@ -203,29 +245,50 @@ module KDF { override predicate requiresMemoryCost() { none() } } + // NOTE: Only finds the params of `scrypt` that are non-optional + // maxmem and dk_len are optional, i.e., can be None or omitted, and if addressed in this predicate + // would result in an unsatisfiable predicate. + predicate hashlibScryptKDFRequiredParams( + HashlibScryptOperation kdf, API::Node nParam, API::Node rParam, + API::Node pParam, API::Node saltParam + ) { + kdf.getKeywordParameter("n") = nParam and + kdf.getKeywordParameter("r") = rParam and + kdf.getKeywordParameter("p") = pParam and + kdf.getKeywordParameter("salt") = saltParam + } + // TODO: better modeling of scrypt /** * Identifies key derivation function hashlib.scrypt accesses. */ - class HashlibScryptAlgorithm extends KeyDerivationAlgorithm, KeyDerivationOperation { - HashlibScryptAlgorithm() { this = API::moduleImport("hashlib").getMember("scrypt").getACall() } + class HashlibScryptOperation extends KeyDerivationAlgorithm, KeyDerivationOperation { + HashlibScryptOperation() { this = API::moduleImport("hashlib").getMember("scrypt").getACall() } override string getName() { result = super.normalizeName("scrypt") } + override DataFlow::Node getInitialisation() { result = this } + + override DataFlow::Node getAnInput() { result = this.getArg(0) or result = this.getArgByName("password") } + override DataFlow::Node getIterationSizeSrc() { none() } + override DataFlow::Node getSaltConfigSink() { + exists(API::Node s | hashlibScryptKDFRequiredParams(this, _, _, _, s) | + result = s.asSink() + ) + } + override DataFlow::Node getSaltConfigSrc() { - // TODO: need to address getting salt from params, unsure how this works in CodeQL - // since the signature is defined as hashlib.scrypt(password, *, salt, n, r, p, maxmem=0, dklen=64) - // What position is 'salt' then such that we can reliably extract it? - none() + exists(API::Node s | hashlibScryptKDFRequiredParams(this, _, _, _, s) | + result = Utils::getUltimateSrcFromApiNode(s) + ) } override DataFlow::Node getHashConfigSrc() { none() } override DataFlow::Node getDerivedKeySizeSrc() { - //TODO: see comment for getSaltConfigSrc above - none() + result = Utils::getUltimateSrcFromApiNode(this.getKeywordParameter("dklen")) } override KeyDerivationAlgorithm getAlgorithm() { result = this } From f8b610632767731d53f5913140e0621b63da588c Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Fri, 1 May 2026 16:56:23 +0100 Subject: [PATCH 10/13] have a crack at modelling argon2-cffi --- .../cryptography/modules/Argon2CffiModule.qll | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 python/ql/lib/experimental/cryptography/modules/Argon2CffiModule.qll diff --git a/python/ql/lib/experimental/cryptography/modules/Argon2CffiModule.qll b/python/ql/lib/experimental/cryptography/modules/Argon2CffiModule.qll new file mode 100644 index 000000000000..504c628f102f --- /dev/null +++ b/python/ql/lib/experimental/cryptography/modules/Argon2CffiModule.qll @@ -0,0 +1,137 @@ +import python +import semmle.python.ApiGraphs +import experimental.cryptography.CryptoArtifact +import experimental.cryptography.CryptoAlgorithmNames +import experimental.cryptography.utils.CallCfgNodeWithTarget +private import experimental.cryptography.utils.Utils as Utils + +/** + * Provides models for the `argon2-cffi` PyPI package. + * See https://argon2-cffi.readthedocs.io/en/stable/. + */ +private module Argon2Cffi { + API::CallNode getAPasswordHasher() { + result = API::moduleImport("argon2") + .getMember("PasswordHasher") + .getACall() + } + + DataFlow::LocalSourceNode getAnArgonType(string fullName, string member) { + result = API::moduleImport("argon2") + .getMember("low_level") + .getMember("Type") + .getMember(member) + .asSource() + and ( + (fullName = "ARGON2D" and member = "D") + or + (fullName = "ARGON2I" and member = "I") + or + (fullName = "ARGON2ID" and member = "ID") + ) + } + + class ArgonType extends DataFlow::LocalSourceNode { + ArgonType() { this = getAnArgonType(_, _) } + string getName() { this = getAnArgonType(result, _) } + } + + // note: password hashers instantiated using `from_parameters` are not modelled (yet) + class PasswordHasher extends KeyDerivationAlgorithm { + PasswordHasher() { this = getAPasswordHasher() } + + API::Node getTypeParameter() { + result = this.(API::CallNode).getParameter(6, "type") + } + + override string getName() { + if exists(this.getTypeParameter()) then exists( + ArgonType type | type.flowsTo(this.getTypeParameter().asSink()) | result = type.getName() + ) + else result = "ARGON2ID" + } + } + + abstract class PasswordHasherOperation extends KeyDerivationOperation { + PasswordHasher instance; + + PasswordHasherOperation() { + this = instance.(API::CallNode).getAMethodCall(["hash", "verify"]) + } + + override PasswordHasher getAlgorithm() { result = instance } + + override DataFlow::Node getInitialisation() { result = instance } + + override predicate requiresHash() { none() } + + override predicate requiresMode() { none() } + + // although Argon is salted, the salt parameter is optional as the library generates one for you + override predicate requiresSalt() { none() } + + override predicate requiresIteration() { none() } + + override predicate requiresLanes() { none() } + + override predicate requiresMemoryCost() { none() } + + override DataFlow::Node getSaltConfigSink() { + result = this.getSaltConfigSink(_) + } + + private DataFlow::Node getSaltConfigSink(API::Node apiNode) { + result = apiNode.asSink() and + apiNode = this.(API::CallNode).getKeywordParameter("salt") + } + + override DataFlow::Node getSaltConfigSrc() { + result = this.getSaltConfigSrc(_) + } + + private DataFlow::Node getSaltConfigSrc(API::Node apiNode) { + exists(this.getSaltConfigSink(apiNode)) and + result = Utils::getUltimateSrcFromApiNode(apiNode) + } + + override DataFlow::Node getHashConfigSrc() { none() } + + override DataFlow::Node getIterationSizeSrc() { + result = Utils::getUltimateSrcFromApiNode(instance.(API::CallNode).getParameter(0, "time_cost")) + } + + override DataFlow::Node getMemoryCostConfigSrc() { + result = Utils::getUltimateSrcFromApiNode(instance.(API::CallNode).getParameter(1, "memory_cost")) + } + + override DataFlow::Node getLanesConfigSrc() { + result = Utils::getUltimateSrcFromApiNode(instance.(API::CallNode).getParameter(2, "parallelism")) + } + + override DataFlow::Node getDerivedKeySizeSrc() { + result = Utils::getUltimateSrcFromApiNode(instance.(API::CallNode).getParameter(3, "hash_len")) + } + + override DataFlow::Node getModeSrc() { none() } + } + + class HashOperation extends PasswordHasherOperation { + HashOperation() { + this = instance.(API::CallNode).getAMethodCall("hash") + } + + override DataFlow::Node getAnInput() { + result = this.(API::CallNode).getArg(0) or result = this.(API::CallNode).getArgByName("password") + } + } + + class VerifyOperation extends PasswordHasherOperation { + VerifyOperation() { + this = instance.(API::CallNode).getAMethodCall("verify") + } + + override DataFlow::Node getAnInput() { + result = this.(API::CallNode).getArg(1) or result = this.(API::CallNode).getArgByName("password") + } + } +} \ No newline at end of file From cfb733b33d7b7573a38272e84aa5c8845dd66658 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Fri, 1 May 2026 17:07:36 +0100 Subject: [PATCH 11/13] un-private module --- .../lib/experimental/cryptography/modules/Argon2CffiModule.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/lib/experimental/cryptography/modules/Argon2CffiModule.qll b/python/ql/lib/experimental/cryptography/modules/Argon2CffiModule.qll index 504c628f102f..71ecc05a741b 100644 --- a/python/ql/lib/experimental/cryptography/modules/Argon2CffiModule.qll +++ b/python/ql/lib/experimental/cryptography/modules/Argon2CffiModule.qll @@ -9,7 +9,7 @@ private import experimental.cryptography.utils.Utils as Utils * Provides models for the `argon2-cffi` PyPI package. * See https://argon2-cffi.readthedocs.io/en/stable/. */ -private module Argon2Cffi { +module Argon2Cffi { API::CallNode getAPasswordHasher() { result = API::moduleImport("argon2") .getMember("PasswordHasher") From 69ed2a8394179952e216141642205e2ac49ff43d Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Fri, 1 May 2026 17:07:58 +0100 Subject: [PATCH 12/13] import argon2-cffi module from concepts --- python/ql/lib/experimental/cryptography/Concepts.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/ql/lib/experimental/cryptography/Concepts.qll b/python/ql/lib/experimental/cryptography/Concepts.qll index e7befd8f2395..2108807934cf 100644 --- a/python/ql/lib/experimental/cryptography/Concepts.qll +++ b/python/ql/lib/experimental/cryptography/Concepts.qll @@ -4,3 +4,5 @@ import semmle.python.ApiGraphs import experimental.cryptography.modules.stdlib.HashlibModule as HashLibModule import experimental.cryptography.modules.stdlib.HmacModule as HMacModule import experimental.cryptography.modules.CryptographyModule as CryptographyModule +// contents are wrapped in a module, so 'as' not needed +import experimental.cryptography.modules.Argon2CffiModule From 1801140c9a9130486e9566dc7fb94c3b10f895e7 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Fri, 1 May 2026 17:08:06 +0100 Subject: [PATCH 13/13] use `this.` --- .../experimental/cryptography/modules/CryptographyModule.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll index 093b35bb1f3e..1cfb26449332 100644 --- a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll +++ b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll @@ -217,7 +217,7 @@ module KDF { } private DataFlow::Node getSaltConfigSrc(API::Node apiNode) { - exists(getSaltConfigSink(apiNode)) and + exists(this.getSaltConfigSink(apiNode)) and result = Utils::getUltimateSrcFromApiNode(apiNode) }