Merge pull request github#10832 from erik-krogh/passRb

RB: add model for the `Digest` and `OpenSSL::Digest` modules

Author: erik-krogh

ruby/ql/lib/change-notes/2022-10-14-digest-model.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The hashing algorithms from `Digest` and `OpenSSL::Digest` are now recognized and can be flagged by the `rb/weak-cryptographic-algorithm` query.

ruby/ql/lib/codeql/ruby/ApiGraphs.qll

@@ -203,7 +203,7 @@ module API {
     /**
      * Gets a node representing a call to `method` on the receiver represented by this node.
      */
-    Node getMethod(string method) {
+    MethodAccessNode getMethod(string method) {
       result = this.getASubclass().getASuccessor(Label::method(method))
     }
 

ruby/ql/lib/codeql/ruby/frameworks/Core.qll

@@ -14,6 +14,7 @@ import core.Hash
 import core.String
 import core.Regexp
 import core.IO
+import core.Digest
 
 /**
  * A system command executed via subshell literal syntax.

ruby/ql/lib/codeql/ruby/frameworks/core/Digest.qll

@@ -0,0 +1,34 @@
+/**
+ * Provides modeling for the `Digest` module.
+ */
+
+private import codeql.ruby.ApiGraphs
+private import codeql.ruby.Concepts
+private import codeql.ruby.DataFlow
+
+/** Gets an API node for a Digest class that hashes using `algo`. */
+private API::Node digest(Cryptography::HashingAlgorithm algo) {
+  exists(string name | result = API::getTopLevelMember("Digest").getMember(name) |
+    name = ["MD5", "SHA1", "SHA2", "RMD160"] and
+    algo.matchesName(name)
+  )
+}
+
+/** A call that hashes some input using a hashing algorithm from the `Digest` module. */
+private class DigestCall extends Cryptography::CryptographicOperation::Range instanceof DataFlow::CallNode {
+  Cryptography::HashingAlgorithm algo;
+
+  DigestCall() {
+    this = digest(algo).getAMethodCall(["hexdigest", "base64digest", "bubblebabble"])
+    or
+    this = digest(algo).getAMethodCall("file") // it's directly hashing the contents of a file, but that's close enough for us.
+    or
+    this = digest(algo).getInstance().getAMethodCall(["digest", "update", "<<"])
+  }
+
+  override Cryptography::HashingAlgorithm getAlgorithm() { result = algo }
+
+  override DataFlow::Node getAnInput() { result = super.getArgument(0) }
+
+  override Cryptography::BlockMode getBlockMode() { none() }
+}

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

@@ -581,3 +581,49 @@ private class CipherOperation extends Cryptography::CryptographicOperation::Rang
     result = cipherNode.getCipherMode().getBlockMode()
   }
 }
+
+/** Predicates and classes modeling the `OpenSSL::Digest` module */
+private module Digest {
+  private import codeql.ruby.ApiGraphs
+
+  /** A call that hashes some input using a hashing algorithm from the `OpenSSL::Digest` module. */
+  private class DigestCall extends Cryptography::CryptographicOperation::Range instanceof DataFlow::CallNode {
+    Cryptography::HashingAlgorithm algo;
+
+    DigestCall() {
+      exists(API::MethodAccessNode call |
+        call = API::getTopLevelMember("OpenSSL").getMember("Digest").getMethod("new")
+      |
+        this = call.getReturn().getAMethodCall(["digest", "update", "<<"]) and
+        algo.matchesName(call.getCallNode()
+              .getArgument(0)
+              .asExpr()
+              .getExpr()
+              .getConstantValue()
+              .getString())
+      )
+    }
+
+    override Cryptography::HashingAlgorithm getAlgorithm() { result = algo }
+
+    override DataFlow::Node getAnInput() { result = super.getArgument(0) }
+
+    override Cryptography::BlockMode getBlockMode() { none() }
+  }
+
+  /** A call to `OpenSSL::Digest.digest` that hashes input directly without constructing a digest instance. */
+  private class DigestCallDirect extends Cryptography::CryptographicOperation::Range instanceof DataFlow::CallNode {
+    Cryptography::HashingAlgorithm algo;
+
+    DigestCallDirect() {
+      this = API::getTopLevelMember("OpenSSL").getMember("Digest").getMethod("digest").getCallNode() and
+      algo.matchesName(this.getArgument(0).asExpr().getExpr().getConstantValue().getString())
+    }
+
+    override Cryptography::HashingAlgorithm getAlgorithm() { result = algo }
+
+    override DataFlow::Node getAnInput() { result = super.getArgument(1) }
+
+    override Cryptography::BlockMode getBlockMode() { none() }
+  }
+}

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

@@ -17,3 +17,13 @@
 | 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. |
+| broken_crypto.rb:81:1:81:28 | call to hexdigest | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
+| broken_crypto.rb:84:1:84:31 | call to base64digest | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
+| broken_crypto.rb:87:1:87:20 | call to digest | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
+| broken_crypto.rb:89:1:89:21 | call to update | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
+| broken_crypto.rb:90:1:90:17 | ... << ... | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
+| broken_crypto.rb:95:1:95:34 | call to bubblebabble | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
+| broken_crypto.rb:97:11:97:37 | call to file | The cryptographic algorithm MD5 is broken or weak, and should not be used. |
+| broken_crypto.rb:103:1:103:21 | call to digest | The cryptographic algorithm SHA1 is broken or weak, and should not be used. |
+| broken_crypto.rb:104:1:104:17 | ... << ... | The cryptographic algorithm SHA1 is broken or weak, and should not be used. |
+| broken_crypto.rb:106:1:106:37 | call to digest | The cryptographic algorithm SHA1 is broken or weak, and should not be used. |

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

@@ -77,3 +77,31 @@
 OpenSSL::Cipher::RC4.new '40'
 # BAD: weak encryption algorithm
 OpenSSL::Cipher::RC4.new 'hmac-md5'
+
+Digest::MD5.hexdigest('foo') # BAD: weak hash algorithm
+Digest::SHA256.hexdigest('foo') # GOOD: strong hash algorithm
+
+Digest::MD5.base64digest('foo') # BAD: weak hash algorithm
+
+md5 = Digest::MD5.new
+md5.digest 'message' # BAD: weak hash algorithm
+
+md5.update 'message1' # BAD: weak hash algorithm
+md5 << 'message2' # << is an alias for update
+
+sha256 = Digest::SHA256.new
+sha256.digest 'message' # GOOD: strong hash algorithm
+
+Digest::MD5.bubblebabble 'message' # BAD: weak hash algorithm
+
+filemd5 = Digest::MD5.file 'testfile'
+filemd5.hexdigest
+
+Digest("MD5").hexdigest('foo') # BAD: weak hash algorithm
+
+sha1 = OpenSSL::Digest.new('SHA1')
+sha1.digest 'message' # BAD: weak hash algorithm
+sha1 << 'message' # << is an alias for update
+
+OpenSSL::Digest.digest('SHA1', "abc") # BAD: weak hash algorithm
+OpenSSL::Digest.digest('SHA3-512', "abc") # GOOD: strong hash algorithm