fixing commit error

ql/src/experimental/CWE-327/BrokenCrypto.ql

@@ -1,15 +0,0 @@
-/**
- * @name Use of a broken or risky cryptographic algorithm
- * @description Using broken or weak cryptographic algorithms can allow an attacker to compromise security.
- * @kind problem
- * @problem.severity error
- * @id go/broken-crypto
- * @tags security
- */
-
-import go
-import CryptoLibraries
-
-from CryptographicOperation badcrypto
-where badcrypto.getAlgorithm().isWeak()
-select badcrypto, "Use of weak or broken cryptographic algorithm"

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

@@ -6,20 +6,84 @@ 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 Wikipedia, OWASP and google (2017).
+ *
+ * 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" or
+    name = "ED25519" or
+    name = "ES256" or
+    name = "ECDSA256" or
+    name = "ES384" or
+    name = "ECDSA384" or
+    name = "ES512" or
+    name = "ECDSA512" or
+    name = "SHA2" or
+    name = "SHA224" or
+    name = "SHA256" or
+    name = "SHA384" or
+    name = "SHA512" or
+    name = "SHA3"
+  }
+
   predicate isWeakHashingAlgorithm(string name) {
+    name = "HAVEL128" or
+    name = "MD2" or
+    name = "MD4" or
     name = "MD5" or
+    name = "PANAMA" or
+    name = "RIPEMD" or
+    name = "RIPEMD128" or
+    name = "RIPEMD256" or
+    name = "RIPEMD320" or
+    name = "SHA0" or
     name = "SHA1"
   }
 
+  predicate isStrongEncryptionAlgorithm(string name) {
+    name = "AES" or
+    name = "AES128" or
+    name = "AES192" or
+    name = "AES256" or
+    name = "AES512" or
+    name = "RSA" or
+    name = "RABBIT" or
+    name = "BLOWFISH"
+  }
+
   predicate isWeakEncryptionAlgorithm(string name) {
     name = "DES" or
-    name = "RC4"
+    name = "3DES" or
+    name = "TRIPLEDES" or
+    name = "TDEA" or
+    name = "TRIPLEDEA" or
+    name = "ARC2" or
+    name = "RC2" or
+    name = "ARC4" or
+    name = "RC4" or
+    name = "ARCFOUR" or
+    name = "ARC5" or
+    name = "RC5"
   }
+
+  predicate isStrongPasswordHashingAlgorithm(string name) {
+    name = "ARGON2" or
+    name = "PBKDF2" or
+    name = "BCRYPT" or
+    name = "SCRYPT"
+  }
+
+  predicate isWeakPasswordHashingAlgorithm(string name) { none() }
 }
 
 private import AlgorithmNames
@@ -28,9 +92,20 @@ private import AlgorithmNames
  * A cryptographic algorithm.
  */
 private newtype TCryptographicAlgorithm =
-  MkHashingAlgorithm(string name, boolean isWeak) { isWeakHashingAlgorithm(name) and isWeak = true } or
+  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
   }
 
 /**
@@ -46,11 +121,13 @@ abstract class CryptographicAlgorithm extends TCryptographicAlgorithm {
   abstract string getName();
 
   /**
-   * Holds if the name of this algorithm matches `name` modulo case, white space, dashes and underscores.
+   * 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" + getName() + "\\E", _, _))
+    // name.toUpperCase().regexpReplaceAll("[-_ ]", "").regexpMatch(".*" + getName() + ".*")
   }
 
   /**
@@ -60,7 +137,7 @@ abstract class CryptographicAlgorithm extends TCryptographicAlgorithm {
 }
 
 /**
- * A hashing algorithm
+ * A hashing algorithm such as `MD5` or `SHA512`.
  */
 class HashingAlgorithm extends MkHashingAlgorithm, CryptographicAlgorithm {
   string name;
@@ -74,7 +151,7 @@ class HashingAlgorithm extends MkHashingAlgorithm, CryptographicAlgorithm {
 }
 
 /**
- * An encryption algorithm
+ * An encryption algorithm such as `DES` or `AES512`.
  */
 class EncryptionAlgorithm extends MkEncryptionAlgorithm, CryptographicAlgorithm {
   string name;
@@ -87,10 +164,29 @@ class EncryptionAlgorithm extends MkEncryptionAlgorithm, CryptographicAlgorithm
   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 Expr {
+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.
    */
@@ -98,86 +194,70 @@ abstract class CryptographicOperation extends Expr {
 }
 
 /**
+ * Below are the cryptographic functions that have been implemented so far for this library.
  * Class that checks for use of Md5 package.
  */
-class Md5 extends CryptographicOperation {
+class Md5 extends CryptographicOperation, DataFlow::CallNode {
+  Expr input;
   CryptographicAlgorithm algorithm;
-  SelectorExpr sel;
-  CallExpr call;
 
   Md5() {
-    this = call and
-    algorithm.matchesName(sel.getBase().toString()) and
-    algorithm.matchesName("MD5") and
-    sel.getSelector().toString() = call.getCalleeName().toString() and
-    (
-      call.getCalleeName().toString() = "New" or
-      call.getCalleeName().toString() = "Sum"
-    )
+    getTarget().hasQualifiedName("crypto/md5", ["New", "Sum"]) and
+    this.getArgument(0).asExpr() = input
   }
 
+  override Expr getInput() { result = input }
+
   override CryptographicAlgorithm getAlgorithm() { result = algorithm }
 }
 
 /**
- * Class that checks for use of SHA1 package.
+ * Class that checks for use of Sha1 package.
  */
-class Sha1 extends CryptographicOperation {
+class Sha1 extends CryptographicOperation, DataFlow::CallNode {
+  Expr input;
   CryptographicAlgorithm algorithm;
-  SelectorExpr sel;
-  CallExpr call;
 
   Sha1() {
-    this = call and
-    algorithm.matchesName(sel.getBase().toString()) and
-    algorithm.matchesName("SHA1") and
-    sel.getSelector().toString() = call.getCalleeName().toString() and
-    (
-      call.getCalleeName().toString() = "New" or
-      call.getCalleeName().toString() = "Sum"
-    )
+    getTarget().hasQualifiedName("crypto/sha1", ["New", "Sum"]) and
+    this.getArgument(0).asExpr() = input
   }
 
+  override Expr getInput() { result = input }
+
   override CryptographicAlgorithm getAlgorithm() { result = algorithm }
 }
 
 /**
  * Class that checks for use of Des package.
  */
-class Des extends CryptographicOperation {
+class Des extends CryptographicOperation, DataFlow::CallNode {
+  Expr input;
   CryptographicAlgorithm algorithm;
-  SelectorExpr sel;
-  CallExpr call;
 
   Des() {
-    this = call and
-    algorithm.matchesName(sel.getBase().toString()) and
-    algorithm.matchesName("DES") and
-    sel.getSelector().toString() = call.getCalleeName().toString() and
-    (
-      call.getCalleeName().toString() = "NewCipher" or
-      call.getCalleeName().toString() = "NewTripleDESCipher"
-    )
+    getTarget().hasQualifiedName("crypto/des", ["NewCipher", "NewTripleDESCipher"]) and
+    this.getArgument(0).asExpr() = input
   }
 
+  override Expr getInput() { result = input }
+
   override CryptographicAlgorithm getAlgorithm() { result = algorithm }
 }
 
 /**
- * Class that checks for use of RC4 package.
+ * Class that checks for use of Rc4 package.
  */
-class Rc4 extends CryptographicOperation {
+class Rc4 extends CryptographicOperation, DataFlow::CallNode {
+  Expr input;
   CryptographicAlgorithm algorithm;
-  SelectorExpr sel;
-  CallExpr call;
 
   Rc4() {
-    this = call and
-    algorithm.matchesName(sel.getBase().toString()) and
-    algorithm.matchesName("RC4") and
-    sel.getSelector().toString() = call.getCalleeName().toString() and
-    call.getCalleeName().toString() = "NewCipher"
+    getTarget().hasQualifiedName("crypto/rc4", ["NewCipher"]) and
+    this.getArgument(0).asExpr() = input
   }
 
+  override Expr getInput() { result = input }
+
   override CryptographicAlgorithm getAlgorithm() { result = algorithm }
 }

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

@@ -0,0 +1,55 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+     <overview>
+          <p>
+               Using weak cryptographic algorithms can leave data
+               vulnerable to being decrypted or forged by an attacker.
+          </p>
+
+          <p>
+               Many cryptographic algorithms provided by cryptography
+               libraries are known to be weak. Using such an
+               algorithm means that encrypted or hashed data is less
+               secure than it appears to be.
+          </p>
+
+     </overview>
+     <recommendation>
+
+          <p>
+               Ensure that you use a strong, modern cryptographic
+               algorithm. Use at least AES-128 or RSA-2048 for
+               encryption, and SHA-2 or SHA-3 for secure hashing.
+          </p>
+
+     </recommendation>
+     <example>
+
+          <p>
+               The following code uses the different packages to encrypt/hash 
+               some secret data. The first few examples uses DES, MD5, RC4, and SHA1, 
+               which are older algorithms that are now considered weak. The following
+               examples use AES and SHA256, which are stronger, more modern algorithms.
+          </p>
+
+          <sample src="examples/Crypto.go" />
+
+     </example>
+
+     <references>
+          <li>OWASP: <a
+          href="https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html">Cryptographic Storage Cheat Sheet</a>.
+          </li>
+          <li>Wikipedia: <a
+          href="https://en.wikipedia.org/wiki/Strong_cryptography#Cryptographically_strong_algorithms">Cryptographically Strong Algorithms</a>.
+          </li>
+          <li>Wikipedia: <a
+          href="https://en.wikipedia.org/wiki/Strong_cryptography#Examples">Strong Cryptography Examples</a>.
+          </li>
+          <li>NIST, FIPS 140 Annex a: <a href="http://csrc.nist.gov/publications/fips/fips140-2/fips1402annexa.pdf"> Approved Security Functions</a>.</li>
+          <li>NIST, SP 800-131A: <a href="http://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar1.pdf"> Transitions: Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Lengths</a>.</li>
+     </references>
+
+</qhelp>

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

@@ -0,0 +1,16 @@
+/**
+ * @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
+ */
+
+import go
+import WeakCryptoAlgorithmCustomizations::WeakCryptoAlgorithm
+import DataFlow::PathGraph
+
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Sensitive data is used in a weak cryptographic algorithm."

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

@@ -0,0 +1,61 @@
+/**
+ * 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
+
+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()
+      )
+    }
+  }
+
+  /**
+   * A configuration depicting taint flow from sensitive information to weak cryptographic algorithms.
+   */
+  class Configuration extends TaintTracking::Configuration {
+    Configuration() { this = "WeakCryptoAlgorithm" }
+
+    override predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+    override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+    override predicate isSanitizer(DataFlow::Node node) {
+      super.isSanitizer(node) or
+      node instanceof Sanitizer
+    }
+  }
+}

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

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"crypto/aes"
 	"crypto/des"
 	"crypto/md5"
 	"crypto/rc4"
@@ -9,23 +10,24 @@ import (
 )
 
 func main() {
-
 	password := []byte("password")
+
 	// BAD, des is a weak crypto algorithm
 	des.NewTripleDESCipher(password)
 
 	// BAD, md5 is a weak crypto algorithm
 	md5.Sum(password)
 
-	key := []byte{ 1, 2, 3, 4, 5, 6, 7 }
 	// BAD, rc4 is a weak crypto algorithm
-	rc4.NewCipher(key)
+	rc4.NewCipher(password)
 
-	data := []byte("this page intentionally left blank.")
 	// BAD, sha1 is a weak crypto algorithm
-	sha1.Sum(data)
+	sha1.Sum(password)
+
+	// GOOD, aes is a strong crypto algorithm
+	aes.NewCipher(password)
+
+	// GOOD, sha256 is a strong crypto algorithm
+	sha256.Sum256(password)
 
-	sha256key := []byte("hello world")
-    // GOOD, sha256 is a strong crypto algorithm
-    sha256.Sum256([]byte(sha256key))
 }

ql/test/experimental/CWE-327/BrokenCrypto.expected

@@ -1,4 +0,0 @@
-| BadCrypto.go:15:2:15:33 | call to NewTripleDESCipher | Use of weak or broken cryptographic algorithm |
-| BadCrypto.go:18:2:18:18 | call to Sum | Use of weak or broken cryptographic algorithm |
-| BadCrypto.go:22:2:22:19 | call to NewCipher | Use of weak or broken cryptographic algorithm |
-| BadCrypto.go:26:2:26:15 | call to Sum | Use of weak or broken cryptographic algorithm |

ql/test/experimental/CWE-327/BrokenCrypto.qlref

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

ql/test/experimental/CWE-327/Crypto.go

@@ -0,0 +1,53 @@
+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)
+}

ql/test/experimental/CWE-327/WeakCryptoAlgorithm.expected

@@ -0,0 +1,16 @@
+edges
+| Crypto.go:16:9:16:16 | password : slice type | Crypto.go:19:25:19:27 | buf |
+| Crypto.go:16:9:16:16 | password : slice type | Crypto.go:22:10:22:12 | buf |
+| Crypto.go:16:9:16:16 | password : slice type | Crypto.go:25:16:25:18 | buf |
+| Crypto.go:16:9:16:16 | password : slice type | Crypto.go:28:11:28:13 | buf |
+nodes
+| Crypto.go:16:9:16:16 | password : slice type | semmle.label | password : slice type |
+| 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 |
+#select
+| Crypto.go:19:25:19:27 | buf | Crypto.go:16:9:16:16 | password : slice type | Crypto.go:19:25:19:27 | buf | Sensitive data is used in a weak cryptographic algorithm. |
+| Crypto.go:22:10:22:12 | buf | Crypto.go:16:9:16:16 | password : slice type | Crypto.go:22:10:22:12 | buf | Sensitive data is used in a weak cryptographic algorithm. |
+| Crypto.go:25:16:25:18 | buf | Crypto.go:16:9:16:16 | password : slice type | Crypto.go:25:16:25:18 | buf | Sensitive data is used in a weak cryptographic algorithm. |
+| Crypto.go:28:11:28:13 | buf | Crypto.go:16:9:16:16 | password : slice type | Crypto.go:28:11:28:13 | buf | Sensitive data is used in a weak cryptographic algorithm. |

ql/test/experimental/CWE-327/WeakCryptoAlgorithm.qlref

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