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 } 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 diff --git a/python/ql/lib/experimental/cryptography/CryptoAlgorithmNames.qll b/python/ql/lib/experimental/cryptography/CryptoAlgorithmNames.qll index 937b45b7aacc..8ce9d25c193c 100644 --- a/python/ql/lib/experimental/cryptography/CryptoAlgorithmNames.qll +++ b/python/ql/lib/experimental/cryptography/CryptoAlgorithmNames.qll @@ -104,9 +104,11 @@ 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' 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 e8939c981113..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,12 +134,17 @@ 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() } + + DataFlow::Node getMemoryCostConfigSrc() { none() } + // TODO: get encryption algorithm for CBC-based KDF? DataFlow::Node getDerivedKeySizeSrc() { none() } @@ -147,6 +157,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(); } @@ -167,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 @@ -192,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 { } @@ -222,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() @@ -232,21 +250,38 @@ abstract class BlockMode extends CryptographicAlgorithm { /** * Gets the source of the IV configuration. */ - abstract DataFlow::Node getIVorNonce(); + abstract DataFlow::Node getIVOrNonceSrc(); - final predicate hasIVorNonce() { exists(this.getIVorNonce()) } + /** + * Gets the sink of the IV configuration. + */ + abstract DataFlow::Node getIVOrNonceSink(); + + 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/Argon2CffiModule.qll b/python/ql/lib/experimental/cryptography/modules/Argon2CffiModule.qll new file mode 100644 index 000000000000..71ecc05a741b --- /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/. + */ +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 diff --git a/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll b/python/ql/lib/experimental/cryptography/modules/CryptographyModule.qll index 0831d625d803..1cfb26449332 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 ["concatkdf", "hkdf", "kbkdf", "pbkdf2", "scrypt", "x963kdf"] and - algName in [ - "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,72 +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"] + this.getAlgorithm().getKDFName() in ["PBKDF2HMAC", "CONCATKDFHMAC", "HKDF", "SCRYPT", "ARGON2D", "ARGON2I", "ARGON2ID"] } - override predicate requiresIteration() { this.getAlgorithm().getKDFName() in ["PBKDF2HMAC"] } + override predicate requiresIteration() { this.getAlgorithm().getKDFName() in ["PBKDF2HMAC", "ARGON2D", "ARGON2I", "ARGON2ID"] } + + override predicate requiresLanes() { this.getAlgorithm().getKDFName() in ["ARGON2D", "ARGON2I", "ARGON2ID"] } + + override predicate requiresMemoryCost() { this.getAlgorithm().getKDFName() in ["ARGON2D", "ARGON2I", "ARGON2ID"] } 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() in ["ARGON2D", "ARGON2I", "ARGON2ID"] + then result = Utils::getUltimateSrcFromApiNode(instance.getKeywordParameter("iterations")) + else + // ASSUMPTION: ONLY EVER in arg 3 in PBKDF2HMAC + 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() in ["ARGON2D", "ARGON2I", "ARGON2ID"] + then apiNode = instance.getKeywordParameter("salt") // SCRYPT has it in arg 1 - if this.getAlgorithm().getKDFName() = "SCRYPT" - then result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "salt")) + else if this.getAlgorithm().getKDFName() = "SCRYPT" + 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(this.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 + // ASSUMPTION: ONLY EVER in keyword parameter + result = Utils::getUltimateSrcFromApiNode(instance.getKeywordParameter("lanes")) + } + + override DataFlow::Node getMemoryCostConfigSrc() { + this.requiresMemoryCost() and + // ASSUMPTION: ONLY EVER in keyword parameter + result = Utils::getUltimateSrcFromApiNode(instance.getKeywordParameter("memory_cost")) } // TODO: get encryption algorithm for CBC-based KDF? override DataFlow::Node getDerivedKeySizeSrc() { - if this.getAlgorithm().getKDFName() in ["KBKDFHMAC", "KBKDFCMAC"] - then result = Utils::getUltimateSrcFromApiNode(this.getParameter(2, "length")) - else result = Utils::getUltimateSrcFromApiNode(this.getParameter(1, "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(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")) } } } @@ -179,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 @@ -198,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 } } @@ -229,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) } @@ -237,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) ) } @@ -266,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) { @@ -290,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 @@ -310,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) } + } } /** @@ -323,7 +513,7 @@ module Encryption { ) } - class CryptographyFernet extends SymmetricPadding, BlockMode, SymmetricEncryptionAlgorithm, + class CryptographyFernet extends SymmetricPadding, BlockModeInstance, SymmetricEncryptionAlgorithm, SymmetricCipher { CryptographyFernet() { this = fernetConstructor() } @@ -344,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) { @@ -361,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) { @@ -393,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) | @@ -402,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) ) } } @@ -463,7 +666,6 @@ module Encryption { API::moduleImport("cryptography") .getMember("hazmat") .getMember("primitives") - .getMember("ciphers") .getMember("padding") .getMember(name) .getACall() and @@ -484,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") @@ -500,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")) } } @@ -639,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")) @@ -752,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")) } @@ -843,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() { @@ -872,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 @@ -907,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 @@ -939,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")) @@ -973,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 @@ -1008,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 346512e9a2db..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 @@ -197,31 +239,56 @@ module KDF { override predicate requiresSalt() { any() } override predicate requiresIteration() { any() } + + override predicate requiresLanes() { none() } + + 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 } @@ -233,5 +300,9 @@ module KDF { override predicate requiresSalt() { any() } override predicate requiresIteration() { none() } + + override predicate requiresLanes() { none() } + + override predicate requiresMemoryCost() { none() } } }