Merge pull request #8421 from alexrford/ruby/weak-cryptographic-algorithm

Ruby: Add `rb/weak-cryptographic-algorithm` query

ruby/ql/lib/codeql/ruby/Concepts.qll

@@ -816,3 +816,54 @@ module Logging {
     abstract DataFlow::Node getAnInput();
   }
 }
+
+/**
+ * Provides models for cryptographic concepts.
+ *
+ * Note: The `CryptographicAlgorithm` class currently doesn't take weak keys into
+ * consideration for the `isWeak` member predicate. So RSA is always considered
+ * secure, although using a low number of bits will actually make it insecure. We plan
+ * to improve our libraries in the future to more precisely capture this aspect.
+ */
+module Cryptography {
+  import security.CryptoAlgorithms
+
+  /**
+   * 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 DataFlow::Node instanceof CryptographicOperation::Range {
+    /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
+    CryptographicAlgorithm getAlgorithm() { result = super.getAlgorithm() }
+
+    /** Gets an input the algorithm is used on, for example the plain text input to be encrypted. */
+    DataFlow::Node getAnInput() { result = super.getAnInput() }
+
+    /** Holds if this encryption operation is known to be weak. */
+    predicate isWeak() { super.isWeak() }
+  }
+
+  /** Provides classes for modeling new applications of a cryptographic algorithms. */
+  module CryptographicOperation {
+    /**
+     * A data-flow node that is an application of a cryptographic algorithm. For example,
+     * encryption, decryption, signature-validation.
+     *
+     * Extend this class to model new APIs. If you want to refine existing API models,
+     * extend `CryptographicOperation` instead.
+     */
+    abstract class Range extends DataFlow::Node {
+      /** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
+      abstract CryptographicAlgorithm getAlgorithm();
+
+      /** Gets an input the algorithm is used on, for example the plain text input to be encrypted. */
+      abstract DataFlow::Node getAnInput();
+
+      /** Holds if this encryption operation is known to be weak. */
+      abstract predicate isWeak();
+    }
+  }
+}

ruby/ql/lib/codeql/ruby/security/OpenSSL.qll

@@ -4,6 +4,10 @@
  */
 
 private import internal.CryptoAlgorithmNames
+private import codeql.ruby.Concepts
+private import codeql.ruby.DataFlow
+private import codeql.ruby.ApiGraphs
+private import codeql.ruby.typetracking.TypeTracker
 
 bindingset[algorithmString]
 private string algorithmRegex(string algorithmString) {
@@ -308,4 +312,241 @@ class OpenSSLCipher extends MkOpenSSLCipher {
 
   /** Gets a textual representation of this element. */
   string toString() { result = this.getCanonicalName() }
+
+  /** Holds if the specified name represents this cipher. */
+  bindingset[candidateName]
+  predicate matchesName(string candidateName) {
+    this.getCanonicalName() = getCanonicalCipherName(candidateName)
+  }
+
+  /** Gets the encryption algorithm used by this cipher. */
+  Cryptography::EncryptionAlgorithm getAlgorithm() { result.matchesName(this.getCanonicalName()) }
+}
+
+/** `OpenSSL::Cipher` or `OpenSSL::Cipher::Cipher` */
+private API::Node cipherApi() {
+  result = API::getTopLevelMember("OpenSSL").getMember("Cipher") or
+  result = API::getTopLevelMember("OpenSSL").getMember("Cipher").getMember("Cipher")
+}
+
+private class BlockMode extends string {
+  BlockMode() { this = ["ECB", "CBC", "GCM", "CCM", "CFB", "OFB", "CTR"] }
+}
+
+private newtype TCipherMode =
+  TStreamCipher() or
+  TBlockMode(BlockMode blockMode)
+
+/**
+ * Represents the mode used by this stream cipher.
+ * If this cipher uses a block encryption algorithm, then this is a specific
+ * block mode.
+ */
+private class CipherMode extends TCipherMode {
+  private BlockMode getBlockMode() { this = TBlockMode(result) }
+
+  /** Gets a textual representation of this node. */
+  string toString() {
+    result = this.getBlockMode()
+    or
+    this = TStreamCipher() and result = "<stream cipher>"
+  }
+
+  /**
+   * Holds if the string `s`, after normalization, represents the block mode
+   * used by this cipher.
+   */
+  bindingset[s]
+  predicate isBlockMode(string s) { this.getBlockMode() = s.toUpperCase() }
+
+  /** Holds if this cipher mode is a weak block mode. */
+  predicate isWeak() { isWeakBlockMode(this.getBlockMode()) }
+}
+
+private string getStringArgument(DataFlow::CallNode call, int i) {
+  result = call.getArgument(i).asExpr().getConstantValue().getStringlikeValue()
+}
+
+private int getIntArgument(DataFlow::CallNode call, int i) {
+  result = call.getArgument(i).asExpr().getConstantValue().getInt()
+}
+
+/**
+ * Holds if `call` is a call to `OpenSSL::Cipher.new` that instantiates a
+ * `cipher` instance with mode `cipherMode`.
+ */
+private predicate cipherInstantiationGeneric(
+  DataFlow::CallNode call, OpenSSLCipher cipher, CipherMode cipherMode
+) {
+  exists(string cipherName | cipher.matchesName(cipherName) |
+    // `OpenSSL::Cipher.new('<cipherName>')`
+    call = cipherApi().getAnInstantiation() and
+    cipherName = getStringArgument(call, 0) and
+    // CBC is used by default
+    cipherMode.isBlockMode("CBC")
+  )
+}
+
+/**
+ * Holds if `call` is a call to `OpenSSL::Cipher::AES.new` or
+ * `OpenSSL::Cipher::AES{128,192,256}.new` that instantiates an AES `cipher` instance
+ * with mode `cipherMode`.
+ */
+private predicate cipherInstantiationAES(
+  DataFlow::CallNode call, OpenSSLCipher cipher, CipherMode cipherMode
+) {
+  exists(string cipherName | cipher.matchesName(cipherName) |
+    // `OpenSSL::Cipher::AES` instantiations
+    call = cipherApi().getMember("AES").getAnInstantiation() and
+    exists(string keyLength, string blockMode |
+      // `OpenSSL::Cipher::AES.new('<keyLength-blockMode>')
+      exists(string arg0 |
+        arg0 = getStringArgument(call, 0) and
+        keyLength = arg0.splitAt("-", 0) and
+        blockMode = arg0.splitAt("-", 1).toUpperCase()
+      )
+      or
+      // `OpenSSL::Cipher::AES.new(<keyLength>, '<blockMode>')`
+      keyLength = getIntArgument(call, 0).toString() and
+      blockMode = getStringArgument(call, 1).toUpperCase()
+    |
+      cipherName = "AES-" + keyLength + "-" + blockMode and
+      cipherMode.isBlockMode(blockMode)
+    )
+    or
+    // Modules for AES with specific key lengths
+    exists(string mod, string blockAlgo | mod = ["AES128", "AES192", "AES256"] |
+      call = cipherApi().getMember(mod).getAnInstantiation() and
+      // Canonical representation is `AES-<keyLength>`
+      blockAlgo = "AES-" + mod.suffix(3) and
+      exists(string blockMode |
+        if exists(getStringArgument(call, 0))
+        then
+          // `OpenSSL::Cipher::<blockAlgo>.new('<blockMode>')`
+          blockMode = getStringArgument(call, 0).toUpperCase()
+        else
+          // `OpenSSL::Cipher::<blockAlgo>.new` uses CBC by default
+          blockMode = "CBC"
+      |
+        cipherName = blockAlgo + "-" + blockMode and
+        cipherMode.isBlockMode(blockMode)
+      )
+    )
+  )
+}
+
+/**
+ * Holds if `call` is a call that instantiates an OpenSSL cipher using a module
+ * specific to a block encryption algorithm, e.g. Blowfish, DES, etc.
+ */
+private predicate cipherInstantiationSpecific(
+  DataFlow::CallNode call, OpenSSLCipher cipher, CipherMode cipherMode
+) {
+  exists(string cipherName | cipher.matchesName(cipherName) |
+    // Block ciphers with dedicated modules
+    exists(string blockAlgo | blockAlgo = ["BF", "CAST5", "DES", "IDEA", "RC2"] |
+      call = cipherApi().getMember(blockAlgo).getAnInstantiation() and
+      exists(string blockMode |
+        if exists(getStringArgument(call, 0))
+        then
+          // `OpenSSL::Cipher::<blockAlgo>.new('<blockMode>')`
+          blockMode = getStringArgument(call, 0).toUpperCase()
+        else
+          // `OpenSSL::Cipher::<blockAlgo>.new` uses CBC by default
+          blockMode = "CBC"
+      |
+        cipherName = blockAlgo + "-" + blockMode and
+        cipherMode.isBlockMode(blockMode)
+      )
+    )
+  )
+}
+
+/**
+ * Holds if `call` is a call to `OpenSSL::Cipher::RC4.new` or an RC4 `cipher`
+ * instance with mode `cipherMode`.
+ */
+private predicate cipherInstantiationRC4(
+  DataFlow::CallNode call, OpenSSLCipher cipher, CipherMode cipherMode
+) {
+  exists(string cipherName | cipher.matchesName(cipherName) |
+    // RC4 stream cipher
+    call = cipherApi().getMember("RC4").getAnInstantiation() and
+    cipherMode = TStreamCipher() and
+    (
+      if exists(getStringArgument(call, 0))
+      then cipherName = "RC4-" + getStringArgument(call, 0).toUpperCase()
+      else cipherName = "RC4"
+    )
+  )
+}
+
+/** A call to `OpenSSL::Cipher.new` or similar. */
+private class CipherInstantiation extends DataFlow::CallNode {
+  private OpenSSLCipher cipher;
+  private CipherMode cipherMode;
+
+  CipherInstantiation() {
+    cipherInstantiationGeneric(this, cipher, cipherMode) or
+    cipherInstantiationAES(this, cipher, cipherMode) or
+    cipherInstantiationSpecific(this, cipher, cipherMode) or
+    cipherInstantiationRC4(this, cipher, cipherMode)
+  }
+
+  /** Gets the `OpenSSLCipher` associated with this instance. */
+  OpenSSLCipher getCipher() { result = cipher }
+
+  /** Gets the mode used by this cipher, if applicable. */
+  CipherMode getCipherMode() { result = cipherMode }
+}
+
+private DataFlow::LocalSourceNode cipherInstance(
+  TypeTracker t, OpenSSLCipher cipher, CipherMode cipherMode
+) {
+  t.start() and
+  result.(CipherInstantiation).getCipher() = cipher and
+  result.(CipherInstantiation).getCipherMode() = cipherMode
+  or
+  exists(TypeTracker t2 | result = cipherInstance(t2, cipher, cipherMode).track(t2, t))
+}
+
+/** A node with flow from `OpenSSL::Cipher.new`. */
+private class CipherNode extends DataFlow::Node {
+  private OpenSSLCipher cipher;
+  private CipherMode cipherMode;
+
+  CipherNode() { cipherInstance(TypeTracker::end(), cipher, cipherMode).flowsTo(this) }
+
+  /** Gets the cipher associated with this node. */
+  OpenSSLCipher getCipher() { result = cipher }
+
+  /** Gets the cipher associated with this node. */
+  CipherMode getCipherMode() { result = cipherMode }
+}
+
+/** An operation using the OpenSSL library that uses a cipher. */
+private class CipherOperation extends Cryptography::CryptographicOperation::Range,
+  DataFlow::CallNode {
+  private CipherNode cipherNode;
+  private DataFlow::Node input;
+
+  CipherOperation() {
+    // cipher instantiation is counted as a cipher operation with no input
+    cipherNode = this and cipherNode instanceof CipherInstantiation
+    or
+    this.getReceiver() = cipherNode and
+    this.getMethodName() = "update" and
+    input = this.getArgument(0)
+  }
+
+  override Cryptography::EncryptionAlgorithm getAlgorithm() {
+    result = cipherNode.getCipher().getAlgorithm()
+  }
+
+  override DataFlow::Node getAnInput() { result = input }
+
+  override predicate isWeak() {
+    cipherNode.getCipher().isWeak() or
+    cipherNode.getCipherMode().isWeak()
+  }
 }

ruby/ql/src/change-notes/2022-03-14-rb-weak-cryptographic-algorithm.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query, `rb/weak-cryptographic-algorithm`. The query finds uses of cryptographic algorithms that are known to be weak, such as DES.

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

@@ -0,0 +1,41 @@
+<!DOCTYPE qhelp PUBLIC
+ "-//Semmle//qhelp//EN"
+ "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>
+      Using broken or 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, or flawed. 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, such as AES-128 or RSA-2048.
+    </p>
+  </recommendation>
+  <example>
+
+    <p>
+      The following code uses the <code>OpenSSL</code> library to encrypt some
+      secret data. When you create a cipher using <code>OpenSSL</code> you must
+      specify the encryption algorithm to use. The first example uses DES, which
+      is an older algorithm that is now considered weak. The second example uses
+      AES, which is a stronger modern algorithm.
+    </p>
+
+    <sample src="examples/broken_crypto.rb" />
+  </example>
+  <references>
+    <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>
+    <li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html#rule---use-strong-approved-authenticated-encryption">Rule - Use strong approved cryptographic algorithms</a>.
+    </li>
+  </references>
+</qhelp>

ruby/ql/src/queries/security/cwe-327/BrokenCryptoAlgorithm.ql

@@ -0,0 +1,20 @@
+/**
+ * @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 rb/weak-cryptographic-algorithm
+ * @tags security
+ *       external/cwe/cwe-327
+ */
+
+import ruby
+import codeql.ruby.Concepts
+
+from Cryptography::CryptographicOperation operation
+where operation.isWeak()
+select operation,
+  "The cryptographic algorithm " + operation.getAlgorithm().getName() +
+    " is broken or weak, and should not be used."

ruby/ql/src/queries/security/cwe-327/examples/broken_crypto.rb

@@ -0,0 +1,21 @@
+require 'openssl'
+
+class Encryptor
+  attr_accessor :secret_key
+
+  def encrypt_message_weak(message)
+    cipher = OpenSSL::Cipher.new('des') # BAD: weak encryption
+    cipher.encrypt
+    cipher.key = secret_key
+    cipher.update(message)
+    cipher.final
+  end
+
+  def encrypt_message_strong(message)
+    cipher = OpenSSL::Cipher::AES128.new # GOOD: strong encryption
+    cipher.encrypt
+    cipher.key = secret_key
+    cipher.update(message)
+    cipher.final
+  end
+end

ruby/ql/test/query-tests/security/cwe-327/BrokenCryptoAlgorithm.expected

@@ -0,0 +1,18 @@
+| broken_crypto.rb:4:8:4:34 | call to new | The cryptographic algorithm DES is broken or weak, and should not be used. |
+| broken_crypto.rb:8:1:8:18 | call to update | The cryptographic algorithm DES is broken or weak, and should not be used. |
+| broken_crypto.rb:12:8:12:43 | call to new | The cryptographic algorithm AES is broken or weak, and should not be used. |
+| broken_crypto.rb:16:1:16:18 | call to update | The cryptographic algorithm AES is broken or weak, and should not be used. |
+| broken_crypto.rb:28:1:28:35 | call to new | The cryptographic algorithm AES is broken or weak, and should not be used. |
+| broken_crypto.rb:37:1:37:33 | call to new | The cryptographic algorithm AES is broken or weak, and should not be used. |
+| broken_crypto.rb:42:1:42:33 | call to new | The cryptographic algorithm AES is broken or weak, and should not be used. |
+| broken_crypto.rb:47:1:47:33 | call to new | The cryptographic algorithm AES is broken or weak, and should not be used. |
+| broken_crypto.rb:52:1:52:29 | call to new | The cryptographic algorithm BF is broken or weak, and should not be used. |
+| broken_crypto.rb:57:1:57:32 | call to new | The cryptographic algorithm CAST5 is broken or weak, and should not be used. |
+| broken_crypto.rb:60:1:60:24 | call to new | The cryptographic algorithm DES is broken or weak, and should not be used. |
+| broken_crypto.rb:62:1:62:30 | call to new | The cryptographic algorithm DES is broken or weak, and should not be used. |
+| broken_crypto.rb:67:1:67:31 | call to new | The cryptographic algorithm IDEA is broken or weak, and should not be used. |
+| broken_crypto.rb:70:1:70:24 | call to new | The cryptographic algorithm RC2 is broken or weak, and should not be used. |
+| broken_crypto.rb:72:1:72:30 | call to new | The cryptographic algorithm RC2 is broken or weak, and should not be used. |
+| broken_crypto.rb:75:1:75:24 | call to new | The cryptographic algorithm RC4 is broken or weak, and should not be used. |
+| broken_crypto.rb:77:1:77:29 | call to new | The cryptographic algorithm RC4 is broken or weak, and should not be used. |
+| broken_crypto.rb:79:1:79:35 | call to new | The cryptographic algorithm RC4 is broken or weak, and should not be used. |

ruby/ql/test/query-tests/security/cwe-327/BrokenCryptoAlgorithm.qlref

@@ -0,0 +1 @@
+queries/security/cwe-327/BrokenCryptoAlgorithm.ql

ruby/ql/test/query-tests/security/cwe-327/broken_crypto.rb

@@ -0,0 +1,79 @@
+require 'openssl'
+
+# BAD: creating a cipher using a weak scheme
+weak = OpenSSL::Cipher.new('des3')
+weak.encrypt
+weak.random_key
+# BAD: encrypting data using a weak cipher
+weak.update('foo')
+weak.final
+
+# BAD: creating a cipher using a weak block mode
+weak = OpenSSL::Cipher::AES.new(128, 'ecb')
+weak.encrypt
+weak.random_key
+# BAD: encrypting data using a weak block mode
+weak.update('foo')
+weak.final
+
+# GOOD: creating a cipher using a strong scheme
+strong = OpenSSL::Cipher.new('blowfish')
+strong.encrypt
+strong.random_key
+# GOOD: encrypting data using a strong cipher
+strong.update('bar')
+strong.final
+
+# BAD: weak block mode
+OpenSSL::Cipher::AES.new(128, :ecb)
+# GOOD: strong encryption algorithm
+OpenSSL::Cipher::AES.new(128, 'cbc')
+# GOOD: strong encryption algorithm
+OpenSSL::Cipher::AES.new('128-cbc')
+
+# GOOD: strong encryption algorithm
+OpenSSL::Cipher::AES128.new
+# BAD: weak block mode
+OpenSSL::Cipher::AES128.new 'ecb'
+
+# GOOD: strong encryption algorithm
+OpenSSL::Cipher::AES192.new
+# BAD: weak block mode
+OpenSSL::Cipher::AES192.new 'ecb'
+
+# GOOD: strong encryption algorithm
+OpenSSL::Cipher::AES256.new
+# BAD: weak block mode
+OpenSSL::Cipher::AES256.new 'ecb'
+
+# GOOD: strong encryption algorithm
+OpenSSL::Cipher::BF.new
+# BAD: weak block mode
+OpenSSL::Cipher::BF.new 'ecb'
+
+# GOOD: strong encryption algorithm
+OpenSSL::Cipher::CAST5.new
+# BAD: weak block mode
+OpenSSL::Cipher::CAST5.new 'ecb'
+
+# BAD: weak encryption algorithm
+OpenSSL::Cipher::DES.new
+# BAD: weak encryption algorithm
+OpenSSL::Cipher::DES.new 'cbc'
+
+# GOOD: strong encryption algorithm
+OpenSSL::Cipher::IDEA.new
+# BAD: weak block mode
+OpenSSL::Cipher::IDEA.new 'ecb'
+
+# BAD: weak encryption algorithm
+OpenSSL::Cipher::RC2.new
+# BAD: weak encryption algorithm
+OpenSSL::Cipher::RC2.new 'ecb'
+
+# BAD: weak encryption algorithm
+OpenSSL::Cipher::RC4.new
+# BAD: weak encryption algorithm
+OpenSSL::Cipher::RC4.new '40'
+# BAD: weak encryption algorithm
+OpenSSL::Cipher::RC4.new 'hmac-md5'