diff --git a/go/ql/lib/semmle/go/Concepts.qll b/go/ql/lib/semmle/go/Concepts.qll index 0f30519d0be..acb16b62d07 100644 --- a/go/ql/lib/semmle/go/Concepts.qll +++ b/go/ql/lib/semmle/go/Concepts.qll @@ -536,4 +536,71 @@ module Cryptography { class BlockMode = SC::BlockMode; class CryptographicAlgorithm = SC::CryptographicAlgorithm; + + /** A data flow node that initializes a hash algorithm. */ + abstract class HashAlgorithmInit extends DataFlow::Node { + /** Gets the hash algorithm being initialized. */ + abstract HashingAlgorithm getAlgorithm(); + } + + /** A data flow node that is an application of a hash algorithm. */ + abstract class HashOperation extends CryptographicOperation::Range { + override BlockMode getBlockMode() { none() } + } + + /** A data flow node that initializes an encryption algorithm. */ + abstract class EncryptionAlgorithmInit extends DataFlow::Node { + /** Gets the encryption algorithm being initialized. */ + abstract EncryptionAlgorithm getAlgorithm(); + } + + /** + * A data flow node that initializes a block cipher mode of operation, and + * may also propagate taint for encryption algorithms. + */ + abstract class BlockModeInit extends DataFlow::CallNode { + /** Gets the block cipher mode of operation being initialized. */ + abstract BlockMode getMode(); + + /** Gets a step propagating the encryption algorithm through this call. */ + abstract predicate step(DataFlow::Node node1, DataFlow::Node node2); + } + + /** + * A data flow node that is an application of an encryption algorithm, where + * the encryption algorithm and the block cipher mode of operation (if there + * is one) have been initialized separately. + */ + abstract class EncryptionOperation extends CryptographicOperation::Range { + DataFlow::Node encryptionFlowTarget; + DataFlow::Node inputNode; + + override DataFlow::Node getInitialization() { + EncryptionFlow::flow(result, encryptionFlowTarget) + } + + override EncryptionAlgorithm getAlgorithm() { + result = this.getInitialization().(EncryptionAlgorithmInit).getAlgorithm() + } + + override DataFlow::Node getAnInput() { result = inputNode } + + override BlockMode getBlockMode() { + result = this.getInitialization().(BlockModeInit).getMode() + } + } + + /** + * An `EncryptionOperation` which is a method call where the encryption + * algorithm and block cipher mode of operation (if there is one) flow to the + * receiver and the input is an argument. + */ + abstract class EncryptionMethodCall extends EncryptionOperation instanceof DataFlow::CallNode { + int inputArg; + + EncryptionMethodCall() { + encryptionFlowTarget = super.getReceiver() and + inputNode = super.getArgument(inputArg) + } + } } diff --git a/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll b/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll index 9cc0235cbbb..0c56d8c7e6a 100644 --- a/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll +++ b/go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll @@ -7,58 +7,482 @@ import semmle.go.Concepts::Cryptography private import codeql.concepts.internal.CryptoAlgorithmNames /** - * A cryptographic operation from the `crypto/md5` package. + * A data flow call node that is an application of a hash operation where the + * hash algorithm is defined in any earlier initialization node, and the input + * is the first argument of the call. */ -private module CryptoMd5 { - private class Md5 extends CryptographicOperation::Range instanceof DataFlow::CallNode { - Md5() { this.getTarget().hasQualifiedName("crypto/md5", ["New", "Sum"]) } +abstract class DirectHashOperation extends HashOperation instanceof DataFlow::CallNode { + override DataFlow::Node getInitialization() { result = this } - override DataFlow::Node getInitialization() { result = this } + override DataFlow::Node getAnInput() { result = super.getArgument(0) } +} - override CryptographicAlgorithm getAlgorithm() { result.matchesName("MD5") } +private module HashConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof HashAlgorithmInit } + + predicate isSink(DataFlow::Node sink) { any() } + + predicate observeDiffInformedIncrementalMode() { any() } +} + +/** Tracks the flow of hash algorithms. */ +module HashFlow = DataFlow::Global; + +/** + * A data flow node that initializes a block mode and propagates the encryption + * algorithm from the first argument to the receiver. + */ +abstract class StdLibNewEncrypter extends BlockModeInit { + override predicate step(DataFlow::Node node1, DataFlow::Node node2) { + node1 = this.getArgument(0) and + node2 = this.getResult(0) + } +} + +private module EncryptionConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source instanceof EncryptionAlgorithmInit or + source instanceof BlockModeInit + } + + predicate isSink(DataFlow::Node sink) { any() } + + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + any(BlockModeInit nbcm).step(node1, node2) + } + + predicate observeDiffInformedIncrementalMode() { any() } +} + +/** + * Tracks algorithms and block cipher modes of operation used for encryption. + */ +module EncryptionFlow = DataFlow::Global; + +private module Crypto { + private module Aes { + private class NewCipher extends EncryptionAlgorithmInit { + NewCipher() { + exists(Function f | this = f.getACall().getResult(0) | + f.hasQualifiedName("crypto/aes", "NewCipher") + ) + } + + override EncryptionAlgorithm getAlgorithm() { result.matchesName("AES") } + } + } + + private module Des { + private class NewCipher extends EncryptionAlgorithmInit { + NewCipher() { + exists(Function f | this = f.getACall().getResult(0) | + f.hasQualifiedName("crypto/des", "NewCipher") + ) + } + + override EncryptionAlgorithm getAlgorithm() { result.matchesName("DES") } + } + + private class NewTripleDESCipher extends EncryptionAlgorithmInit { + NewTripleDESCipher() { + exists(Function f | this = f.getACall().getResult(0) | + f.hasQualifiedName("crypto/des", "NewTripleDESCipher") + ) + } + + override EncryptionAlgorithm getAlgorithm() { result.matchesName("TRIPLEDES") } + } + } + + private module Md5 { + private class Sum extends DirectHashOperation instanceof DataFlow::CallNode { + Sum() { this.getTarget().hasQualifiedName("crypto/md5", "Sum") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("MD5") } + } + + private class New extends HashAlgorithmInit instanceof DataFlow::CallNode { + New() { this.getTarget().hasQualifiedName("crypto/md5", "New") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("MD5") } + } + } + + private module Rc4 { + private class CipherXORKeyStream extends CryptographicOperation::Range instanceof DataFlow::CallNode + { + CipherXORKeyStream() { + this.(DataFlow::MethodCallNode) + .getTarget() + .hasQualifiedName("crypto/rc4", "Cipher", "XORKeyStream") + } + + override DataFlow::Node getInitialization() { result = this } + + override EncryptionAlgorithm getAlgorithm() { result.matchesName("RC4") } + + override DataFlow::Node getAnInput() { result = super.getArgument(1) } + + override BlockMode getBlockMode() { none() } + } + } + + /** + * Cryptographic operations from the `crypto/sha1` package. + */ + private module Sha1 { + private class Sum extends DirectHashOperation instanceof DataFlow::CallNode { + Sum() { this.getTarget().hasQualifiedName("crypto/sha1", "Sum") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA1") } + } + + private class New extends HashAlgorithmInit instanceof DataFlow::CallNode { + New() { this.getTarget().hasQualifiedName("crypto/sha1", "New") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA1") } + } + } + + /** + * Cryptographic operations from the `crypto/sha256` package. + */ + private module Sha256 { + private class Sum256 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum256() { this.getTarget().hasQualifiedName("crypto/sha256", "Sum256") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA256") } + } + + private class Sum224 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum224() { this.getTarget().hasQualifiedName("crypto/sha256", "Sum224") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA224") } + } + + private class New extends HashAlgorithmInit instanceof DataFlow::CallNode { + New() { this.getTarget().hasQualifiedName("crypto/sha256", "New") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA256") } + } + + private class New224 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New224() { this.getTarget().hasQualifiedName("crypto/sha256", "New224") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA224") } + } + } + + private module Sha3 { + private class Sum224 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum224() { this.getTarget().hasQualifiedName("crypto/sha3", "Sum224") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3224") } + } + + private class Sum256 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum256() { this.getTarget().hasQualifiedName("crypto/sha3", "Sum256") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3256") } + } + + private class Sum384 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum384() { this.getTarget().hasQualifiedName("crypto/sha3", "Sum384") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3384") } + } + + private class Sum512 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum512() { this.getTarget().hasQualifiedName("crypto/sha3", "Sum512") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3512") } + } + + private class SumShake128 extends DirectHashOperation instanceof DataFlow::CallNode { + SumShake128() { this.getTarget().hasQualifiedName("crypto/sha3", "SumSHAKE128") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHAKE128") } + } + + private class SumShake256 extends DirectHashOperation instanceof DataFlow::CallNode { + SumShake256() { this.getTarget().hasQualifiedName("crypto/sha3", "SumSHAKE256") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHAKE256") } + } + + private class New224 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New224() { this.getTarget().hasQualifiedName("crypto/sha3", "New224") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3224") } + } + + private class New256 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New256() { this.getTarget().hasQualifiedName("crypto/sha3", "New256") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3256") } + } + + private class New384 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New384() { this.getTarget().hasQualifiedName("crypto/sha3", "New384") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3384") } + } + + private class New512 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New512() { this.getTarget().hasQualifiedName("crypto/sha3", "New512") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA3512") } + } + + private class NewShake128 extends HashAlgorithmInit instanceof DataFlow::CallNode { + NewShake128() { + this.getTarget().hasQualifiedName("crypto/sha3", ["NewCSHAKE128", "NewSHAKE128"]) + } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHAKE128") } + } + + private class NewShake256 extends HashAlgorithmInit instanceof DataFlow::CallNode { + NewShake256() { + this.getTarget().hasQualifiedName("crypto/sha3", ["NewCSHAKE256", "NewSHAKE256"]) + } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHAKE256") } + } + + private class ShakeWrite extends HashOperation instanceof DataFlow::MethodCallNode { + ShakeWrite() { this.getTarget().hasQualifiedName("crypto/sha3", "SHAKE", "Write") } + + override HashAlgorithmInit getInitialization() { HashFlow::flow(result, super.getReceiver()) } + + override HashingAlgorithm getAlgorithm() { result = this.getInitialization().getAlgorithm() } + + override DataFlow::Node getAnInput() { result = super.getArgument(0) } + } + } + + private module Sha512 { + private class Sum384 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum384() { this.getTarget().hasQualifiedName("crypto/sha512", "Sum384") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA384") } + } + + private class Sum512 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum512() { this.getTarget().hasQualifiedName("crypto/sha512", "Sum512") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA512") } + } + + private class Sum512_224 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum512_224() { this.getTarget().hasQualifiedName("crypto/sha512", "Sum512_224") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA512224") } + } + + private class Sum512_256 extends DirectHashOperation instanceof DataFlow::CallNode { + Sum512_256() { this.getTarget().hasQualifiedName("crypto/sha512", "Sum512_256") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA512256") } + } + + private class New extends HashAlgorithmInit instanceof DataFlow::CallNode { + New() { this.getTarget().hasQualifiedName("crypto/sha512", "New") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA512") } + } + + private class New384 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New384() { this.getTarget().hasQualifiedName("crypto/sha512", "New384") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA384") } + } + + private class New512_224 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New512_224() { this.getTarget().hasQualifiedName("crypto/sha512", "New512_224") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA512224") } + } + + private class New512_256 extends HashAlgorithmInit instanceof DataFlow::CallNode { + New512_256() { this.getTarget().hasQualifiedName("crypto/sha512", "New512_256") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("SHA512256") } + } + } + + private module Cipher { + private class NewCBCEncrypter extends StdLibNewEncrypter { + NewCBCEncrypter() { this.getTarget().hasQualifiedName("crypto/cipher", "NewCBCEncrypter") } + + override BlockMode getMode() { result = "CBC" } + } + + private class NewCFBEncrypter extends StdLibNewEncrypter { + NewCFBEncrypter() { this.getTarget().hasQualifiedName("crypto/cipher", "NewCFBEncrypter") } + + override BlockMode getMode() { result = "CFB" } + } + + private class NewCTR extends StdLibNewEncrypter { + NewCTR() { this.getTarget().hasQualifiedName("crypto/cipher", "NewCTR") } + + override BlockMode getMode() { result = "CTR" } + } + + private class NewGCM extends StdLibNewEncrypter { + NewGCM() { + exists(string name | this.getTarget().hasQualifiedName("crypto/cipher", name) | + name = ["NewGCM", "NewGCMWithNonceSize", "NewGCMWithRandomNonce", "NewGCMWithTagSize"] + ) + } + + override BlockMode getMode() { result = "GCM" } + } + + private class NewOFB extends StdLibNewEncrypter { + NewOFB() { this.getTarget().hasQualifiedName("crypto/cipher", "NewOFB") } + + override BlockMode getMode() { result = "OFB" } + } + + private class AeadSeal extends EncryptionMethodCall { + AeadSeal() { + this.(DataFlow::MethodCallNode) + .getTarget() + .hasQualifiedName("crypto/cipher", "AEAD", "Seal") and + inputArg = 2 + } + } + + private class BlockEncrypt extends EncryptionMethodCall { + BlockEncrypt() { + this.(DataFlow::MethodCallNode) + .getTarget() + .hasQualifiedName("crypto/cipher", "Block", "Encrypt") and + inputArg = 1 + } + } + + private class BlockModeCryptBlocks extends EncryptionMethodCall { + BlockModeCryptBlocks() { + this.(DataFlow::MethodCallNode) + .getTarget() + .hasQualifiedName("crypto/cipher", "BlockMode", "CryptBlocks") and + inputArg = 1 + } + } + + private class StreamXORKeyStream extends EncryptionMethodCall { + StreamXORKeyStream() { + this.(DataFlow::MethodCallNode) + .getTarget() + .hasQualifiedName("crypto/cipher", "Stream", "XORKeyStream") and + inputArg = 1 + } + } + + private class StreamReader extends EncryptionOperation { + StreamReader() { + lookThroughPointerType(this.getType()).hasQualifiedName("crypto/cipher", "StreamReader") and + exists(DataFlow::Write w, DataFlow::Node base, Field f | + f.hasQualifiedName("crypto/cipher", "StreamReader", "S") and + w.writesField(base, f, encryptionFlowTarget) and + DataFlow::localFlow(base, this) + ) and + exists(DataFlow::Write w, DataFlow::Node base, Field f | + f.hasQualifiedName("crypto/cipher", "StreamReader", "R") and + w.writesField(base, f, inputNode) and + DataFlow::localFlow(base, this) + ) + } + } + + /** + * Limitation: StreamWriter wraps a Writer, so we need to look forward to + * where the Writer is written to. Currently this is done using local flow, + * so it only works within one function. + */ + private class StreamWriter extends EncryptionOperation { + StreamWriter() { + lookThroughPointerType(this.getType()).hasQualifiedName("crypto/cipher", "StreamWriter") and + inputNode = this and + exists(DataFlow::Write w, DataFlow::Node base, Field f | + w.writesField(base, f, encryptionFlowTarget) and + f.hasQualifiedName("crypto/cipher", "StreamWriter", "S") + | + base = this or + TaintTracking::localTaint(base, this.(DataFlow::PostUpdateNode).getPreUpdateNode()) + ) + } + } + } +} + +private module Hash { + private class HashSum extends HashOperation instanceof DataFlow::MethodCallNode { + HashSum() { this.getTarget().implements("hash", "Hash", "Sum") } + + override HashAlgorithmInit getInitialization() { HashFlow::flow(result, super.getReceiver()) } + + override HashingAlgorithm getAlgorithm() { result = this.getInitialization().getAlgorithm() } override DataFlow::Node getAnInput() { result = super.getArgument(0) } - - // not relevant for md5 - override BlockMode getBlockMode() { none() } } } +private DataFlow::Node getANonIoWriterPredecessor(DataFlow::Node node) { + node.getType().implements("io", "Writer") and + exists(DataFlow::Node pre | TaintTracking::localTaintStep(pre, node) | + if pre.getType().implements("io", "Writer") + then result = getANonIoWriterPredecessor(pre) + else result = pre + ) +} + /** - * A cryptographic operation from the `crypto/sha1` package. + * Taint flowing to an `io.Writer` (such as `hash.Hash` or `*sha3.SHAKE`) via + * its implementation of the `io.Writer` interface. */ -class Sha1 extends CryptographicOperation::Range instanceof DataFlow::CallNode { - Sha1() { this.getTarget().hasQualifiedName("crypto/sha1", ["New", "Sum"]) } +private class FlowToIoWriter extends HashOperation instanceof DataFlow::Node { + HashAlgorithmInit init; + DataFlow::Node input; - override Expr getInput() { result = this.getArgument(0).asExpr() } - - override CryptographicAlgorithm getAlgorithm() { - result.matchesName(this.getTarget().getPackage().getName()) + FlowToIoWriter() { + this.getType().implements("io", "Writer") and + HashFlow::flow(init, this) and + // If we have `h.Write(taint)` or `io.WriteString(h, taint)` then it's + // the post-update node of `h` that gets tainted. + exists(DataFlow::PostUpdateNode pun | pun.getPreUpdateNode() = this | + input = getANonIoWriterPredecessor(pun) + ) } + + override HashAlgorithmInit getInitialization() { result = init } + + override HashingAlgorithm getAlgorithm() { result = this.getInitialization().getAlgorithm() } + + override DataFlow::Node getAnInput() { result = input } } /** - * A cryptographic operation from the `crypto/des` package. + * Currently only weak algorithms from the `golang.org/x/crypto` module are + * modeled here. */ -class Des extends CryptographicOperation::Range instanceof DataFlow::CallNode { - Des() { this.getTarget().hasQualifiedName("crypto/des", ["NewCipher", "NewTripleDESCipher"]) } +private module GolangOrgXCrypto { + private module Md4 { + private class New extends HashAlgorithmInit instanceof DataFlow::CallNode { + New() { this.getTarget().hasQualifiedName("golang.org/x/crypto/md4", "New") } - override Expr getInput() { result = this.getArgument(0).asExpr() } + override HashingAlgorithm getAlgorithm() { result.matchesName("MD4") } + } + } - override CryptographicAlgorithm getAlgorithm() { - result.matchesName(this.getTarget().getPackage().getName()) - } -} - -/** - * A cryptographic operation from the `crypto/rc4` package. - */ -class Rc4 extends CryptographicOperation::Range instanceof DataFlow::CallNode { - Rc4() { this.getTarget().hasQualifiedName("crypto/rc4", "NewCipher") } - - override Expr getInput() { result = this.getArgument(0).asExpr() } - - override CryptographicAlgorithm getAlgorithm() { - result.matchesName(this.getTarget().getPackage().getName()) + private module Ripemd160 { + private class New extends HashAlgorithmInit instanceof DataFlow::CallNode { + New() { this.getTarget().hasQualifiedName("golang.org/x/crypto/ripemd160", "New") } + + override HashingAlgorithm getAlgorithm() { result.matchesName("RIPEMD160") } + } } } diff --git a/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected b/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected index 6f40dfcc7ad..4fbf77dda62 100644 --- a/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected +++ b/go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected @@ -1,17 +1,83 @@ #select -| Crypto.go:21:25:21:27 | buf | Crypto.go:18:9:18:16 | password | Crypto.go:21:25:21:27 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:18:9:18:16 | password | Sensitive data | -| Crypto.go:24:10:24:12 | buf | Crypto.go:18:9:18:16 | password | Crypto.go:24:10:24:12 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:18:9:18:16 | password | Sensitive data | -| Crypto.go:27:16:27:18 | buf | Crypto.go:18:9:18:16 | password | Crypto.go:27:16:27:18 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:18:9:18:16 | password | Sensitive data | -| Crypto.go:30:11:30:13 | buf | Crypto.go:18:9:18:16 | password | Crypto.go:30:11:30:13 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:18:9:18:16 | password | Sensitive data | +| Crypto.go:36:21:36:28 | password | Crypto.go:36:21:36:28 | password | Crypto.go:36:21:36:28 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:36:21:36:28 | password | Sensitive data | +| Crypto.go:41:22:41:29 | password | Crypto.go:41:22:41:29 | password | Crypto.go:41:22:41:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:41:22:41:29 | password | Sensitive data | +| Crypto.go:46:22:46:29 | password | Crypto.go:46:22:46:29 | password | Crypto.go:46:22:46:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:46:22:46:29 | password | Sensitive data | +| Crypto.go:51:22:51:29 | password | Crypto.go:51:22:51:29 | password | Crypto.go:51:22:51:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:51:22:51:29 | password | Sensitive data | +| Crypto.go:56:22:56:29 | password | Crypto.go:56:22:56:29 | password | Crypto.go:56:22:56:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:56:22:56:29 | password | Sensitive data | +| Crypto.go:61:32:61:39 | password | Crypto.go:61:32:61:39 | password | Crypto.go:61:32:61:39 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:61:32:61:39 | password | Sensitive data | +| Crypto.go:66:30:66:37 | password | Crypto.go:66:30:66:37 | password | Crypto.go:66:30:66:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:66:30:66:37 | password | Sensitive data | +| Crypto.go:68:59:68:83 | call to NewReader | Crypto.go:68:75:68:82 | password | Crypto.go:68:59:68:83 | call to NewReader | $@ is used in a weak cryptographic algorithm. | Crypto.go:68:75:68:82 | password | Sensitive data | +| Crypto.go:72:10:72:24 | ctrStreamWriter [postupdate] | Crypto.go:72:43:72:50 | password | Crypto.go:72:10:72:24 | ctrStreamWriter [postupdate] | $@ is used in a weak cryptographic algorithm. | Crypto.go:72:43:72:50 | password | Sensitive data | +| Crypto.go:78:30:78:37 | password | Crypto.go:78:30:78:37 | password | Crypto.go:78:30:78:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:78:30:78:37 | password | Sensitive data | +| Crypto.go:83:30:83:37 | password | Crypto.go:83:30:83:37 | password | Crypto.go:83:30:83:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:83:30:83:37 | password | Sensitive data | +| Crypto.go:91:21:91:33 | call to getPassword | Crypto.go:91:21:91:33 | call to getPassword | Crypto.go:91:21:91:33 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:91:21:91:33 | call to getPassword | Sensitive data | +| Crypto.go:96:22:96:34 | call to getPassword | Crypto.go:96:22:96:34 | call to getPassword | Crypto.go:96:22:96:34 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:96:22:96:34 | call to getPassword | Sensitive data | +| Crypto.go:101:22:101:34 | call to getPassword | Crypto.go:101:22:101:34 | call to getPassword | Crypto.go:101:22:101:34 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:101:22:101:34 | call to getPassword | Sensitive data | +| Crypto.go:106:22:106:29 | password | Crypto.go:106:22:106:29 | password | Crypto.go:106:22:106:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:106:22:106:29 | password | Sensitive data | +| Crypto.go:111:22:111:29 | password | Crypto.go:111:22:111:29 | password | Crypto.go:111:22:111:29 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:111:22:111:29 | password | Sensitive data | +| Crypto.go:116:32:116:44 | call to getPassword | Crypto.go:116:32:116:44 | call to getPassword | Crypto.go:116:32:116:44 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:116:32:116:44 | call to getPassword | Sensitive data | +| Crypto.go:121:30:121:42 | call to getPassword | Crypto.go:121:30:121:42 | call to getPassword | Crypto.go:121:30:121:42 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:121:30:121:42 | call to getPassword | Sensitive data | +| Crypto.go:123:59:123:88 | call to NewReader | Crypto.go:123:75:123:87 | call to getPassword | Crypto.go:123:59:123:88 | call to NewReader | $@ is used in a weak cryptographic algorithm. | Crypto.go:123:75:123:87 | call to getPassword | Sensitive data | +| Crypto.go:127:10:127:24 | ctrStreamWriter [postupdate] | Crypto.go:127:43:127:55 | call to getPassword | Crypto.go:127:10:127:24 | ctrStreamWriter [postupdate] | $@ is used in a weak cryptographic algorithm. | Crypto.go:127:43:127:55 | call to getPassword | Sensitive data | +| Crypto.go:133:30:133:37 | password | Crypto.go:133:30:133:37 | password | Crypto.go:133:30:133:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:133:30:133:37 | password | Sensitive data | +| Crypto.go:138:30:138:37 | password | Crypto.go:138:30:138:37 | password | Crypto.go:138:30:138:37 | password | $@ is used in a weak cryptographic algorithm. | Crypto.go:138:30:138:37 | password | Sensitive data | +| Crypto.go:198:22:198:34 | call to getPassword | Crypto.go:198:22:198:34 | call to getPassword | Crypto.go:198:22:198:34 | call to getPassword | $@ is used in a weak cryptographic algorithm. | Crypto.go:198:22:198:34 | call to getPassword | Sensitive data | +| Crypto.go:205:8:205:10 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:205:8:205:10 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | +| Crypto.go:206:10:206:12 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:206:10:206:12 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | +| Crypto.go:207:20:207:33 | passwordString | Crypto.go:207:20:207:33 | passwordString | Crypto.go:207:20:207:33 | passwordString | $@ is used in a weak cryptographic algorithm. | Crypto.go:207:20:207:33 | passwordString | Sensitive data | +| Crypto.go:208:10:208:12 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:208:10:208:12 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | +| Crypto.go:210:17:210:19 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:210:17:210:19 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | +| Crypto.go:211:11:211:13 | buf | Crypto.go:202:9:202:16 | password | Crypto.go:211:11:211:13 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:202:9:202:16 | password | Sensitive data | edges -| Crypto.go:18:9:18:16 | password | Crypto.go:21:25:21:27 | buf | provenance | | -| Crypto.go:18:9:18:16 | password | Crypto.go:24:10:24:12 | buf | provenance | | -| Crypto.go:18:9:18:16 | password | Crypto.go:27:16:27:18 | buf | provenance | | -| Crypto.go:18:9:18:16 | password | Crypto.go:30:11:30:13 | buf | provenance | | +| Crypto.go:68:75:68:82 | password | Crypto.go:68:59:68:83 | call to NewReader | provenance | MaD:1 | +| Crypto.go:72:27:72:51 | call to NewReader | Crypto.go:72:10:72:24 | ctrStreamWriter [postupdate] | provenance | MaD:2 | +| Crypto.go:72:43:72:50 | password | Crypto.go:72:27:72:51 | call to NewReader | provenance | MaD:1 | +| Crypto.go:123:75:123:87 | call to getPassword | Crypto.go:123:59:123:88 | call to NewReader | provenance | MaD:1 | +| Crypto.go:127:27:127:56 | call to NewReader | Crypto.go:127:10:127:24 | ctrStreamWriter [postupdate] | provenance | MaD:2 | +| Crypto.go:127:43:127:55 | call to getPassword | Crypto.go:127:27:127:56 | call to NewReader | provenance | MaD:1 | +| Crypto.go:202:9:202:16 | password | Crypto.go:205:8:205:10 | buf | provenance | | +| Crypto.go:202:9:202:16 | password | Crypto.go:206:10:206:12 | buf | provenance | | +| Crypto.go:202:9:202:16 | password | Crypto.go:208:10:208:12 | buf | provenance | | +| Crypto.go:202:9:202:16 | password | Crypto.go:210:17:210:19 | buf | provenance | | +| Crypto.go:202:9:202:16 | password | Crypto.go:211:11:211:13 | buf | provenance | | +models +| 1 | Summary: bytes; ; false; NewReader; ; ; Argument[0]; ReturnValue; taint; manual | +| 2 | Summary: io; ; false; Copy; ; ; Argument[1]; Argument[0]; taint; manual | nodes -| Crypto.go:18:9:18:16 | password | semmle.label | password | -| Crypto.go:21:25:21:27 | buf | semmle.label | buf | -| Crypto.go:24:10:24:12 | buf | semmle.label | buf | -| Crypto.go:27:16:27:18 | buf | semmle.label | buf | -| Crypto.go:30:11:30:13 | buf | semmle.label | buf | +| Crypto.go:36:21:36:28 | password | semmle.label | password | +| Crypto.go:41:22:41:29 | password | semmle.label | password | +| Crypto.go:46:22:46:29 | password | semmle.label | password | +| Crypto.go:51:22:51:29 | password | semmle.label | password | +| Crypto.go:56:22:56:29 | password | semmle.label | password | +| Crypto.go:61:32:61:39 | password | semmle.label | password | +| Crypto.go:66:30:66:37 | password | semmle.label | password | +| Crypto.go:68:59:68:83 | call to NewReader | semmle.label | call to NewReader | +| Crypto.go:68:75:68:82 | password | semmle.label | password | +| Crypto.go:72:10:72:24 | ctrStreamWriter [postupdate] | semmle.label | ctrStreamWriter [postupdate] | +| Crypto.go:72:27:72:51 | call to NewReader | semmle.label | call to NewReader | +| Crypto.go:72:43:72:50 | password | semmle.label | password | +| Crypto.go:78:30:78:37 | password | semmle.label | password | +| Crypto.go:83:30:83:37 | password | semmle.label | password | +| Crypto.go:91:21:91:33 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:96:22:96:34 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:101:22:101:34 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:106:22:106:29 | password | semmle.label | password | +| Crypto.go:111:22:111:29 | password | semmle.label | password | +| Crypto.go:116:32:116:44 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:121:30:121:42 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:123:59:123:88 | call to NewReader | semmle.label | call to NewReader | +| Crypto.go:123:75:123:87 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:127:10:127:24 | ctrStreamWriter [postupdate] | semmle.label | ctrStreamWriter [postupdate] | +| Crypto.go:127:27:127:56 | call to NewReader | semmle.label | call to NewReader | +| Crypto.go:127:43:127:55 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:133:30:133:37 | password | semmle.label | password | +| Crypto.go:138:30:138:37 | password | semmle.label | password | +| Crypto.go:198:22:198:34 | call to getPassword | semmle.label | call to getPassword | +| Crypto.go:202:9:202:16 | password | semmle.label | password | +| Crypto.go:205:8:205:10 | buf | semmle.label | buf | +| Crypto.go:206:10:206:12 | buf | semmle.label | buf | +| Crypto.go:207:20:207:33 | passwordString | semmle.label | passwordString | +| Crypto.go:208:10:208:12 | buf | semmle.label | buf | +| Crypto.go:210:17:210:19 | buf | semmle.label | buf | +| Crypto.go:211:11:211:13 | buf | semmle.label | buf | subpaths diff --git a/go/ql/test/query-tests/Security/CWE-327/Crypto.go b/go/ql/test/query-tests/Security/CWE-327/Crypto.go index a58052df38d..1a7afc35cb9 100644 --- a/go/ql/test/query-tests/Security/CWE-327/Crypto.go +++ b/go/ql/test/query-tests/Security/CWE-327/Crypto.go @@ -1,55 +1,254 @@ package main import ( + "bytes" "crypto/aes" + "crypto/cipher" "crypto/des" "crypto/md5" "crypto/rc4" "crypto/sha1" "crypto/sha256" + "crypto/sha3" + "crypto/sha512" + "io" + "os" ) -func crypto() { - public := []byte("hello") +var dst []byte = make([]byte, 16) +var password []byte = []byte("123456") - password := []byte("123456") +const passwordString string = "correct horse battery staple" - // testing dataflow by passing into different variable +var public []byte = []byte("hello") + +func getPassword() []byte { + return []byte("123456") +} + +// Note that we do not alert on decryption as we may need to decrypt legacy formats + +func BlockCipherDes() { + // BAD, des is a weak crypto algorithm + block, _ := des.NewCipher(nil) + + block.Encrypt(dst, public) // $ CryptographicOperation="DES. init from line 33." + block.Encrypt(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." + block.Decrypt(dst, password) + + gcm1, _ := cipher.NewGCM(block) + gcm1.Seal(nil, nil, public, nil) // $ CryptographicOperation="DES. init from line 33." + gcm1.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." + gcm1.Open(nil, nil, password, nil) + + gcm2, _ := cipher.NewGCMWithNonceSize(block, 12) + gcm2.Seal(nil, nil, public, nil) // $ CryptographicOperation="DES. init from line 33." + gcm2.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." + gcm2.Open(nil, nil, password, nil) + + gcm3, _ := cipher.NewGCMWithRandomNonce(block) + gcm3.Seal(nil, nil, public, nil) // $ CryptographicOperation="DES. init from line 33." + gcm3.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." + gcm3.Open(nil, nil, password, nil) + + gcm4, _ := cipher.NewGCMWithTagSize(block, 12) + gcm4.Seal(nil, nil, public, nil) // $ CryptographicOperation="DES. init from line 33." + gcm4.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. init from line 33." + gcm4.Open(nil, nil, password, nil) + + cbcEncrypter := cipher.NewCBCEncrypter(block, nil) + cbcEncrypter.CryptBlocks(dst, public) // $ CryptographicOperation="DES. blockMode: CBC. init from lines 33,59." + cbcEncrypter.CryptBlocks(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CBC. init from lines 33,59." + cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, password) + + ctrStream := cipher.NewCTR(block, nil) + ctrStream.XORKeyStream(dst, public) // $ CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." + ctrStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." + + ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(password)} // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." + io.Copy(os.Stdout, ctrStreamReader) + + ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." + io.Copy(ctrStreamWriter, bytes.NewReader(password)) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CTR. init from lines 33,64." + + // deprecated + + cfbStream := cipher.NewCFBEncrypter(block, nil) + cfbStream.XORKeyStream(dst, public) // $ CryptographicOperation="DES. blockMode: CFB. init from lines 33,76." + cfbStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: CFB. init from lines 33,76." + cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, password) + + ofbStream := cipher.NewOFB(block, nil) + ofbStream.XORKeyStream(dst, public) // $ CryptographicOperation="DES. blockMode: OFB. init from lines 33,81." + ofbStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="DES. blockMode: OFB. init from lines 33,81." +} + +func BlockCipherTripleDes() { + // BAD, triple des is a weak crypto algorithm and password is sensitive data + block, _ := des.NewTripleDESCipher(nil) + + block.Encrypt(dst, public) // $ CryptographicOperation="TRIPLEDES. init from line 88." + block.Encrypt(dst, getPassword()) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." + block.Decrypt(dst, getPassword()) + + gcm1, _ := cipher.NewGCM(block) + gcm1.Seal(nil, nil, public, nil) // $ CryptographicOperation="TRIPLEDES. init from line 88." + gcm1.Seal(nil, nil, getPassword(), nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." + gcm1.Open(nil, nil, getPassword(), nil) + + gcm2, _ := cipher.NewGCMWithNonceSize(block, 12) + gcm2.Seal(nil, nil, public, nil) // $ CryptographicOperation="TRIPLEDES. init from line 88." + gcm2.Seal(nil, nil, getPassword(), nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." + gcm2.Open(nil, nil, getPassword(), nil) + + gcm3, _ := cipher.NewGCMWithRandomNonce(block) + gcm3.Seal(nil, nil, public, nil) // $ CryptographicOperation="TRIPLEDES. init from line 88." + gcm3.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." + gcm3.Open(nil, nil, password, nil) + + gcm4, _ := cipher.NewGCMWithTagSize(block, 12) + gcm4.Seal(nil, nil, public, nil) // $ CryptographicOperation="TRIPLEDES. init from line 88." + gcm4.Seal(nil, nil, password, nil) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. init from line 88." + gcm4.Open(nil, nil, password, nil) + + cbcEncrypter := cipher.NewCBCEncrypter(block, nil) + cbcEncrypter.CryptBlocks(dst, public) // $ CryptographicOperation="TRIPLEDES. blockMode: CBC. init from lines 114,88." + cbcEncrypter.CryptBlocks(dst, getPassword()) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CBC. init from lines 114,88." + cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, getPassword()) + + ctrStream := cipher.NewCTR(block, nil) + ctrStream.XORKeyStream(dst, public) // $ CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." + ctrStream.XORKeyStream(dst, getPassword()) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." + + ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(getPassword())} // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." + io.Copy(os.Stdout, ctrStreamReader) + + ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." + io.Copy(ctrStreamWriter, bytes.NewReader(getPassword())) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from lines 119,88." + + // deprecated + + cfbStream := cipher.NewCFBEncrypter(block, nil) + cfbStream.XORKeyStream(dst, public) // $ CryptographicOperation="TRIPLEDES. blockMode: CFB. init from lines 131,88." + cfbStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CFB. init from lines 131,88." + cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, password) + + ofbStream := cipher.NewOFB(block, nil) + ofbStream.XORKeyStream(dst, public) // $ CryptographicOperation="TRIPLEDES. blockMode: OFB. init from lines 136,88." + ofbStream.XORKeyStream(dst, password) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="TRIPLEDES. blockMode: OFB. init from lines 136,88." +} + +func BlockCipherAes() { + // GOOD, aes is a strong crypto algorithm + block, _ := aes.NewCipher(nil) + + block.Encrypt(dst, public) // $ CryptographicOperation="AES. init from line 143." + block.Encrypt(dst, password) // $ CryptographicOperation="AES. init from line 143." + block.Decrypt(dst, password) + + gcm1, _ := cipher.NewGCM(block) + gcm1.Seal(nil, nil, public, nil) // $ CryptographicOperation="AES. init from line 143." + gcm1.Seal(nil, nil, password, nil) // $ CryptographicOperation="AES. init from line 143." + gcm1.Open(nil, nil, password, nil) + + gcm2, _ := cipher.NewGCMWithNonceSize(block, 12) + gcm2.Seal(nil, nil, public, nil) // $ CryptographicOperation="AES. init from line 143." + gcm2.Seal(nil, nil, password, nil) // $ CryptographicOperation="AES. init from line 143." + gcm2.Open(nil, nil, password, nil) + + gcm3, _ := cipher.NewGCMWithRandomNonce(block) + gcm3.Seal(nil, nil, public, nil) // $ CryptographicOperation="AES. init from line 143." + gcm3.Seal(nil, nil, password, nil) // $ CryptographicOperation="AES. init from line 143." + gcm3.Open(nil, nil, password, nil) + + gcm4, _ := cipher.NewGCMWithTagSize(block, 12) + gcm4.Seal(nil, nil, public, nil) // $ CryptographicOperation="AES. init from line 143." + gcm4.Seal(nil, nil, password, nil) // $ CryptographicOperation="AES. init from line 143." + gcm4.Open(nil, nil, password, nil) + + cbcEncrypter := cipher.NewCBCEncrypter(block, nil) + cbcEncrypter.CryptBlocks(dst, public) // $ CryptographicOperation="AES. blockMode: CBC. init from lines 143,169." + cbcEncrypter.CryptBlocks(dst, password) // $ CryptographicOperation="AES. blockMode: CBC. init from lines 143,169." + cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, password) + + ctrStream := cipher.NewCTR(block, nil) + ctrStream.XORKeyStream(dst, public) // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." + ctrStream.XORKeyStream(dst, password) // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." + + ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(password)} // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." + io.Copy(os.Stdout, ctrStreamReader) + + ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." + io.Copy(ctrStreamWriter, bytes.NewReader(password)) // $ CryptographicOperation="AES. blockMode: CTR. init from lines 143,174." + + // deprecated + + cfbStream := cipher.NewCFBEncrypter(block, nil) + cfbStream.XORKeyStream(dst, public) // $ CryptographicOperation="AES. blockMode: CFB. init from lines 143,186." + cfbStream.XORKeyStream(dst, password) // $ CryptographicOperation="AES. blockMode: CFB. init from lines 143,186." + cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, password) + + ofbStream := cipher.NewOFB(block, nil) + ofbStream.XORKeyStream(dst, public) // $ CryptographicOperation="AES. blockMode: OFB. init from lines 143,191." + ofbStream.XORKeyStream(dst, password) // $ CryptographicOperation="AES. blockMode: OFB. init from lines 143,191." +} + +func CipherRc4() { + c, _ := rc4.NewCipher(nil) + c.XORKeyStream(dst, getPassword()) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="RC4. init from line 198." +} + +func WeakHashes() { buf := password // $ Source - // BAD, des is a weak crypto algorithm and password is sensitive data - des.NewTripleDESCipher(buf) // $ Alert + h := md5.New() + h.Sum(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="MD5. init from line 204." + h.Write(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="MD5. init from line 204." + io.WriteString(h, passwordString) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="MD5. init from line 204." + md5.Sum(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="MD5. init from line 208." - // BAD, md5 is a weak crypto algorithm and password is sensitive data - md5.Sum(buf) // $ Alert - - // BAD, rc4 is a weak crypto algorithm and password is sensitive data - rc4.NewCipher(buf) // $ Alert - - // BAD, sha1 is a weak crypto algorithm and password is sensitive data - sha1.Sum(buf) // $ Alert - - // GOOD, password is sensitive data but aes is a strong crypto algorithm - aes.NewCipher(buf) - - // GOOD, password is sensitive data but sha256 is a strong crypto algorithm - sha256.Sum256(buf) - - // GOOD, des is a weak crypto algorithm but public is not sensitive data - des.NewTripleDESCipher(public) - - // GOOD, md5 is a weak crypto algorithm but public is not sensitive data - md5.Sum(public) - - // GOOD, rc4 is a weak crypto algorithm but public is not sensitive data - rc4.NewCipher(public) - - // GOOD, sha1 is a weak crypto algorithm but public is not sensitive data - sha1.Sum(public) - - // GOOD, aes is a strong crypto algorithm and public is not sensitive data - aes.NewCipher(public) - - // GOOD, sha256 is a strong crypto algorithm and public is not sensitive data - sha256.Sum256(public) + sha1.New().Sum(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="SHA1. init from line 210." + sha1.Sum(buf) // $ Alert[go/weak-crypto-algorithm] CryptographicOperation="SHA1. init from line 211." +} + +func StrongHashes() { + buf := password + + sha256.New224().Sum(buf) // $ CryptographicOperation="SHA224. init from line 217." + sha256.Sum224(buf) // $ CryptographicOperation="SHA224. init from line 218." + + sha256.New().Sum(buf) // $ CryptographicOperation="SHA256. init from line 220." + sha256.Sum256(buf) // $ CryptographicOperation="SHA256. init from line 221." + + sha512.New().Sum(buf) // $ CryptographicOperation="SHA512. init from line 223." + sha512.Sum512(buf) // $ CryptographicOperation="SHA512. init from line 224." + + sha512.New384().Sum(buf) // $ CryptographicOperation="SHA384. init from line 226." + sha512.Sum384(buf) // $ CryptographicOperation="SHA384. init from line 227." + + sha512.New512_224().Sum(buf) // $ CryptographicOperation="SHA512224. init from line 229." + sha512.Sum512_224(buf) // $ CryptographicOperation="SHA512224. init from line 230." + + sha512.New512_256().Sum(buf) // $ CryptographicOperation="SHA512256. init from line 232." + sha512.Sum512_256(buf) // $ CryptographicOperation="SHA512256. init from line 233." + + sha3.New224().Sum(buf) // $ CryptographicOperation="SHA3224. init from line 235." + sha3.Sum224(buf) // $ CryptographicOperation="SHA3224. init from line 236." + + sha3.New256().Sum(buf) // $ CryptographicOperation="SHA3256. init from line 238." + sha3.Sum256(buf) // $ CryptographicOperation="SHA3256. init from line 239." + + sha3.New384().Sum(buf) // $ CryptographicOperation="SHA3384. init from line 241." + sha3.Sum384(buf) // $ CryptographicOperation="SHA3384. init from line 242." + + sha3.New512().Sum(buf) // $ CryptographicOperation="SHA3512. init from line 244." + sha3.Sum512(buf) // $ CryptographicOperation="SHA3512. init from line 245." + + sha3.NewSHAKE128().Write(buf) // $ CryptographicOperation="SHAKE128. init from line 247." + sha3.NewCSHAKE128(nil, nil).Write(buf) // $ CryptographicOperation="SHAKE128. init from line 248." + sha3.SumSHAKE128(buf, 100) // $ CryptographicOperation="SHAKE128. init from line 249." + + sha3.NewSHAKE256().Write(buf) // $ CryptographicOperation="SHAKE256. init from line 251." + sha3.NewCSHAKE256(nil, nil).Write(buf) // $ CryptographicOperation="SHAKE256. init from line 252." + sha3.SumSHAKE256(buf, 100) // $ CryptographicOperation="SHAKE256. init from line 253." } diff --git a/go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.expected b/go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.expected new file mode 100644 index 00000000000..55e9aed2e93 --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.expected @@ -0,0 +1,2 @@ +testFailures +invalidModelRow diff --git a/go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.ql b/go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.ql new file mode 100644 index 00000000000..ab096f9df2a --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.ql @@ -0,0 +1,47 @@ +import go +import ModelValidation +import utils.test.InlineExpectationsTest + +module Test implements TestSig { + string getARelevantTag() { result = "CryptographicOperation" } + + predicate hasActualResult(Location location, string element, string tag, string value) { + tag = "CryptographicOperation" and + exists( + CryptographicOperation::Range ho, string algorithm, string initialization, string blockMode + | + algorithm = ho.getAlgorithm().toString() + "." and + ( + blockMode = " blockMode: " + ho.getBlockMode().toString() + "." + or + not exists(ho.getBlockMode()) and blockMode = "" + ) and + exists(int c | c = count(ho.getInitialization()) | + c = 0 and initialization = "" + or + c = 1 and + initialization = + " init from line " + + strictconcat(DataFlow::Node init | + init = ho.getInitialization() + | + init.getStartLine().toString(), "," + ) + "." + or + c > 1 and + initialization = + " init from lines " + + strictconcat(DataFlow::Node init | + init = ho.getInitialization() + | + init.getStartLine().toString(), "," + ) + "." + ) and + ho.getLocation() = location and + element = ho.toString() and + value = "\"" + algorithm + blockMode + initialization + "\"" + ) + } +} + +import MakeTest diff --git a/go/ql/test/query-tests/Security/CWE-327/go.mod b/go/ql/test/query-tests/Security/CWE-327/go.mod new file mode 100644 index 00000000000..bf42b84feef --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-327/go.mod @@ -0,0 +1,3 @@ +module test + +go 1.24 diff --git a/shared/concepts/codeql/concepts/internal/CryptoAlgorithmNames.qll b/shared/concepts/codeql/concepts/internal/CryptoAlgorithmNames.qll index efcd870c724..86392890caf 100644 --- a/shared/concepts/codeql/concepts/internal/CryptoAlgorithmNames.qll +++ b/shared/concepts/codeql/concepts/internal/CryptoAlgorithmNames.qll @@ -23,7 +23,8 @@ predicate isStrongHashingAlgorithm(string name) { "BLAKE3", // "DSA", "ED25519", "ES256", "ECDSA256", "ES384", "ECDSA384", "ES512", "ECDSA512", "SHA2", - "SHA224", "SHA256", "SHA384", "SHA512", "SHA3", "SHA3224", "SHA3256", "SHA3384", "SHA3512", + "SHA224", "SHA256", "SHA384", "SHA512", "SHA512224", "SHA512256", "SHA3", "SHA3224", + "SHA3256", "SHA3384", "SHA3512", // see https://cryptography.io/en/latest/hazmat/primitives/cryptographic-hashes/#cryptography.hazmat.primitives.hashes.SHAKE128 "SHAKE128", "SHAKE256", // see https://cryptography.io/en/latest/hazmat/primitives/cryptographic-hashes/#sm3