Merge pull request #20666 from owen-mc/go/promote-weak-crypto-algorithm

Go: promote `go/weak-crypto-algorithm`

go/ql/integration-tests/query-suite/go-code-scanning.qls.expected

@@ -18,7 +18,9 @@ ql/go/ql/src/Security/CWE-295/DisabledCertificateCheck.ql
 ql/go/ql/src/Security/CWE-312/CleartextLogging.ql
 ql/go/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql
 ql/go/ql/src/Security/CWE-326/InsufficientKeySize.ql
+ql/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql
 ql/go/ql/src/Security/CWE-327/InsecureTLS.ql
+ql/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql
 ql/go/ql/src/Security/CWE-338/InsecureRandomness.ql
 ql/go/ql/src/Security/CWE-347/MissingJwtSignatureCheck.ql
 ql/go/ql/src/Security/CWE-352/ConstantOauth2State.ql

go/ql/integration-tests/query-suite/go-security-and-quality.qls.expected

@@ -41,7 +41,9 @@ ql/go/ql/src/Security/CWE-295/DisabledCertificateCheck.ql
 ql/go/ql/src/Security/CWE-312/CleartextLogging.ql
 ql/go/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql
 ql/go/ql/src/Security/CWE-326/InsufficientKeySize.ql
+ql/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql
 ql/go/ql/src/Security/CWE-327/InsecureTLS.ql
+ql/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql
 ql/go/ql/src/Security/CWE-338/InsecureRandomness.ql
 ql/go/ql/src/Security/CWE-347/MissingJwtSignatureCheck.ql
 ql/go/ql/src/Security/CWE-352/ConstantOauth2State.ql

go/ql/integration-tests/query-suite/go-security-extended.qls.expected

@@ -19,7 +19,9 @@ ql/go/ql/src/Security/CWE-295/DisabledCertificateCheck.ql
 ql/go/ql/src/Security/CWE-312/CleartextLogging.ql
 ql/go/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql
 ql/go/ql/src/Security/CWE-326/InsufficientKeySize.ql
+ql/go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql
 ql/go/ql/src/Security/CWE-327/InsecureTLS.ql
+ql/go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql
 ql/go/ql/src/Security/CWE-338/InsecureRandomness.ql
 ql/go/ql/src/Security/CWE-347/MissingJwtSignatureCheck.ql
 ql/go/ql/src/Security/CWE-352/ConstantOauth2State.ql

go/ql/integration-tests/query-suite/not_included_in_qls.expected

@@ -14,7 +14,6 @@ ql/go/ql/src/experimental/CWE-203/Timing.ql
 ql/go/ql/src/experimental/CWE-285/PamAuthBypass.ql
 ql/go/ql/src/experimental/CWE-287/ImproperLdapAuth.ql
 ql/go/ql/src/experimental/CWE-321-V2/HardCodedKeys.ql
-ql/go/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.ql
 ql/go/ql/src/experimental/CWE-369/DivideByZero.ql
 ql/go/ql/src/experimental/CWE-400/DatabaseCallInLoop.ql
 ql/go/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.ql

go/ql/lib/go.qll

@@ -33,6 +33,7 @@ import semmle.go.frameworks.AwsLambda
 import semmle.go.frameworks.Beego
 import semmle.go.frameworks.BeegoOrm
 import semmle.go.frameworks.Bun
+import semmle.go.frameworks.CryptoLibraries
 import semmle.go.frameworks.RsCors
 import semmle.go.frameworks.Couchbase
 import semmle.go.frameworks.Echo

go/ql/lib/qlpack.yml

@@ -6,6 +6,7 @@ extractor: go
 library: true
 upgrades: upgrades
 dependencies:
+  codeql/concepts: ${workspace}
   codeql/dataflow: ${workspace}
   codeql/mad: ${workspace}
   codeql/threat-models: ${workspace}

go/ql/lib/semmle/go/Concepts.qll

@@ -8,6 +8,10 @@ import go
 import semmle.go.dataflow.FunctionInputsAndOutputs
 import semmle.go.concepts.HTTP
 import semmle.go.concepts.GeneratedFile
+private import codeql.concepts.ConceptsShared
+private import semmle.go.dataflow.internal.DataFlowImplSpecific
+
+private module ConceptsShared = ConceptsMake<Location, GoDataFlow>;
 
 /**
  * A data-flow node that executes an operating system command,
@@ -505,3 +509,98 @@ module UnmarshalingFunction {
     abstract string getFormat();
   }
 }
+
+/**
+ * Provides models for cryptographic things.
+ */
+module Cryptography {
+  private import ConceptsShared::Cryptography as SC
+
+  /**
+   * A data-flow node that is an application of a cryptographic algorithm. For example,
+   * encryption, decryption, signature-validation.
+   *
+   * Extend this class to refine existing API models. If you want to model new APIs,
+   * extend `CryptographicOperation::Range` instead.
+   */
+  class CryptographicOperation extends SC::CryptographicOperation { }
+
+  class EncryptionAlgorithm = SC::EncryptionAlgorithm;
+
+  class HashingAlgorithm = SC::HashingAlgorithm;
+
+  class PasswordHashingAlgorithm = SC::PasswordHashingAlgorithm;
+
+  module CryptographicOperation = SC::CryptographicOperation;
+
+  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)
+    }
+  }
+}

go/ql/lib/semmle/go/frameworks/CryptoLibraries.qll

@@ -0,0 +1,484 @@
+/**
+ * Provides classes for modeling cryptographic libraries.
+ */
+
+import go
+import semmle.go.Concepts::Cryptography
+private import codeql.concepts.internal.CryptoAlgorithmNames
+
+/**
+ * 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.
+ */
+abstract class DirectHashOperation extends HashOperation instanceof DataFlow::CallNode {
+  override DataFlow::Node getInitialization() { result = this }
+
+  override DataFlow::Node getAnInput() { result = super.getArgument(0) }
+}
+
+private module HashConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof HashAlgorithmInit }
+
+  predicate isSink(DataFlow::Node sink) { any() }
+}
+
+/** Tracks the flow of hash algorithms. */
+module HashFlow = DataFlow::Global<HashConfig>;
+
+/**
+ * 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)
+  }
+}
+
+/**
+ * Tracks algorithms and block cipher modes of operation used for encryption.
+ */
+module EncryptionFlow = DataFlow::Global<EncryptionConfig>;
+
+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) }
+  }
+}
+
+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
+  )
+}
+
+/**
+ * Taint flowing to an `io.Writer` (such as `hash.Hash` or `*sha3.SHAKE`) via
+ * its implementation of the `io.Writer` interface.
+ */
+private class FlowToIoWriter extends HashOperation instanceof DataFlow::Node {
+  HashAlgorithmInit init;
+  DataFlow::Node input;
+
+  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 }
+}
+
+/**
+ * Currently only weak algorithms from the `golang.org/x/crypto` module are
+ * modeled here.
+ */
+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 HashingAlgorithm getAlgorithm() { result.matchesName("MD4") }
+    }
+  }
+
+  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") }
+    }
+  }
+}

go/ql/lib/semmle/go/security/WeakSensitiveDataHashingCustomizations.qll

@@ -0,0 +1,171 @@
+/**
+ * Provides default sources, sinks and sanitizers for detecting "use of a
+ * broken or weak cryptographic hashing algorithm on sensitive data"
+ * vulnerabilities, as well as extension points for adding your own. This is
+ * divided into two general cases:
+ *  - hashing sensitive data
+ *  - hashing passwords (which requires the hashing algorithm to be
+ *    sufficiently computationally expensive in addition to other requirements)
+ */
+
+import go
+private import semmle.go.security.SensitiveActions
+
+/**
+ * Provides default sources, sinks and sanitizers for detecting "use of a broken or weak
+ * cryptographic hashing algorithm on sensitive data" vulnerabilities on sensitive data that does
+ * NOT require computationally expensive hashing, as well as extension points for adding your own.
+ *
+ * Also see the `ComputationallyExpensiveHashFunction` module.
+ */
+module NormalHashFunction {
+  /**
+   * A data flow source for "use of a broken or weak cryptographic hashing algorithm on sensitive
+   * data" vulnerabilities that does not require computationally expensive hashing. That is, a
+   * piece of sensitive data that is not a password.
+   */
+  abstract class Source extends DataFlow::Node {
+    Source() { not this instanceof ComputationallyExpensiveHashFunction::Source }
+
+    /**
+     * Gets the classification of the sensitive data.
+     */
+    abstract string getClassification();
+  }
+
+  /**
+   * A data flow sink for "use of a broken or weak cryptographic hashing algorithm on sensitive
+   * data" vulnerabilities that applies to data that does not require computationally expensive
+   * hashing. That is, a broken or weak hashing algorithm.
+   */
+  abstract class Sink extends DataFlow::Node {
+    /**
+     * Gets the name of the weak hashing algorithm.
+     */
+    abstract string getAlgorithmName();
+  }
+
+  /**
+   * A barrier for "use of a broken or weak cryptographic hashing algorithm on sensitive data"
+   * vulnerabilities that applies to data that does not require computationally expensive hashing.
+   */
+  abstract class Barrier extends DataFlow::Node { }
+
+  /**
+   * A flow source modeled by the `SensitiveData` library.
+   */
+  class SensitiveDataAsSource extends Source {
+    SensitiveExpr::Classification classification;
+
+    SensitiveDataAsSource() {
+      classification = this.asExpr().(SensitiveExpr).getClassification() and
+      not classification = SensitiveExpr::password() and // (covered in ComputationallyExpensiveHashFunction)
+      not classification = SensitiveExpr::id() // (not accurate enough)
+    }
+
+    override SensitiveExpr::Classification getClassification() { result = classification }
+  }
+
+  /**
+   * A flow sink modeled by the `Cryptography` module.
+   */
+  class WeakHashingOperationInputAsSink extends Sink {
+    Cryptography::HashingAlgorithm algorithm;
+
+    WeakHashingOperationInputAsSink() {
+      exists(Cryptography::CryptographicOperation operation |
+        algorithm.isWeak() and
+        algorithm = operation.getAlgorithm() and
+        this = operation.getAnInput()
+      )
+    }
+
+    override string getAlgorithmName() { result = algorithm.getName() }
+  }
+}
+
+/**
+ * Provides default sources, sinks and sanitizers for detecting "use of a broken or weak
+ * cryptographic hashing algorithm on sensitive data" vulnerabilities on sensitive data that DOES
+ * require computationally expensive hashing, as well as extension points for adding your own.
+ *
+ * Also see the `NormalHashFunction` module.
+ */
+module ComputationallyExpensiveHashFunction {
+  /**
+   * A data flow source for "use of a broken or weak cryptographic hashing algorithm on sensitive
+   * data" vulnerabilities that does require computationally expensive hashing. That is, a
+   * password.
+   */
+  abstract class Source extends DataFlow::Node {
+    /**
+     * Gets the classification of the sensitive data.
+     */
+    abstract string getClassification();
+  }
+
+  /**
+   * A data flow sink for "use of a broken or weak cryptographic hashing algorithm on sensitive
+   * data" vulnerabilities that applies to data that does require computationally expensive
+   * hashing. That is, a broken or weak hashing algorithm or one that is not computationally
+   * expensive enough for password hashing.
+   */
+  abstract class Sink extends DataFlow::Node {
+    /**
+     * Gets the name of the weak hashing algorithm.
+     */
+    abstract string getAlgorithmName();
+
+    /**
+     * Holds if this sink is for a computationally expensive hash function (meaning that hash
+     * function is just weak in some other regard.
+     */
+    abstract predicate isComputationallyExpensive();
+  }
+
+  /**
+   * A barrier for "use of a broken or weak cryptographic hashing algorithm on sensitive data"
+   * vulnerabilities that applies to data that does require computationally expensive hashing.
+   */
+  abstract class Barrier extends DataFlow::Node { }
+
+  /**
+   * A flow source modeled by the `SensitiveData` library.
+   */
+  class PasswordAsSource extends Source {
+    SensitiveExpr::Classification classification;
+
+    PasswordAsSource() {
+      classification = this.asExpr().(SensitiveExpr).getClassification() and
+      classification = SensitiveExpr::password()
+    }
+
+    override SensitiveExpr::Classification getClassification() { result = classification }
+  }
+
+  /**
+   * A flow sink modeled by the `Cryptography` module.
+   */
+  class WeakPasswordHashingOperationInputSink extends Sink {
+    Cryptography::CryptographicAlgorithm algorithm;
+
+    WeakPasswordHashingOperationInputSink() {
+      exists(Cryptography::CryptographicOperation operation |
+        (
+          algorithm instanceof Cryptography::PasswordHashingAlgorithm and
+          algorithm.isWeak()
+          or
+          algorithm instanceof Cryptography::HashingAlgorithm // Note that HashingAlgorithm and PasswordHashingAlgorithm are disjoint
+        ) and
+        algorithm = operation.getAlgorithm() and
+        this = operation.getAnInput()
+      )
+    }
+
+    override string getAlgorithmName() { result = algorithm.getName() }
+
+    override predicate isComputationallyExpensive() {
+      algorithm instanceof Cryptography::PasswordHashingAlgorithm
+    }
+  }
+}

go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.qhelp

@@ -28,10 +28,10 @@
      <example>
 
           <p>
-               The following code uses the different packages to encrypt/hash 
+               The following code uses the different packages to encrypt
-               some secret data. The first few examples uses DES, MD5, RC4, and SHA1, 
+               some secret data. The first example uses DES,
-               which are older algorithms that are now considered weak. The following
+               which is an older algorithm that is now considered weak. The following
-               examples use AES and SHA256, which are stronger, more modern algorithms.
+               example uses AES, which is a stronger, more modern algorithm.
           </p>
 
           <sample src="examples/Crypto.go" />

go/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql

@@ -0,0 +1,33 @@
+/**
+ * @name Use of a broken or weak cryptographic algorithm
+ * @description Using broken or weak cryptographic algorithms can compromise security.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 7.5
+ * @precision high
+ * @id go/weak-cryptographic-algorithm
+ * @tags security
+ *       external/cwe/cwe-327
+ *       external/cwe/cwe-328
+ */
+
+import go
+
+from Cryptography::CryptographicOperation operation, string msgPrefix, DataFlow::Node init
+where
+  init = operation.getInitialization() and
+  // `init` may be a `BlockModeInit`, a `EncryptionAlgorithmInit`, or `operation` itself.
+  (
+    not init instanceof BlockModeInit and
+    exists(Cryptography::CryptographicAlgorithm algorithm |
+      algorithm = operation.getAlgorithm() and
+      algorithm.isWeak() and
+      msgPrefix = "The cryptographic algorithm " + algorithm.getName() and
+      not algorithm instanceof Cryptography::HashingAlgorithm
+    )
+    or
+    not init instanceof EncryptionAlgorithmInit and
+    operation.getBlockMode().isWeak() and
+    msgPrefix = "The block mode " + operation.getBlockMode()
+  )
+select operation, "$@ is broken or weak, and should not be used.", init, msgPrefix

go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.qhelp

@@ -0,0 +1,97 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+     <overview>
+          <p>
+               Using a broken or weak cryptographic hash function can leave data
+               vulnerable, and should not be used in security related code.
+          </p>
+
+          <p>
+               A strong cryptographic hash function should be resistant to:
+          </p>
+          <ul>
+               <li>
+                    pre-image attacks: if you know a hash value <code>h(x)</code>,
+                    you should not be able to easily find the input <code>x</code>.
+               </li>
+               <li>
+                    collision attacks: if you know a hash value <code>h(x)</code>,
+                    you should not be able to easily find a different input <code>y</code>
+                    with the same hash value <code>h(x) = h(y)</code>.
+               </li>
+          </ul>
+          <p>
+               In cases with a limited input space, such as for passwords, the hash
+               function also needs to be computationally expensive to be resistant to
+               brute-force attacks. Passwords should also have an unique salt applied
+               before hashing, but that is not considered by this query.
+          </p>
+
+          <p>
+               As an example, both MD5 and SHA-1 are known to be vulnerable to collision attacks.
+          </p>
+
+          <p>
+               Since it's OK to use a weak cryptographic hash function in a non-security
+               context, this query only alerts when these are used to hash sensitive
+               data (such as passwords, certificates, usernames).
+          </p>
+
+          <p>
+               Use of broken or weak cryptographic algorithms that are not hashing algorithms, is
+               handled by the <code>rb/weak-cryptographic-algorithm</code> query.
+          </p>
+
+     </overview>
+     <recommendation>
+
+          <p>
+               Ensure that you use a strong, modern cryptographic hash function:
+          </p>
+
+          <ul>
+               <li>
+                    such as Argon2, scrypt, bcrypt, or PBKDF2 for passwords and other data with limited input space.
+               </li>
+               <li>
+                    such as SHA-2, or SHA-3 in other cases.
+               </li>
+          </ul>
+
+     </recommendation>
+     <example>
+
+          <p>
+               The following example shows two functions for checking whether the hash
+               of a secret matches a known value.
+
+               The first function uses SHA-1 that is known to be vulnerable to collision attacks.
+
+               The second function uses SHA-256 that is a strong cryptographic hashing function.
+          </p>
+
+          <sample src="examples/WeakSecretHashing.go" />
+
+     </example>
+     <example>
+          <p>
+               The following example shows two functions for hashing passwords.
+
+               The first example uses SHA-256 to hash passwords. Although
+               SHA-256 is a strong cryptographic hash function, it is not suitable for password
+               hashing since it is not computationally expensive.
+
+               The second function uses PBKDF2, which is a strong password hashing algorithm.
+          </p>
+
+          <sample src="examples/WeakPasswordHashing.go" />
+
+     </example>
+
+     <references>
+          <li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html">Password Storage Cheat Sheet</a></li>
+     </references>
+
+</qhelp>

go/ql/src/Security/CWE-327/WeakSensitiveDataHashing.ql

@@ -0,0 +1,118 @@
+/**
+ * @name Use of a broken or weak cryptographic hashing algorithm on sensitive data
+ * @description Using broken or weak cryptographic hashing algorithms can compromise security.
+ * @kind path-problem
+ * @problem.severity warning
+ * @security-severity 7.5
+ * @precision high
+ * @id go/weak-sensitive-data-hashing
+ * @tags security
+ *       external/cwe/cwe-327
+ *       external/cwe/cwe-328
+ *       external/cwe/cwe-916
+ */
+
+import go
+import semmle.go.security.WeakSensitiveDataHashingCustomizations
+
+/**
+ * Provides a taint-tracking configuration for detecting use of a broken or weak
+ * cryptographic hash function on sensitive data, that does NOT require a
+ * computationally expensive hash function.
+ */
+module NormalHashFunctionFlow {
+  import NormalHashFunction
+
+  private module Config implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+    predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+    predicate isBarrier(DataFlow::Node node) { node instanceof Barrier }
+
+    predicate isBarrierIn(DataFlow::Node node) {
+      // make sources barriers so that we only report the closest instance
+      isSource(node)
+    }
+
+    predicate isBarrierOut(DataFlow::Node node) {
+      // make sinks barriers so that we only report the closest instance
+      isSink(node)
+    }
+
+    predicate observeDiffInformedIncrementalMode() { any() }
+  }
+
+  import TaintTracking::Global<Config>
+}
+
+/**
+ * Provides a taint-tracking configuration for detecting use of a broken or weak
+ * cryptographic hashing algorithm on passwords.
+ *
+ * Passwords has stricter requirements on the hashing algorithm used (must be
+ * computationally expensive to prevent brute-force attacks).
+ */
+module ComputationallyExpensiveHashFunctionFlow {
+  import ComputationallyExpensiveHashFunction
+
+  private module Config implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+    predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+    predicate isBarrier(DataFlow::Node node) { node instanceof Barrier }
+
+    predicate isBarrierIn(DataFlow::Node node) {
+      // make sources barriers so that we only report the closest instance
+      isSource(node)
+    }
+
+    predicate isBarrierOut(DataFlow::Node node) {
+      // make sinks barriers so that we only report the closest instance
+      isSink(node)
+    }
+
+    predicate observeDiffInformedIncrementalMode() { any() }
+  }
+
+  import TaintTracking::Global<Config>
+}
+
+/**
+ * Global taint-tracking for detecting both variants of "use of a broken or weak
+ * cryptographic hashing algorithm on sensitive data" vulnerabilities. The two configurations are
+ * merged to generate a combined path graph.
+ */
+module WeakSensitiveDataHashingFlow =
+  DataFlow::MergePathGraph<NormalHashFunctionFlow::PathNode,
+    ComputationallyExpensiveHashFunctionFlow::PathNode, NormalHashFunctionFlow::PathGraph,
+    ComputationallyExpensiveHashFunctionFlow::PathGraph>;
+
+import WeakSensitiveDataHashingFlow::PathGraph
+
+from
+  WeakSensitiveDataHashingFlow::PathNode source, WeakSensitiveDataHashingFlow::PathNode sink,
+  string ending, string algorithmName, string classification
+where
+  NormalHashFunctionFlow::flowPath(source.asPathNode1(), sink.asPathNode1()) and
+  algorithmName = sink.getNode().(NormalHashFunction::Sink).getAlgorithmName() and
+  classification = source.getNode().(NormalHashFunction::Source).getClassification() and
+  ending = "."
+  or
+  ComputationallyExpensiveHashFunctionFlow::flowPath(source.asPathNode2(), sink.asPathNode2()) and
+  algorithmName = sink.getNode().(ComputationallyExpensiveHashFunction::Sink).getAlgorithmName() and
+  classification =
+    source.getNode().(ComputationallyExpensiveHashFunction::Source).getClassification() and
+  (
+    sink.getNode().(ComputationallyExpensiveHashFunction::Sink).isComputationallyExpensive() and
+    ending = "."
+    or
+    not sink.getNode().(ComputationallyExpensiveHashFunction::Sink).isComputationallyExpensive() and
+    ending =
+      " for " + classification +
+        " hashing, since it is not a computationally expensive hash function."
+  )
+select sink.getNode(), source, sink,
+  "$@ is used in a hashing algorithm (" + algorithmName + ") that is insecure" + ending,
+  source.getNode(), "Sensitive data (" + classification + ")"

go/ql/src/Security/CWE-327/examples/Crypto.go

@@ -0,0 +1,20 @@
+package main
+
+import (
+	"crypto/aes"
+	"crypto/des"
+)
+
+func EncryptMessageWeak(key []byte, message []byte) (dst []byte) {
+	// BAD, DES is a weak crypto algorithm
+	block, _ := des.NewCipher(key)
+	block.Encrypt(dst, message)
+	return
+}
+
+func EncryptMessageStrong(key []byte, message []byte) (dst []byte) {
+	// GOOD, AES is a weak crypto algorithm
+	block, _ := aes.NewCipher(key)
+	block.Encrypt(dst, message)
+	return
+}

go/ql/src/Security/CWE-327/examples/WeakPasswordHashing.go

@@ -0,0 +1,21 @@
+package main
+
+import (
+	"crypto/pbkdf2"
+	"crypto/rand"
+	"crypto/sha256"
+	"crypto/sha512"
+)
+
+func GetPasswordHashBad(password string) [32]byte {
+	// BAD, SHA256 is a strong hashing algorithm but it is not computationally expensive
+	return sha256.Sum256([]byte(password))
+}
+
+func GetPasswordHashGood(password string) []byte {
+	// GOOD, PBKDF2 is a strong hashing algorithm and it is computationally expensive
+	salt := make([]byte, 16)
+	rand.Read(salt)
+	key, _ := pbkdf2.Key(sha512.New, password, salt, 4096, 32)
+	return key
+}

go/ql/src/Security/CWE-327/examples/WeakSecretHashing.go

@@ -0,0 +1,19 @@
+package main
+
+import (
+	"crypto/sha1"
+	"crypto/sha256"
+	"slices"
+)
+
+func SecretMatchesKnownHashBad(secret []byte, known_hash []byte) bool {
+	// BAD, SHA1 is a weak crypto algorithm and secret is sensitive data
+	h := sha1.New()
+	return slices.Equal(h.Sum(secret), known_hash)
+}
+
+func SecretMatchesKnownHashGood(secret []byte, known_hash []byte) bool {
+	// GOOD, SHA256 is a strong hashing algorithm
+	h := sha256.New()
+	return slices.Equal(h.Sum(secret), known_hash)
+}

go/ql/src/change-notes/2025-10-21-go-weak-crypto-algorithm.md

@@ -0,0 +1,5 @@
+---
+category: newQuery
+---
+* Added a new query, `go/weak-crypto-algorithm`, to detect the use of a broken or weak cryptographic algorithm. A very simple version of this query was originally contributed as an [experimental query by @dilanbhalla](https://github.com/github/codeql-go/pull/284).
+* Added a new query, `go/weak-sensitive-data-hashing`, to detect the use of a broken or weak cryptographic hash algorithm on sensitive data.

go/ql/src/experimental/CWE-327/CryptoLibraries.qll

@@ -1,213 +0,0 @@
-/**
- * Provides classes for modeling cryptographic libraries.
- */
-
-import go
-
-/**
- * Names of cryptographic algorithms, separated into strong and weak variants.
- *
- * The names are normalized: upper-case, no spaces, dashes or underscores.
- *
- * The names are inspired by the names used in real world crypto libraries.
- *
- * The classification into strong and weak are based on OWASP and Wikipedia (2020).
- *
- * Sources (more links in qhelp file):
- * https://en.wikipedia.org/wiki/Strong_cryptography#Cryptographically_strong_algorithms
- * https://en.wikipedia.org/wiki/Strong_cryptography#Examples
- * https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html
- */
-private module AlgorithmNames {
-  predicate isStrongHashingAlgorithm(string name) {
-    name =
-      [
-        "DSA", "ED25519", "SHA256", "SHA384", "SHA512", "SHA3", "ES256", "ECDSA256", "ES384",
-        "ECDSA384", "ES512", "ECDSA512", "SHA2", "SHA224"
-      ]
-  }
-
-  predicate isWeakHashingAlgorithm(string name) {
-    name =
-      [
-        "HAVEL128", "MD2", "SHA1", "MD4", "MD5", "PANAMA", "RIPEMD", "RIPEMD128", "RIPEMD256",
-        "RIPEMD320", "SHA0"
-      ]
-  }
-
-  predicate isStrongEncryptionAlgorithm(string name) {
-    name = ["AES", "AES128", "AES192", "AES256", "AES512", "RSA", "RABBIT", "BLOWFISH"]
-  }
-
-  predicate isWeakEncryptionAlgorithm(string name) {
-    name =
-      [
-        "DES", "3DES", "ARC5", "RC5", "TRIPLEDES", "TDEA", "TRIPLEDEA", "ARC2", "RC2", "ARC4",
-        "RC4", "ARCFOUR"
-      ]
-  }
-
-  predicate isStrongPasswordHashingAlgorithm(string name) {
-    name = ["ARGON2", "PBKDF2", "BCRYPT", "SCRYPT"]
-  }
-
-  predicate isWeakPasswordHashingAlgorithm(string name) { none() }
-}
-
-private import AlgorithmNames
-
-/**
- * A cryptographic algorithm.
- */
-private newtype TCryptographicAlgorithm =
-  MkHashingAlgorithm(string name, boolean isWeak) {
-    isStrongHashingAlgorithm(name) and isWeak = false
-    or
-    isWeakHashingAlgorithm(name) and isWeak = true
-  } or
-  MkEncryptionAlgorithm(string name, boolean isWeak) {
-    isStrongEncryptionAlgorithm(name) and isWeak = false
-    or
-    isWeakEncryptionAlgorithm(name) and isWeak = true
-  } or
-  MkPasswordHashingAlgorithm(string name, boolean isWeak) {
-    isStrongPasswordHashingAlgorithm(name) and isWeak = false
-    or
-    isWeakPasswordHashingAlgorithm(name) and isWeak = true
-  }
-
-/**
- * A cryptographic algorithm.
- */
-abstract class CryptographicAlgorithm extends TCryptographicAlgorithm {
-  /** Gets a textual representation of this element. */
-  string toString() { result = this.getName() }
-
-  /**
-   * Gets the name of this algorithm.
-   */
-  abstract string getName();
-
-  /**
-   * Holds if the name of this algorithm matches `name` modulo case,
-   * white space, dashes and underscores.
-   */
-  bindingset[name]
-  predicate matchesName(string name) {
-    exists(name.regexpReplaceAll("[-_]", "").regexpFind("(?i)\\Q" + this.getName() + "\\E", _, _))
-  }
-
-  /**
-   * Holds if this algorithm is weak.
-   */
-  abstract predicate isWeak();
-}
-
-/**
- * A hashing algorithm such as `MD5` or `SHA512`.
- */
-class HashingAlgorithm extends MkHashingAlgorithm, CryptographicAlgorithm {
-  string name;
-  boolean isWeak;
-
-  HashingAlgorithm() { this = MkHashingAlgorithm(name, isWeak) }
-
-  override string getName() { result = name }
-
-  override predicate isWeak() { isWeak = true }
-}
-
-/**
- * An encryption algorithm such as `DES` or `AES512`.
- */
-class EncryptionAlgorithm extends MkEncryptionAlgorithm, CryptographicAlgorithm {
-  string name;
-  boolean isWeak;
-
-  EncryptionAlgorithm() { this = MkEncryptionAlgorithm(name, isWeak) }
-
-  override string getName() { result = name }
-
-  override predicate isWeak() { isWeak = true }
-}
-
-/**
- * A password hashing algorithm such as `PBKDF2` or `SCRYPT`.
- */
-class PasswordHashingAlgorithm extends MkPasswordHashingAlgorithm, CryptographicAlgorithm {
-  string name;
-  boolean isWeak;
-
-  PasswordHashingAlgorithm() { this = MkPasswordHashingAlgorithm(name, isWeak) }
-
-  override string getName() { result = name }
-
-  override predicate isWeak() { isWeak = true }
-}
-
-/**
- * An application of a cryptographic algorithm.
- */
-abstract class CryptographicOperation extends DataFlow::Node {
-  /**
-   * Gets the input the algorithm is used on, e.g. the plain text input to be encrypted.
-   */
-  abstract Expr getInput();
-
-  /**
-   * Gets the applied algorithm.
-   */
-  abstract CryptographicAlgorithm getAlgorithm();
-}
-
-/**
- * A cryptographic operation from the `crypto/md5` package.
- */
-class Md5 extends CryptographicOperation, DataFlow::CallNode {
-  Md5() { this.getTarget().hasQualifiedName("crypto/md5", ["New", "Sum"]) }
-
-  override Expr getInput() { result = this.getArgument(0).asExpr() }
-
-  override CryptographicAlgorithm getAlgorithm() {
-    result.matchesName(this.getTarget().getPackage().getName())
-  }
-}
-
-/**
- * A cryptographic operation from the `crypto/sha1` package.
- */
-class Sha1 extends CryptographicOperation, DataFlow::CallNode {
-  Sha1() { this.getTarget().hasQualifiedName("crypto/sha1", ["New", "Sum"]) }
-
-  override Expr getInput() { result = this.getArgument(0).asExpr() }
-
-  override CryptographicAlgorithm getAlgorithm() {
-    result.matchesName(this.getTarget().getPackage().getName())
-  }
-}
-
-/**
- * A cryptographic operation from the `crypto/des` package.
- */
-class Des extends CryptographicOperation, DataFlow::CallNode {
-  Des() { this.getTarget().hasQualifiedName("crypto/des", ["NewCipher", "NewTripleDESCipher"]) }
-
-  override Expr getInput() { result = this.getArgument(0).asExpr() }
-
-  override CryptographicAlgorithm getAlgorithm() {
-    result.matchesName(this.getTarget().getPackage().getName())
-  }
-}
-
-/**
- * A cryptographic operation from the `crypto/rc4` package.
- */
-class Rc4 extends CryptographicOperation, 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())
-  }
-}

go/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.ql

@@ -1,20 +0,0 @@
-/**
- * @name Use of a weak cryptographic algorithm
- * @description Using weak cryptographic algorithms can allow an attacker to compromise security.
- * @kind path-problem
- * @problem.severity error
- * @id go/weak-crypto-algorithm
- * @tags security
- *       experimental
- *       external/cwe/cwe-327
- *       external/cwe/cwe-328
- */
-
-import go
-import WeakCryptoAlgorithmCustomizations
-import WeakCryptoAlgorithm::Flow::PathGraph
-
-from WeakCryptoAlgorithm::Flow::PathNode source, WeakCryptoAlgorithm::Flow::PathNode sink
-where WeakCryptoAlgorithm::Flow::flowPath(source, sink)
-select sink.getNode(), source, sink, "$@ is used in a weak cryptographic algorithm.",
-  source.getNode(), "Sensitive data"

go/ql/src/experimental/CWE-327/WeakCryptoAlgorithmCustomizations.qll

@@ -1,66 +0,0 @@
-/**
- * Provides default sources, sinks and sanitizers for reasoning about
- * sensitive information in weak cryptographic algorithms,
- * as well as extension points for adding your own.
- */
-
-import go
-private import semmle.go.security.SensitiveActions
-private import CryptoLibraries
-
-/**
- * Provides default sources, sinks and sanitizers for reasoning about
- * sensitive information in weak cryptographic algorithms,
- * as well as extension points for adding your own.
- */
-module WeakCryptoAlgorithm {
-  /**
-   * A data flow source for sensitive information in weak cryptographic algorithms.
-   */
-  abstract class Source extends DataFlow::Node { }
-
-  /**
-   * A data flow sink for sensitive information in weak cryptographic algorithms.
-   */
-  abstract class Sink extends DataFlow::Node { }
-
-  /**
-   * A sanitizer for sensitive information in weak cryptographic algorithms.
-   */
-  abstract class Sanitizer extends DataFlow::Node { }
-
-  /**
-   * A sensitive source.
-   */
-  class SensitiveSource extends Source {
-    SensitiveSource() { this.asExpr() instanceof SensitiveExpr }
-  }
-
-  /**
-   * An expression used by a weak cryptographic algorithm.
-   */
-  class WeakCryptographicOperationSink extends Sink {
-    WeakCryptographicOperationSink() {
-      exists(CryptographicOperation application |
-        application.getAlgorithm().isWeak() and
-        this.asExpr() = application.getInput()
-      )
-    }
-  }
-
-  private module Config implements DataFlow::ConfigSig {
-    predicate isSource(DataFlow::Node source) { source instanceof Source }
-
-    predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
-
-    predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
-
-    predicate observeDiffInformedIncrementalMode() { any() }
-  }
-
-  /**
-   * Tracks taint flow from sensitive information to weak cryptographic
-   * algorithms.
-   */
-  module Flow = TaintTracking::Global<Config>;
-}

go/ql/src/experimental/CWE-327/examples/Crypto.go

@@ -1,53 +0,0 @@
-package main
-
-import (
-	"crypto/aes"
-	"crypto/des"
-	"crypto/md5"
-	"crypto/rc4"
-	"crypto/sha1"
-	"crypto/sha256"
-)
-
-func main() {
-	public := []byte("hello")
-
-	password := []byte("123456")
-	buf := password // testing dataflow by passing into different variable
-
-	// BAD, des is a weak crypto algorithm and password is sensitive data
-	des.NewTripleDESCipher(buf)
-
-	// BAD, md5 is a weak crypto algorithm and password is sensitive data
-	md5.Sum(buf)
-
-	// BAD, rc4 is a weak crypto algorithm and password is sensitive data
-	rc4.NewCipher(buf)
-
-	// BAD, sha1 is a weak crypto algorithm and password is sensitive data
-	sha1.Sum(buf)
-
-	// 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)
-}

go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected

@@ -0,0 +1,29 @@
+| encryption.go:30:2:30:36 | call to Encrypt | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:34:2:34:42 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:38:2:38:42 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:42:2:42:42 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:46:2:46:42 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:50:2:50:47 | call to CryptBlocks | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:54:2:54:45 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:56:22:56:91 | struct literal | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:59:21:59:68 | &... [postupdate] | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:59:22:59:68 | struct literal | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:59:22:59:68 | struct literal [postupdate] | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:60:10:60:24 | ctrStreamWriter [postupdate] | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:65:2:65:45 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:69:2:69:45 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:28:2:28:31 | ... := ...[0] | The cryptographic algorithm DES |
+| encryption.go:76:2:76:32 | call to Encrypt | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:80:2:80:38 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:84:2:84:38 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:88:2:88:42 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:92:2:92:42 | call to Seal | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:96:2:96:43 | call to CryptBlocks | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:100:2:100:41 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:102:22:102:87 | struct literal | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:105:21:105:68 | &... [postupdate] | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:105:22:105:68 | struct literal | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:105:22:105:68 | struct literal [postupdate] | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:106:10:106:24 | ctrStreamWriter [postupdate] | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:111:2:111:45 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:115:2:115:45 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:74:2:74:40 | ... := ...[0] | The cryptographic algorithm TRIPLEDES |
+| encryption.go:166:2:166:33 | call to XORKeyStream | $@ is broken or weak, and should not be used. | encryption.go:166:2:166:33 | call to XORKeyStream | The cryptographic algorithm RC4 |

go/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.qlref

@@ -0,0 +1,4 @@
+query: Security/CWE-327/BrokenCryptoAlgorithm.ql
+postprocess:
+  - utils/test/PrettyPrintModels.ql
+  - utils/test/InlineExpectationsTestQuery.ql

go/ql/test/query-tests/Security/CWE-327/Crypto.go

@@ -1,53 +0,0 @@
-package main
-
-import (
-	"crypto/aes"
-	"crypto/des"
-	"crypto/md5"
-	"crypto/rc4"
-	"crypto/sha1"
-	"crypto/sha256"
-)
-
-func crypto() {
-	public := []byte("hello")
-
-	password := []byte("123456")
-	buf := password // testing dataflow by passing into different variable
-
-	// BAD, des is a weak crypto algorithm and password is sensitive data
-	des.NewTripleDESCipher(buf)
-
-	// BAD, md5 is a weak crypto algorithm and password is sensitive data
-	md5.Sum(buf)
-
-	// BAD, rc4 is a weak crypto algorithm and password is sensitive data
-	rc4.NewCipher(buf)
-
-	// BAD, sha1 is a weak crypto algorithm and password is sensitive data
-	sha1.Sum(buf)
-
-	// 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)
-}

go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.expected

@@ -0,0 +1,2 @@
+testFailures
+invalidModelRow

go/ql/test/query-tests/Security/CWE-327/CryptoAlgorithm.ql

@@ -0,0 +1,39 @@
+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 > 0 and
+        initialization =
+          " init from " +
+            strictconcat(DataFlow::Node init, int n |
+              init = ho.getInitialization() and
+              n = ho.getStartLine() - init.getStartLine()
+            |
+              n.toString(), ","
+            ) + " lines above."
+      ) and
+      ho.getLocation() = location and
+      element = ho.toString() and
+      value = "\"" + algorithm + blockMode + initialization + "\""
+    )
+  }
+}
+
+import MakeTest<Test>

go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.expected

@@ -1,17 +0,0 @@
-edges
-| Crypto.go:16:9:16:16 | password | Crypto.go:19:25:19:27 | buf | provenance |  |
-| Crypto.go:16:9:16:16 | password | Crypto.go:22:10:22:12 | buf | provenance |  |
-| Crypto.go:16:9:16:16 | password | Crypto.go:25:16:25:18 | buf | provenance |  |
-| Crypto.go:16:9:16:16 | password | Crypto.go:28:11:28:13 | buf | provenance |  |
-nodes
-| Crypto.go:16:9:16:16 | password | semmle.label | password |
-| Crypto.go:19:25:19:27 | buf | semmle.label | buf |
-| Crypto.go:22:10:22:12 | buf | semmle.label | buf |
-| Crypto.go:25:16:25:18 | buf | semmle.label | buf |
-| Crypto.go:28:11:28:13 | buf | semmle.label | buf |
-subpaths
-#select
-| Crypto.go:19:25:19:27 | buf | Crypto.go:16:9:16:16 | password | Crypto.go:19:25:19:27 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:16:9:16:16 | password | Sensitive data |
-| Crypto.go:22:10:22:12 | buf | Crypto.go:16:9:16:16 | password | Crypto.go:22:10:22:12 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:16:9:16:16 | password | Sensitive data |
-| Crypto.go:25:16:25:18 | buf | Crypto.go:16:9:16:16 | password | Crypto.go:25:16:25:18 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:16:9:16:16 | password | Sensitive data |
-| Crypto.go:28:11:28:13 | buf | Crypto.go:16:9:16:16 | password | Crypto.go:28:11:28:13 | buf | $@ is used in a weak cryptographic algorithm. | Crypto.go:16:9:16:16 | password | Sensitive data |

go/ql/test/query-tests/Security/CWE-327/WeakCryptoAlgorithm.qlref

@@ -1 +0,0 @@
-experimental/CWE-327/WeakCryptoAlgorithm.ql

go/ql/test/query-tests/Security/CWE-327/WeakSensitiveDataHashing.expected

@@ -0,0 +1,24 @@
+#select
+| hashing.go:22:8:22:22 | secretByteSlice | hashing.go:22:8:22:22 | secretByteSlice | hashing.go:22:8:22:22 | secretByteSlice | $@ is used in a hashing algorithm (MD5) that is insecure. | hashing.go:22:8:22:22 | secretByteSlice | Sensitive data (secret) |
+| hashing.go:23:10:23:24 | secretByteSlice | hashing.go:23:10:23:24 | secretByteSlice | hashing.go:23:10:23:24 | secretByteSlice | $@ is used in a hashing algorithm (MD5) that is insecure. | hashing.go:23:10:23:24 | secretByteSlice | Sensitive data (secret) |
+| hashing.go:24:20:24:31 | secretString | hashing.go:24:20:24:31 | secretString | hashing.go:24:20:24:31 | secretString | $@ is used in a hashing algorithm (MD5) that is insecure. | hashing.go:24:20:24:31 | secretString | Sensitive data (secret) |
+| hashing.go:25:10:25:24 | secretByteSlice | hashing.go:25:10:25:24 | secretByteSlice | hashing.go:25:10:25:24 | secretByteSlice | $@ is used in a hashing algorithm (MD5) that is insecure. | hashing.go:25:10:25:24 | secretByteSlice | Sensitive data (secret) |
+| hashing.go:27:17:27:31 | secretByteSlice | hashing.go:27:17:27:31 | secretByteSlice | hashing.go:27:17:27:31 | secretByteSlice | $@ is used in a hashing algorithm (SHA1) that is insecure. | hashing.go:27:17:27:31 | secretByteSlice | Sensitive data (secret) |
+| hashing.go:28:11:28:25 | secretByteSlice | hashing.go:28:11:28:25 | secretByteSlice | hashing.go:28:11:28:25 | secretByteSlice | $@ is used in a hashing algorithm (SHA1) that is insecure. | hashing.go:28:11:28:25 | secretByteSlice | Sensitive data (secret) |
+| hashing.go:30:16:30:30 | secretByteSlice | hashing.go:30:16:30:30 | secretByteSlice | hashing.go:30:16:30:30 | secretByteSlice | $@ is used in a hashing algorithm (MD4) that is insecure. | hashing.go:30:16:30:30 | secretByteSlice | Sensitive data (secret) |
+| hashing.go:31:22:31:36 | secretByteSlice | hashing.go:31:22:31:36 | secretByteSlice | hashing.go:31:22:31:36 | secretByteSlice | $@ is used in a hashing algorithm (RIPEMD160) that is insecure. | hashing.go:31:22:31:36 | secretByteSlice | Sensitive data (secret) |
+| hashing.go:82:23:82:38 | type conversion | hashing.go:82:30:82:37 | password | hashing.go:82:23:82:38 | type conversion | $@ is used in a hashing algorithm (SHA256) that is insecure for password hashing, since it is not a computationally expensive hash function. | hashing.go:82:30:82:37 | password | Sensitive data (password) |
+edges
+| hashing.go:82:30:82:37 | password | hashing.go:82:23:82:38 | type conversion | provenance |  |
+nodes
+| hashing.go:22:8:22:22 | secretByteSlice | semmle.label | secretByteSlice |
+| hashing.go:23:10:23:24 | secretByteSlice | semmle.label | secretByteSlice |
+| hashing.go:24:20:24:31 | secretString | semmle.label | secretString |
+| hashing.go:25:10:25:24 | secretByteSlice | semmle.label | secretByteSlice |
+| hashing.go:27:17:27:31 | secretByteSlice | semmle.label | secretByteSlice |
+| hashing.go:28:11:28:25 | secretByteSlice | semmle.label | secretByteSlice |
+| hashing.go:30:16:30:30 | secretByteSlice | semmle.label | secretByteSlice |
+| hashing.go:31:22:31:36 | secretByteSlice | semmle.label | secretByteSlice |
+| hashing.go:82:23:82:38 | type conversion | semmle.label | type conversion |
+| hashing.go:82:30:82:37 | password | semmle.label | password |
+subpaths

go/ql/test/query-tests/Security/CWE-327/WeakSensitiveDataHashing.qlref

@@ -0,0 +1,4 @@
+query: Security/CWE-327/WeakSensitiveDataHashing.ql
+postprocess:
+  - utils/test/PrettyPrintModels.ql
+  - utils/test/InlineExpectationsTestQuery.ql

go/ql/test/query-tests/Security/CWE-327/encryption.go

@@ -0,0 +1,167 @@
+package main
+
+import (
+	"bytes"
+	"crypto/aes"
+	"crypto/cipher"
+	"crypto/des"
+	"crypto/rc4"
+	"io"
+	"os"
+)
+
+var dst []byte = make([]byte, 16)
+var secretByteSlice []byte = []byte("")
+
+const secretString string = ""
+
+var public []byte = []byte("")
+
+func getUserID() []byte {
+	return []byte("")
+}
+
+// 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, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. init from 2 lines above."
+	block.Decrypt(dst, secretByteSlice)
+
+	gcm1, _ := cipher.NewGCM(block)
+	gcm1.Seal(nil, nil, secretByteSlice, nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. init from 6 lines above."
+	gcm1.Open(nil, nil, secretByteSlice, nil)
+
+	gcm2, _ := cipher.NewGCMWithNonceSize(block, 12)
+	gcm2.Seal(nil, nil, secretByteSlice, nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. init from 10 lines above."
+	gcm2.Open(nil, nil, secretByteSlice, nil)
+
+	gcm3, _ := cipher.NewGCMWithRandomNonce(block)
+	gcm3.Seal(nil, nil, secretByteSlice, nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. init from 14 lines above."
+	gcm3.Open(nil, nil, secretByteSlice, nil)
+
+	gcm4, _ := cipher.NewGCMWithTagSize(block, 12)
+	gcm4.Seal(nil, nil, secretByteSlice, nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. init from 18 lines above."
+	gcm4.Open(nil, nil, secretByteSlice, nil)
+
+	cbcEncrypter := cipher.NewCBCEncrypter(block, nil)
+	cbcEncrypter.CryptBlocks(dst, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: CBC. init from 1,22 lines above."
+	cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, secretByteSlice)
+
+	ctrStream := cipher.NewCTR(block, nil)
+	ctrStream.XORKeyStream(dst, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: CTR. init from 1,26 lines above."
+
+	ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(secretByteSlice)} // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: CTR. init from 28,3 lines above."
+	io.Copy(os.Stdout, ctrStreamReader)
+
+	ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: CTR. init from 31,6 lines above."
+	io.Copy(ctrStreamWriter, bytes.NewReader(secretByteSlice))          // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: CTR. init from 32,7 lines above."
+
+	// deprecated
+
+	cfbStream := cipher.NewCFBEncrypter(block, nil)
+	cfbStream.XORKeyStream(dst, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: CFB. init from 1,37 lines above."
+	cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, secretByteSlice)
+
+	ofbStream := cipher.NewOFB(block, nil)
+	ofbStream.XORKeyStream(dst, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="DES. blockMode: OFB. init from 1,41 lines above."
+}
+
+func BlockCipherTripleDes() {
+	// BAD, triple des is a weak crypto algorithm and secretByteSlice is sensitive data
+	block, _ := des.NewTripleDESCipher(nil)
+
+	block.Encrypt(dst, getUserID()) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. init from 2 lines above."
+	block.Decrypt(dst, getUserID())
+
+	gcm1, _ := cipher.NewGCM(block)
+	gcm1.Seal(nil, nil, getUserID(), nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. init from 6 lines above."
+	gcm1.Open(nil, nil, getUserID(), nil)
+
+	gcm2, _ := cipher.NewGCMWithNonceSize(block, 12)
+	gcm2.Seal(nil, nil, getUserID(), nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. init from 10 lines above."
+	gcm2.Open(nil, nil, getUserID(), nil)
+
+	gcm3, _ := cipher.NewGCMWithRandomNonce(block)
+	gcm3.Seal(nil, nil, secretByteSlice, nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. init from 14 lines above."
+	gcm3.Open(nil, nil, secretByteSlice, nil)
+
+	gcm4, _ := cipher.NewGCMWithTagSize(block, 12)
+	gcm4.Seal(nil, nil, secretByteSlice, nil) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. init from 18 lines above."
+	gcm4.Open(nil, nil, secretByteSlice, nil)
+
+	cbcEncrypter := cipher.NewCBCEncrypter(block, nil)
+	cbcEncrypter.CryptBlocks(dst, getUserID()) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CBC. init from 1,22 lines above."
+	cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, getUserID())
+
+	ctrStream := cipher.NewCTR(block, nil)
+	ctrStream.XORKeyStream(dst, getUserID()) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from 1,26 lines above."
+
+	ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(getUserID())} // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from 28,3 lines above."
+	io.Copy(os.Stdout, ctrStreamReader)
+
+	ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from 31,6 lines above."
+	io.Copy(ctrStreamWriter, bytes.NewReader(getUserID()))              // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CTR. init from 32,7 lines above."
+
+	// deprecated
+
+	cfbStream := cipher.NewCFBEncrypter(block, nil)
+	cfbStream.XORKeyStream(dst, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: CFB. init from 1,37 lines above."
+	cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, secretByteSlice)
+
+	ofbStream := cipher.NewOFB(block, nil)
+	ofbStream.XORKeyStream(dst, secretByteSlice) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="TRIPLEDES. blockMode: OFB. init from 1,41 lines above."
+}
+
+func BlockCipherAes() {
+	// GOOD, aes is a strong crypto algorithm
+	block, _ := aes.NewCipher(nil)
+
+	block.Encrypt(dst, secretByteSlice) // $ CryptographicOperation="AES. init from 2 lines above."
+	block.Decrypt(dst, secretByteSlice)
+
+	gcm1, _ := cipher.NewGCM(block)
+	gcm1.Seal(nil, nil, secretByteSlice, nil) // $ CryptographicOperation="AES. init from 6 lines above."
+	gcm1.Open(nil, nil, secretByteSlice, nil)
+
+	gcm2, _ := cipher.NewGCMWithNonceSize(block, 12)
+	gcm2.Seal(nil, nil, secretByteSlice, nil) // $ CryptographicOperation="AES. init from 10 lines above."
+	gcm2.Open(nil, nil, secretByteSlice, nil)
+
+	gcm3, _ := cipher.NewGCMWithRandomNonce(block)
+	gcm3.Seal(nil, nil, secretByteSlice, nil) // $ CryptographicOperation="AES. init from 14 lines above."
+	gcm3.Open(nil, nil, secretByteSlice, nil)
+
+	gcm4, _ := cipher.NewGCMWithTagSize(block, 12)
+	gcm4.Seal(nil, nil, secretByteSlice, nil) // $ CryptographicOperation="AES. init from 18 lines above."
+	gcm4.Open(nil, nil, secretByteSlice, nil)
+
+	cbcEncrypter := cipher.NewCBCEncrypter(block, nil)
+	cbcEncrypter.CryptBlocks(dst, secretByteSlice) // $ CryptographicOperation="AES. blockMode: CBC. init from 1,22 lines above."
+	cipher.NewCBCDecrypter(block, nil).CryptBlocks(dst, secretByteSlice)
+
+	ctrStream := cipher.NewCTR(block, nil)
+	ctrStream.XORKeyStream(dst, secretByteSlice) // $ CryptographicOperation="AES. blockMode: CTR. init from 1,26 lines above."
+
+	ctrStreamReader := &cipher.StreamReader{S: ctrStream, R: bytes.NewReader(secretByteSlice)} // $ CryptographicOperation="AES. blockMode: CTR. init from 28,3 lines above."
+	io.Copy(os.Stdout, ctrStreamReader)
+
+	ctrStreamWriter := &cipher.StreamWriter{S: ctrStream, W: os.Stdout} // $ CryptographicOperation="AES. blockMode: CTR. init from 31,6 lines above."
+	io.Copy(ctrStreamWriter, bytes.NewReader(secretByteSlice))          // $ CryptographicOperation="AES. blockMode: CTR. init from 32,7 lines above."
+
+	// deprecated
+
+	cfbStream := cipher.NewCFBEncrypter(block, nil)
+	cfbStream.XORKeyStream(dst, secretByteSlice) // $ CryptographicOperation="AES. blockMode: CFB. init from 1,37 lines above."
+	cipher.NewCFBDecrypter(block, nil).XORKeyStream(dst, secretByteSlice)
+
+	ofbStream := cipher.NewOFB(block, nil)
+	ofbStream.XORKeyStream(dst, secretByteSlice) // $ CryptographicOperation="AES. blockMode: OFB. init from 1,41 lines above."
+}
+
+func CipherRc4() {
+	c, _ := rc4.NewCipher(nil)
+	c.XORKeyStream(dst, getUserID()) // $ Alert[go/weak-cryptographic-algorithm] CryptographicOperation="RC4. init from 0 lines above."
+}

go/ql/test/query-tests/Security/CWE-327/go.mod

@@ -0,0 +1,5 @@
+module test
+
+go 1.24.0
+
+require golang.org/x/crypto v0.43.0

go/ql/test/query-tests/Security/CWE-327/hashing.go

@@ -0,0 +1,91 @@
+package main
+
+//go:generate depstubber -vendor golang.org/x/crypto/md4 "" New
+//go:generate depstubber -vendor golang.org/x/crypto/ripemd160 "" New
+
+import (
+	"crypto/md5"
+	"crypto/pbkdf2"
+	"crypto/rand"
+	"crypto/sha1"
+	"crypto/sha256"
+	"crypto/sha3"
+	"crypto/sha512"
+	"io"
+
+	"golang.org/x/crypto/md4"
+	"golang.org/x/crypto/ripemd160"
+)
+
+func WeakHashes() {
+	h := md5.New()
+	h.Sum(secretByteSlice)          // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="MD5. init from 1 lines above."
+	h.Write(secretByteSlice)        // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="MD5. init from 2 lines above."
+	io.WriteString(h, secretString) // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="MD5. init from 3 lines above."
+	md5.Sum(secretByteSlice)        // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="MD5. init from 0 lines above."
+
+	sha1.New().Sum(secretByteSlice) // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="SHA1. init from 0 lines above."
+	sha1.Sum(secretByteSlice)       // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="SHA1. init from 0 lines above."
+
+	md4.New().Sum(secretByteSlice)       // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="MD4. init from 0 lines above."
+	ripemd160.New().Sum(secretByteSlice) // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="RIPEMD160. init from 0 lines above."
+
+	// Only alert when sensitive data is hashed.
+	md5.New().Sum(public)  // $ CryptographicOperation="MD5. init from 0 lines above."
+	md5.Sum(public)        // $ CryptographicOperation="MD5. init from 0 lines above."
+	sha1.New().Sum(public) // $ CryptographicOperation="SHA1. init from 0 lines above."
+	sha1.Sum(public)       // $ CryptographicOperation="SHA1. init from 0 lines above."
+}
+
+func StrongHashes() {
+	sha256.New224().Sum(secretByteSlice) // $ CryptographicOperation="SHA224. init from 0 lines above."
+	sha256.Sum224(secretByteSlice)       // $ CryptographicOperation="SHA224. init from 0 lines above."
+
+	sha256.New().Sum(secretByteSlice) // $ CryptographicOperation="SHA256. init from 0 lines above."
+	sha256.Sum256(secretByteSlice)    // $ CryptographicOperation="SHA256. init from 0 lines above."
+
+	sha512.New().Sum(secretByteSlice) // $ CryptographicOperation="SHA512. init from 0 lines above."
+	sha512.Sum512(secretByteSlice)    // $ CryptographicOperation="SHA512. init from 0 lines above."
+
+	sha512.New384().Sum(secretByteSlice) // $ CryptographicOperation="SHA384. init from 0 lines above."
+	sha512.Sum384(secretByteSlice)       // $ CryptographicOperation="SHA384. init from 0 lines above."
+
+	sha512.New512_224().Sum(secretByteSlice) // $ CryptographicOperation="SHA512224. init from 0 lines above."
+	sha512.Sum512_224(secretByteSlice)       // $ CryptographicOperation="SHA512224. init from 0 lines above."
+
+	sha512.New512_256().Sum(secretByteSlice) // $ CryptographicOperation="SHA512256. init from 0 lines above."
+	sha512.Sum512_256(secretByteSlice)       // $ CryptographicOperation="SHA512256. init from 0 lines above."
+
+	sha3.New224().Sum(secretByteSlice) // $ CryptographicOperation="SHA3224. init from 0 lines above."
+	sha3.Sum224(secretByteSlice)       // $ CryptographicOperation="SHA3224. init from 0 lines above."
+
+	sha3.New256().Sum(secretByteSlice) // $ CryptographicOperation="SHA3256. init from 0 lines above."
+	sha3.Sum256(secretByteSlice)       // $ CryptographicOperation="SHA3256. init from 0 lines above."
+
+	sha3.New384().Sum(secretByteSlice) // $ CryptographicOperation="SHA3384. init from 0 lines above."
+	sha3.Sum384(secretByteSlice)       // $ CryptographicOperation="SHA3384. init from 0 lines above."
+
+	sha3.New512().Sum(secretByteSlice) // $ CryptographicOperation="SHA3512. init from 0 lines above."
+	sha3.Sum512(secretByteSlice)       // $ CryptographicOperation="SHA3512. init from 0 lines above."
+
+	sha3.NewSHAKE128().Write(secretByteSlice)          // $ CryptographicOperation="SHAKE128. init from 0 lines above."
+	sha3.NewCSHAKE128(nil, nil).Write(secretByteSlice) // $ CryptographicOperation="SHAKE128. init from 0 lines above."
+	sha3.SumSHAKE128(secretByteSlice, 100)             // $ CryptographicOperation="SHAKE128. init from 0 lines above."
+
+	sha3.NewSHAKE256().Write(secretByteSlice)          // $ CryptographicOperation="SHAKE256. init from 0 lines above."
+	sha3.NewCSHAKE256(nil, nil).Write(secretByteSlice) // $ CryptographicOperation="SHAKE256. init from 0 lines above."
+	sha3.SumSHAKE256(secretByteSlice, 100)             // $ CryptographicOperation="SHAKE256. init from 0 lines above."
+}
+
+func GetPasswordHashBad(password string) [32]byte {
+	// BAD, SHA256 is a strong hashing algorithm but it is not computationally expensive
+	return sha256.Sum256([]byte(password)) // $ Alert[go/weak-sensitive-data-hashing] CryptographicOperation="SHA256. init from 0 lines above."
+}
+
+func GetPasswordHashGood(password string) []byte {
+	// GOOD, PBKDF2 is a strong hashing algorithm and it is computationally expensive
+	salt := make([]byte, 16)
+	rand.Read(salt)
+	key, _ := pbkdf2.Key(sha512.New, password, salt, 4096, 32)
+	return key
+}

go/ql/test/query-tests/Security/CWE-327/vendor/golang.org/x/crypto/md4/stub.go

@@ -0,0 +1,16 @@
+// Code generated by depstubber. DO NOT EDIT.
+// This is a simple stub for golang.org/x/crypto/md4, strictly for use in testing.
+
+// See the LICENSE file for information about the licensing of the original library.
+// Source: golang.org/x/crypto/md4 (exports: ; functions: New)
+
+// Package md4 is a stub of golang.org/x/crypto/md4, generated by depstubber.
+package md4
+
+import (
+	hash "hash"
+)
+
+func New() hash.Hash {
+	return nil
+}

go/ql/test/query-tests/Security/CWE-327/vendor/golang.org/x/crypto/ripemd160/stub.go

@@ -0,0 +1,16 @@
+// Code generated by depstubber. DO NOT EDIT.
+// This is a simple stub for golang.org/x/crypto/ripemd160, strictly for use in testing.
+
+// See the LICENSE file for information about the licensing of the original library.
+// Source: golang.org/x/crypto/ripemd160 (exports: ; functions: New)
+
+// Package ripemd160 is a stub of golang.org/x/crypto/ripemd160, generated by depstubber.
+package ripemd160
+
+import (
+	hash "hash"
+)
+
+func New() hash.Hash {
+	return nil
+}

go/ql/test/query-tests/Security/CWE-327/vendor/modules.txt

@@ -0,0 +1,4 @@
+# golang.org/x/crypto v0.43.0
+## explicit
+golang.org/x/crypto/md4
+golang.org/x/crypto/ripemd160

ruby/ql/src/queries/security/cwe-327/WeakSensitiveDataHashing.qhelp

@@ -67,7 +67,7 @@
                The following example shows two functions for checking whether the hash
                of a certificate matches a known value -- to prevent tampering.
 
-               The first function uses MD5 that is known to be vulnerable to collision attacks.
+               The first function uses SHA-1 that is known to be vulnerable to collision attacks.
 
                The second function uses SHA-256 that is a strong cryptographic hashing function.
           </p>

ruby/ql/test/library-tests/security/CryptoAlgorithms.expected

@@ -34,6 +34,8 @@ strongHashingAlgorithms
 | SHA3256 |
 | SHA3384 |
 | SHA3512 |
+| SHA512224 |
+| SHA512256 |
 | SHAKE128 |
 | SHAKE256 |
 | SM3 |

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