Merge pull request #14554 from maikypedia/maikypedia/insecure-randomness

Ruby: Add Insecure Randomness Query

Author: hmac

Diff for: ruby/ql/lib/codeql/ruby/security/InsecureRandomnessCustomizations.qll

@@ -0,0 +1,74 @@
+/**
+ * Provides default sources, sinks, and sanitizers for reasoning about random values that
+ * are not cryptographically secure, as well as extension points for adding your own.
+ */
+
+private import codeql.ruby.CFG
+private import codeql.ruby.AST
+private import codeql.ruby.DataFlow
+private import codeql.ruby.security.SensitiveActions
+private import codeql.ruby.Concepts
+private import codeql.ruby.ApiGraphs
+import codeql.ruby.frameworks.core.Kernel
+
+/**
+ * Provides default sources, sinks, and sanitizers for reasoning about random values that
+ * are not cryptographically secure, as well as extension points for adding your own.
+ */
+module InsecureRandomness {
+  /**
+   * A data flow source for random values that are not cryptographically secure.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for random values that are not cryptographically secure.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for random values that are not cryptographically secure.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /**
+   * A simple random number generator that is not cryptographically secure.
+   */
+  class DefaultSource extends Source, DataFlow::CallNode {
+    DefaultSource() {
+      this.asExpr().getExpr() instanceof UnknownMethodCall and
+      (
+        this.getReceiver().asExpr().getExpr() instanceof SelfVariableAccess and
+        super.getMethodName() = "rand"
+      )
+      or
+      this.(Kernel::KernelMethodCall).getMethodName() = "rand"
+    }
+  }
+
+  /**
+   * A sensitive write, considered as a sink for random values that are not cryptographically
+   * secure.
+   */
+  class SensitiveWriteSink extends Sink instanceof SensitiveWrite { }
+
+  /**
+   * A cryptographic key, considered as a sink for random values that are not cryptographically
+   * secure.
+   */
+  class CryptoKeySink extends Sink {
+    CryptoKeySink() {
+      exists(Cryptography::CryptographicOperation operation | this = operation.getAnInput())
+    }
+  }
+
+  /**
+   * A index call, considered as a sink for random values that are not cryptographiocally
+   * secure
+   */
+  class CharacterIndexing extends Sink {
+    CharacterIndexing() {
+      exists(DataFlow::CallNode c | this = c.getAMethodCall("[]").getArgument(0))
+    }
+  }
+}

Diff for: ruby/ql/lib/codeql/ruby/security/InsecureRandomnessQuery.qll

@@ -0,0 +1,21 @@
+/**
+ * Provides default sources, sinks and sanitizers for detecting Insecure Randomness
+ * vulnerabilities, as well as extension points for adding your own.
+ */
+
+private import codeql.ruby.DataFlow
+private import codeql.ruby.TaintTracking
+import InsecureRandomnessCustomizations::InsecureRandomness
+
+private module InsecureRandomnessConfig 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 }
+}
+
+/**
+ * Taint-tracking for detecting Insecure Randomness vulnerabilities.
+ */
+module InsecureRandomnessFlow = TaintTracking::Global<InsecureRandomnessConfig>;

Diff for: ruby/ql/src/change-notes/2023-12-18-insecure-randomness-query.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new experimental query, `rb/insecure-randomness`, to detect when application uses random values that are not cryptographically secure.

Diff for: ruby/ql/src/experimental/insecure-randomness/InsecureRandomness.qhelp

@@ -0,0 +1,55 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Using a cryptographically weak pseudo-random number generator to generate a security-sensitive value, 
+such as a password, makes it easier for an attacker to predict the value.
+
+Pseudo-random number generators generate a sequence of numbers that only approximates the
+properties of random numbers. The sequence is not truly random because it is completely 
+determined by a relatively small set of initial values, the seed. If the random number generator is 
+cryptographically weak, then this sequence may be easily predictable through outside observations.
+</p>
+</overview>
+
+<recommendation>
+<p>
+When generating values for use in security-sensitive contexts, it's essential to utilize a 
+cryptographically secure pseudo-random number generator. As a general guideline, a value 
+should be deemed "security-sensitive" if its predictability would empower an attacker to 
+perform actions that would otherwise be beyond their reach. For instance, if an attacker could 
+predict a newly generated user's random password, they would gain unauthorized access to that user's 
+account.
+
+For Ruby, <code>SecureRandom</code> provides a cryptographically secure pseudo-random number generator. 
+<code>rand</code> is not cryptographically secure, and should be avoided in security contexts. 
+For contexts which are not security sensitive, <code>Random</code> may be preferable as it has a more convenient 
+interface.
+
+</p>
+</recommendation>
+
+<example>
+<p>
+The following examples show different ways of generating a password.
+</p>
+
+<p>The first example uses <code>Random.rand()</code> which is not for security purposes</p>
+
+<sample src="examples/InsecureRandomnessBad.rb" />
+
+<p>In the second example, the password is generated using <code>SecureRandom.random_bytes()</code> which is a 
+cryptographically secure method.</p>
+
+<sample src="examples/InsecureRandomnessGood.rb" />
+</example>
+
+<references>
+<li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Pseudorandom_number_generator">Pseudo-random number generator</a>.</li>
+<li>Common Weakness Enumeration: <a href="https://cwe.mitre.org/data/definitions/338.html">CWE-338</a>.</li>
+<li>Ruby-doc: <a href="https://ruby-doc.org/core-3.1.2/Random.html">Random</a>.</li>
+</references>
+</qhelp>

Diff for: ruby/ql/src/experimental/insecure-randomness/InsecureRandomness.ql

@@ -0,0 +1,23 @@
+/**
+ * @name Insecure randomness
+ * @description Using a cryptographically weak pseudo-random number generator to generate a
+ *              security-sensitive value may allow an attacker to predict what value will
+ *              be generated.
+ * @kind path-problem
+ * @problem.severity warning
+ * @security-severity 7.8
+ * @precision high
+ * @id rb/insecure-randomness
+ * @tags security
+ *       external/cwe/cwe-338
+ */
+
+import codeql.ruby.DataFlow
+import codeql.ruby.security.InsecureRandomnessQuery
+import InsecureRandomnessFlow::PathGraph
+
+from InsecureRandomnessFlow::PathNode source, InsecureRandomnessFlow::PathNode sink
+where InsecureRandomnessFlow::flowPath(source, sink)
+select sink.getNode(), source, sink,
+  "This uses a cryptographically insecure random number generated at $@ in a security context.",
+  source.getNode(), source.getNode().toString()

Diff for: ruby/ql/src/experimental/insecure-randomness/examples/InsecureRandomnessBad.rb

@@ -0,0 +1,7 @@
+def generate_password()
+  chars = ('a'..'z').to_a + ('A'..'Z').to_a + ('0'..'9').to_a + ['!', '@', '#', '$', '%']
+  # BAD: rand is not cryptographically secure
+  password = (1..10).collect { chars[rand(chars.size)] }.join
+end
+
+password = generate_password

Diff for: ruby/ql/src/experimental/insecure-randomness/examples/InsecureRandomnessGood.rb

@@ -0,0 +1,12 @@
+require 'securerandom'
+
+def generate_password()
+  chars = ('a'..'z').to_a + ('A'..'Z').to_a + ('0'..'9').to_a + ['!', '@', '#', '$', '%']
+
+  # GOOD: SecureRandom is cryptographically secure
+  password = SecureRandom.random_bytes(10).each_byte.map do |byte|
+    chars[byte % chars.length]
+  end.join
+end
+
+password = generate_password()

Diff for: ruby/ql/test/query-tests/experimental/InsecureRandomness/InsecureRandomness.expected

@@ -0,0 +1,6 @@
+edges
+nodes
+| InsecureRandomness.rb:6:42:6:57 | call to rand | semmle.label | call to rand |
+subpaths
+#select
+| InsecureRandomness.rb:6:42:6:57 | call to rand | InsecureRandomness.rb:6:42:6:57 | call to rand | InsecureRandomness.rb:6:42:6:57 | call to rand | This uses a cryptographically insecure random number generated at $@ in a security context. | InsecureRandomness.rb:6:42:6:57 | call to rand | call to rand |

Diff for: ruby/ql/test/query-tests/experimental/InsecureRandomness/InsecureRandomness.qlref

@@ -0,0 +1 @@
+experimental/insecure-randomness/InsecureRandomness.ql

Diff for: ruby/ql/test/query-tests/experimental/InsecureRandomness/InsecureRandomness.rb

@@ -0,0 +1,19 @@
+require 'securerandom'
+
+def generate_password_1(length)
+  chars = ('a'..'z').to_a + ('A'..'Z').to_a + ('0'..'9').to_a + ['!', '@', '#', '$', '%']
+  # BAD: rand is not cryptographically secure
+  password = (1..length).collect { chars[rand(chars.size)] }.join
+end
+
+def generate_password_2(length)
+  chars = ('a'..'z').to_a + ('A'..'Z').to_a + ('0'..'9').to_a + ['!', '@', '#', '$', '%']
+
+  # GOOD: SecureRandom is cryptographically secure
+  password = SecureRandom.random_bytes(length).each_byte.map do |byte|
+    chars[byte % chars.length]
+  end.join
+end
+
+password = generate_password_1(10)
+password = generate_password_2(10)