Merge pull request #8598 from hmac/hmac/insecure-dep-resolution

Ruby: Add rb/insecure-dependency query

Author: hmac

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

@@ -0,0 +1,91 @@
+/**
+ * Provides predicates for reasoning about insecure dependency configurations.
+ */
+
+private import ruby
+
+/**
+ * A method call in a Gemfile.
+ */
+private class GemfileMethodCall extends MethodCall {
+  GemfileMethodCall() { this.getLocation().getFile().getBaseName() = "Gemfile" }
+}
+
+/**
+ * Method calls that configure gem dependencies and can specify (possibly insecure) URLs.
+ */
+abstract private class RelevantGemCall extends GemfileMethodCall {
+  abstract Expr getAUrlPart();
+}
+
+/**
+ * A call to `source`.
+ */
+private class SourceCall extends RelevantGemCall {
+  SourceCall() { this.getMethodName() = "source" }
+
+  override Expr getAUrlPart() { result = this.getAnArgument() }
+}
+
+/**
+ * A call to `git_source`.
+ */
+private class GitSourceCall extends RelevantGemCall {
+  GitSourceCall() { this.getMethodName() = "git_source" }
+
+  override Expr getAUrlPart() { result = this.getBlock().getLastStmt() }
+}
+
+/**
+ * A call to `gem`.
+ */
+private class GemCall extends RelevantGemCall {
+  GemCall() { this.getMethodName() = "gem" }
+
+  override Expr getAUrlPart() { result = this.getKeywordArgument(["source", "git"]) }
+}
+
+/**
+ * Holds if `s` is a URL with an insecure protocol. `proto` is the protocol.
+ */
+bindingset[s]
+private predicate hasInsecureProtocol(string s, string proto) {
+  proto = s.regexpCapture("^(http|ftp):.+", 1).toUpperCase()
+}
+
+/**
+ * Holds if `e` is a string containing a URL that uses the insecure protocol `proto`.
+ */
+private predicate containsInsecureUrl(Expr e, string proto) {
+  // Handle cases where the string as a whole has no constant value (due to interpolations)
+  // but has a known prefix. E.g. "http://#{foo}"
+  exists(StringComponent c | c = e.(StringlikeLiteral).getComponent(0) |
+    hasInsecureProtocol(c.getConstantValue().getString(), proto)
+  )
+  or
+  hasInsecureProtocol(e.getConstantValue().getString(), proto)
+}
+
+/**
+ * Returns the suggested protocol to use in place of the insecure protocol `proto`.
+ */
+bindingset[proto]
+private string suggestedProtocol(string proto) {
+  proto = "HTTP" and result = "HTTPS"
+  or
+  proto = "FTP" and result = "FTPS or SFTP"
+}
+
+/**
+ * Holds if `url` is a string containing a URL that uses an insecure protocol.
+ * `msg` is the alert message that will be displayed to the user.
+ */
+predicate insecureDependencyUrl(Expr url, string msg) {
+  exists(RelevantGemCall call, string proto |
+    url = call.getAUrlPart() and
+    containsInsecureUrl(url, proto) and
+    msg =
+      "Dependency source URL uses the unencrypted protocol " + proto + ". Use " +
+        suggestedProtocol(proto) + " instead."
+  )
+}

ruby/ql/src/change-notes/2022-03-30-insecure-dependency.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query, `rb/insecure-dependency`. The query finds cases where Ruby gems may be downloaded over an insecure communication channel.

ruby/ql/src/queries/security/cwe-300/InsecureDependencyResolution.qhelp

@@ -0,0 +1,54 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Using an insecure protocol like HTTP or FTP to download dependencies makes the build process vulnerable to a 
+man-in-the-middle (MITM) attack.
+</p>
+<p>
+This can allow attackers to inject malicious code into the downloaded dependencies, and thereby 
+infect the build artifacts and execute arbitrary code on the machine building the artifacts.
+</p>
+
+</overview>
+<recommendation>
+
+<p>Always use a secure protocol, such as HTTPS or SFTP, when downloading artifacts from a URL.</p>
+
+</recommendation>
+
+<example>
+<p>
+The below example shows a <code>Gemfile</code> that specifies a gem source using the insecure HTTP protocol.
+</p>
+<sample src="examples/bad_gemfile.rb" />
+<p>
+The fix is to change the protocol to HTTPS.
+</p>
+<sample src="examples/good_gemfile.rb" />
+</example>
+
+<references>
+<li>
+  Jonathan Leitschuh: 
+  <a href="https://infosecwriteups.com/want-to-take-over-the-java-ecosystem-all-you-need-is-a-mitm-1fc329d898fb">
+    Want to take over the Java ecosystem? All you need is a MITM!
+  </a>
+</li>
+<li>
+  Max Veytsman: 
+  <a href="https://max.computer/blog/how-to-take-over-the-computer-of-any-java-or-clojure-or-scala-developer/">
+    How to take over the computer of any Java (or Clojure or Scala) Developer.
+  </a>
+</li>
+<li>
+  Wikipedia: <a href="https://en.wikipedia.org/wiki/Supply_chain_attack">Supply chain attack.</a>
+</li>
+<li>
+  Wikipedia: <a href="https://en.wikipedia.org/wiki/Man-in-the-middle_attack">Man-in-the-middle attack.</a>
+</li>
+</references>
+</qhelp>

ruby/ql/src/queries/security/cwe-300/InsecureDependencyResolution.ql

@@ -0,0 +1,22 @@
+/**
+ * @name Dependency download using unencrypted communication channel
+ * @description Using unencrypted protocols to fetch dependencies can leave an application
+ *              open to man-in-the-middle attacks.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 8.1
+ * @precision high
+ * @id rb/insecure-dependency
+ * @tags security
+ *       external/cwe/cwe-300
+ *       external/cwe/cwe-319
+ *       external/cwe/cwe-494
+ *       external/cwe/cwe-829
+ */
+
+import ruby
+import codeql.ruby.security.InsecureDependencyQuery
+
+from Expr url, string msg
+where insecureDependencyUrl(url, msg)
+select url, msg

ruby/ql/src/queries/security/cwe-300/examples/bad_gemfile.rb

@@ -0,0 +1,3 @@
+source "http://rubygems.org"
+
+gem "my-gem-a", "1.2.3"

ruby/ql/src/queries/security/cwe-300/examples/good_gemfile.rb

@@ -0,0 +1,3 @@
+source "https://rubygems.org"
+
+gem "my-gem-a", "1.2.3"

ruby/ql/test/query-tests/security/cwe-300/Gemfile

@@ -0,0 +1,56 @@
+source "https://rubygems.org" # GOOD
+source "http://rubygems.org" # $result=BAD
+source "ftp://rubygems.org" # $result=BAD
+source "ftps://rubygems.org" # GOOD
+source "unknown://rubygems.org" # GOOD
+
+git_source(:a) { "https://github.com"   } # GOOD
+git_source(:b) { "http://github.com"    } # $result=BAD
+git_source(:c) { "ftp://github.com"     } # $result=BAD
+git_source(:d) { "ftps://github.com"    } # GOOD
+git_source(:e) { "unknown://github.com" } # GOOD
+
+git_source(:f) { |name| "https://github.com/#{name}" } # GOOD
+git_source(:g) { |name| "http://github.com/#{name}" } # $result=BAD
+git_source(:h) { |name| "ftp://github.com/#{name}" } # $result=BAD
+git_source(:i) { |name| "ftps://github.com/#{name}" } # GOOD
+git_source(:j) { |name| "unknown://github.com/#{name}" } # GOOD
+
+git_source(:k) do |name|
+    foo
+    "https://github.com/#{name}" } # GOOD
+end
+git_source(:l) do |name|
+    foo
+    "http://github.com/#{name}" } # $result=BAD
+end
+git_source(:m) do |name|
+    foo
+    "ftp://github.com/#{name}" } # $result=BAD
+end
+git_source(:n) do |name|
+    foo
+    "ftps://github.com/#{name}" } # GOOD
+end
+git_source(:o) do |name|
+    foo
+    "unknown://github.com/#{name}" } # GOOD
+end
+
+gem "jwt", "1.2.3", git: "https://github.com/jwt/ruby-jwt" # GOOD
+gem "jwt", "1.2.3", git: "http://github.com/jwt/ruby-jwt" # $result=BAD
+gem "jwt", "1.2.3", git: "ftp://github.com/jwt/ruby-jwt" # $result=BAD
+gem "jwt", "1.2.3", git: "ftps://github.com/jwt/ruby-jwt" # GOOD
+gem "jwt", "1.2.3", git: "unknown://github.com/jwt/ruby-jwt" # GOOD
+
+gem "jwt", "1.2.3", :git => "https://github.com/jwt/ruby-jwt" # GOOD
+gem "jwt", "1.2.3", :git => "http://github.com/jwt/ruby-jwt" # $result=BAD
+gem "jwt", "1.2.3", :git => "ftp://github.com/jwt/ruby-jwt" # $result=BAD
+gem "jwt", "1.2.3", :git => "ftps://github.com/jwt/ruby-jwt" # GOOD
+gem "jwt", "1.2.3", :git => "unknown://github.com/jwt/ruby-jwt" # GOOD
+
+gem "jwt", "1.2.3", source: "https://rubygems.org" # GOOD
+gem "jwt", "1.2.3", source: "http://rubygems.org" # $result=BAD
+gem "jwt", "1.2.3", source: "ftp://rubygems.org" # $result=BAD
+gem "jwt", "1.2.3", source: "ftps://rubygems.org" # GOOD
+gem "jwt", "1.2.3", source: "unknown://rubygems.org" # GOOD

ruby/ql/test/query-tests/security/cwe-300/InsecureDependency.expected

@@ -0,0 +1,16 @@
+failures
+#select
+| Gemfile:2:8:2:28 | "http://rubygems.org" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. |
+| Gemfile:3:8:3:27 | "ftp://rubygems.org" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. |
+| Gemfile:8:18:8:36 | "http://github.com" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. |
+| Gemfile:9:18:9:35 | "ftp://github.com" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. |
+| Gemfile:14:25:14:51 | "http://github.com/#{...}" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. |
+| Gemfile:15:25:15:50 | "ftp://github.com/#{...}" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. |
+| Gemfile:25:5:25:31 | "http://github.com/#{...}" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. |
+| Gemfile:29:5:29:30 | "ftp://github.com/#{...}" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. |
+| Gemfile:41:26:41:57 | "http://github.com/jwt/ruby-jwt" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. |
+| Gemfile:42:26:42:56 | "ftp://github.com/jwt/ruby-jwt" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. |
+| Gemfile:47:29:47:60 | "http://github.com/jwt/ruby-jwt" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. |
+| Gemfile:48:29:48:59 | "ftp://github.com/jwt/ruby-jwt" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. |
+| Gemfile:53:29:53:49 | "http://rubygems.org" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. |
+| Gemfile:54:29:54:48 | "ftp://rubygems.org" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. |

ruby/ql/test/query-tests/security/cwe-300/InsecureDependency.ql

@@ -0,0 +1,23 @@
+import ruby
+import TestUtilities.InlineExpectationsTest
+import codeql.ruby.security.InsecureDependencyQuery
+
+class InsecureDependencyTest extends InlineExpectationsTest {
+  InsecureDependencyTest() { this = "InsecureDependencyTest" }
+
+  override string getARelevantTag() { result = "BAD" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "result" and
+    value = "BAD" and
+    exists(Expr e |
+      insecureDependencyUrl(e, _) and
+      location = e.getLocation() and
+      element = e.toString()
+    )
+  }
+}
+
+from Expr url, string msg
+where insecureDependencyUrl(url, msg)
+select url, msg

ruby/ql/test/query-tests/security/cwe-300/foo.rb

@@ -0,0 +1,5 @@
+# Calls to `gem` etc. outside of the Gemfile should be ignored, since they may not be configuring dependencies.
+
+gem "foo", git: "http://foo.com"
+git_source :a { |x| "http://foo.com" }
+source "http://foo.com"