diff --git a/ruby/ql/lib/codeql/ruby/security/InsecureDependencyQuery.qll b/ruby/ql/lib/codeql/ruby/security/InsecureDependencyQuery.qll new file mode 100644 index 00000000000..e49a2f04913 --- /dev/null +++ b/ruby/ql/lib/codeql/ruby/security/InsecureDependencyQuery.qll @@ -0,0 +1,87 @@ +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(_) | + 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." + ) +} diff --git a/ruby/ql/src/queries/security/cwe-300/InsecureDependencyResolution.qhelp b/ruby/ql/src/queries/security/cwe-300/InsecureDependencyResolution.qhelp new file mode 100644 index 00000000000..97b332f8335 --- /dev/null +++ b/ruby/ql/src/queries/security/cwe-300/InsecureDependencyResolution.qhelp @@ -0,0 +1,54 @@ + + + + +

+Using an insecure protocol like HTTP or FTP to download dependencies makes the build process vulnerable to a +man-in-the-middle (MITM) attack. +

+

+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. +

+ +
+ + +

Always use a secure protocol, such as HTTPS or SFTP, when downloading artifacts from an URL.

+ +
+ + +

+The below example shows a Gemfile that specifies a gem source using the insecure HTTP protocol. +

+ +

+The fix is to change the protocol to HTTPS. +

+ +
+ + +
  • + Jonathan Leitschuh: + + Want to take over the Java ecosystem? All you need is a MITM! + +
  • +
  • + Max Veytsman: + + How to take over the computer of any Java (or Clojure or Scala) Developer. + +
  • +
  • + Wikipedia: Supply chain attack. +
  • +
  • + Wikipedia: Man-in-the-middle attack. +
  • +
    +
    diff --git a/ruby/ql/src/queries/security/cwe-300/InsecureDependencyResolution.ql b/ruby/ql/src/queries/security/cwe-300/InsecureDependencyResolution.ql new file mode 100644 index 00000000000..0687717951c --- /dev/null +++ b/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 diff --git a/ruby/ql/src/queries/security/cwe-300/examples/bad_gemfile.rb b/ruby/ql/src/queries/security/cwe-300/examples/bad_gemfile.rb new file mode 100644 index 00000000000..8ab9fc0075d --- /dev/null +++ b/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" \ No newline at end of file diff --git a/ruby/ql/src/queries/security/cwe-300/examples/good_gemfile.rb b/ruby/ql/src/queries/security/cwe-300/examples/good_gemfile.rb new file mode 100644 index 00000000000..ae6de349d86 --- /dev/null +++ b/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" \ No newline at end of file diff --git a/ruby/ql/test/query-tests/security/cwe-300/Gemfile b/ruby/ql/test/query-tests/security/cwe-300/Gemfile new file mode 100644 index 00000000000..32844624643 --- /dev/null +++ b/ruby/ql/test/query-tests/security/cwe-300/Gemfile @@ -0,0 +1,50 @@ +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", 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 \ No newline at end of file diff --git a/ruby/ql/test/query-tests/security/cwe-300/InsecureDependencyResolution.expected b/ruby/ql/test/query-tests/security/cwe-300/InsecureDependencyResolution.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ruby/ql/test/query-tests/security/cwe-300/InsecureDependencyResolution.ql b/ruby/ql/test/query-tests/security/cwe-300/InsecureDependencyResolution.ql new file mode 100644 index 00000000000..6a0095da796 --- /dev/null +++ b/ruby/ql/test/query-tests/security/cwe-300/InsecureDependencyResolution.ql @@ -0,0 +1,19 @@ +import ruby +import TestUtilities.InlineExpectationsTest +import codeql.ruby.security.InsecureDependencyQuery + +class InsecureDependencyResolutionTest extends InlineExpectationsTest { + InsecureDependencyResolutionTest() { this = "InsecureDependencyResolutionTest" } + + 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() + ) + } +} diff --git a/ruby/ql/test/query-tests/security/cwe-300/foo.rb b/ruby/ql/test/query-tests/security/cwe-300/foo.rb new file mode 100644 index 00000000000..286a7bdc377 --- /dev/null +++ b/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"