Merge pull request #7062 from github/ruby/rails-csrf

Ruby: Add `rb/csrf-protection-disabled` query

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

@@ -596,6 +596,37 @@ module OrmInstantiation {
   }
 }
 
+/**
+ * A data-flow node that may set or unset Cross-site request forgery protection.
+ *
+ * Extend this class to refine existing API models. If you want to model new APIs,
+ * extend `CSRFProtectionSetting::Range` instead.
+ */
+class CSRFProtectionSetting extends DataFlow::Node instanceof CSRFProtectionSetting::Range {
+  /**
+   * Gets the boolean value corresponding to if CSRF protection is enabled
+   * (`true`) or disabled (`false`) by this node.
+   */
+  boolean getVerificationSetting() { result = super.getVerificationSetting() }
+}
+
+/** Provides a class for modeling new CSRF protection setting APIs. */
+module CSRFProtectionSetting {
+  /**
+   * A data-flow node that may set or unset Cross-site request forgery protection.
+   *
+   * Extend this class to model new APIs. If you want to refine existing API models,
+   * extend `CSRFProtectionSetting` instead.
+   */
+  abstract class Range extends DataFlow::Node {
+    /**
+     * Gets the boolean value corresponding to if CSRF protection is enabled
+     * (`true`) or disabled (`false`) by this node.
+     */
+    abstract boolean getVerificationSetting();
+  }
+}
+
 /** Provides classes for modeling path-related APIs. */
 module Path {
   /**

ruby/ql/lib/codeql/ruby/Frameworks.qll

@@ -6,6 +6,7 @@ private import codeql.ruby.frameworks.ActionController
 private import codeql.ruby.frameworks.ActiveRecord
 private import codeql.ruby.frameworks.ActiveStorage
 private import codeql.ruby.frameworks.ActionView
+private import codeql.ruby.frameworks.Rails
 private import codeql.ruby.frameworks.StandardLibrary
 private import codeql.ruby.frameworks.Files
 private import codeql.ruby.frameworks.HttpClients

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

@@ -257,3 +257,21 @@ predicate controllerTemplateFile(ActionControllerControllerClass cls, ErbFile te
     )
   )
 }
+
+/**
+ * A call to either `skip_forgery_protection` or
+ * `skip_before_action :verify_authenticity_token` to disable CSRF authenticity
+ * token protection.
+ */
+class ActionControllerSkipForgeryProtectionCall extends CSRFProtectionSetting::Range {
+  ActionControllerSkipForgeryProtectionCall() {
+    exists(MethodCall call | call = this.asExpr().getExpr() |
+      call.getMethodName() = "skip_forgery_protection"
+      or
+      call.getMethodName() = "skip_before_action" and
+      call.getAnArgument().(SymbolLiteral).getValueText() = "verify_authenticity_token"
+    )
+  }
+
+  override boolean getVerificationSetting() { result = false }
+}

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

@@ -0,0 +1,131 @@
+/**
+ * Provides classes for working with Rails.
+ */
+
+private import codeql.files.FileSystem
+private import codeql.ruby.AST
+private import codeql.ruby.Concepts
+private import codeql.ruby.DataFlow
+private import codeql.ruby.frameworks.ActionController
+private import codeql.ruby.frameworks.ActionView
+private import codeql.ruby.frameworks.ActiveRecord
+private import codeql.ruby.frameworks.ActiveStorage
+private import codeql.ruby.ast.internal.Module
+private import codeql.ruby.ApiGraphs
+
+/**
+ * A reference to either `Rails::Railtie`, `Rails::Engine`, or `Rails::Application`.
+ * `Engine` and `Application` extend `Railtie`, but may not have definitions present in the database.
+ */
+private class RailtieClassAccess extends ConstantReadAccess {
+  RailtieClassAccess() {
+    this.getScopeExpr().(ConstantAccess).getName() = "Rails" and
+    this.getName() = ["Railtie", "Engine", "Application"]
+  }
+}
+
+// A `ClassDeclaration` that (transitively) extends `Rails::Railtie`
+private class RailtieClass extends ClassDeclaration {
+  RailtieClass() {
+    this.getSuperclassExpr() instanceof RailtieClassAccess or
+    exists(RailtieClass other | other.getModule() = resolveScopeExpr(this.getSuperclassExpr()))
+  }
+}
+
+private DataFlow::CallNode getAConfigureCallNode() {
+  // `Rails.application.configure`
+  result = API::getTopLevelMember("Rails").getReturn("application").getAMethodCall("configure")
+  or
+  // `Rails::Application.configure`
+  exists(ConstantReadAccess read, RailtieClass cls |
+    read = result.getReceiver().asExpr().getExpr() and
+    resolveScopeExpr(read) = cls.getModule() and
+    result.asExpr().getExpr().(MethodCall).getMethodName() = "configure"
+  )
+}
+
+/**
+ * An access to a Rails config object.
+ */
+private class ConfigSourceNode extends DataFlow::LocalSourceNode {
+  ConfigSourceNode() {
+    // `Foo < Rails::Application ... config ...`
+    exists(MethodCall configCall | this.asExpr().getExpr() = configCall |
+      configCall.getMethodName() = "config" and
+      configCall.getEnclosingModule() instanceof RailtieClass
+    )
+    or
+    // `Rails.application.config`
+    this =
+      API::getTopLevelMember("Rails")
+          .getReturn("application")
+          .getReturn("config")
+          .getAnImmediateUse()
+    or
+    // `Rails.application.configure { ... config ... }`
+    // `Rails::Application.configure { ... config ... }`
+    exists(DataFlow::CallNode configureCallNode, Block block, MethodCall configCall |
+      configCall = this.asExpr().getExpr()
+    |
+      configureCallNode = getAConfigureCallNode() and
+      block = configureCallNode.asExpr().getExpr().(MethodCall).getBlock() and
+      configCall.getParent+() = block and
+      configCall.getMethodName() = "config"
+    )
+  }
+}
+
+private class ConfigNode extends DataFlow::Node {
+  ConfigNode() { exists(ConfigSourceNode src | src.flowsTo(this)) }
+}
+
+// A call where the Rails application config is the receiver
+private class CallAgainstConfig extends DataFlow::CallNode {
+  CallAgainstConfig() { this.getReceiver() instanceof ConfigNode }
+
+  MethodCall getCall() { result = this.asExpr().getExpr() }
+
+  Block getBlock() { result = this.getCall().getBlock() }
+}
+
+private class ActionControllerConfigNode extends DataFlow::Node {
+  ActionControllerConfigNode() {
+    exists(CallAgainstConfig source | source.getCall().getMethodName() = "action_controller" |
+      source.flowsTo(this)
+    )
+  }
+}
+
+/** Holds if `node` can contain `value`. */
+private predicate hasBooleanValue(DataFlow::Node node, boolean value) {
+  exists(DataFlow::LocalSourceNode literal |
+    literal.asExpr().getExpr().(BooleanLiteral).getValue() = value and
+    literal.flowsTo(node)
+  )
+}
+
+// `<actionControllerConfig>.allow_forgery_protection = <verificationSetting>`
+private DataFlow::CallNode getAnAllowForgeryProtectionCall(boolean verificationSetting) {
+  // exclude some test configuration
+  not (
+    result.getLocation().getFile().getRelativePath().matches("%test/%") or
+    result.getLocation().getFile().getStem() = "test"
+  ) and
+  result.getReceiver() instanceof ActionControllerConfigNode and
+  result.asExpr().getExpr().(MethodCall).getMethodName() = "allow_forgery_protection=" and
+  hasBooleanValue(result.getArgument(0), verificationSetting)
+}
+
+/**
+ * A `DataFlow::Node` that may enable or disable Rails CSRF protection in
+ * production code.
+ */
+private class AllowForgeryProtectionSetting extends CSRFProtectionSetting::Range {
+  private boolean verificationSetting;
+
+  AllowForgeryProtectionSetting() { this = getAnAllowForgeryProtectionCall(verificationSetting) }
+
+  override boolean getVerificationSetting() { result = verificationSetting }
+}
+// TODO: initialization hooks, e.g. before_configuration, after_initialize...
+// TODO: initializers

ruby/ql/src/queries/security/cwe-352/CSRFProtectionDisabled.qhelp

@@ -0,0 +1,61 @@
+<!DOCTYPE qhelp PUBLIC
+ "-//Semmle//qhelp//EN"
+ "qhelp.dtd">
+<qhelp>
+
+  <overview>
+    <p>
+      Cross-site request forgery (CSRF) is a type of vulnerability in which an
+      attacker is able to force a user carry out an action that the user did
+      not intend.
+    </p>
+
+    <p>
+      The attacker tricks an authenticated user into submitting a request to the
+      web application. Typically this request will result in a state change on
+      the server, such as changing the user's password. The request can be
+      initiated when the user visits a site controlled by the attacker. If the
+      web application relies only on cookies for authentication, or on other
+      credentials that are automatically included in the request, then this
+      request will appear as legitimate to the server.
+    </p>
+
+    <p>
+      A common countermeasure for CSRF is to generate a unique token to be
+      included in the HTML sent from the server to a user. This token can be
+      used as a hidden field to be sent back with requests to the server, where
+      the server can then check that the token is valid and associated with the
+      relevant user session.
+    </p>
+  </overview>
+
+  <recommendation>
+    <p>
+      In many web frameworks, CSRF protection is enabled by default. In these
+      cases, using the default configuration is sufficient to guard against most
+      CSRF attacks.
+    </p>
+  </recommendation>
+
+  <example>
+    <p>
+      The following example shows a case where CSRF protection is disabled by
+      skipping token verification.
+    </p>
+
+    <sample src="examples/UsersController.rb"/>
+
+    <p>
+      Verification can be re-enabled by removing the call to
+      <code>skip_before_action</code>.
+    </p>
+
+  </example>
+
+  <references>
+    <li>Wikipedia: <a href="https://en.wikipedia.org/wiki/Cross-site_request_forgery">Cross-site request forgery</a></li>
+    <li>OWASP: <a href="https://owasp.org/www-community/attacks/csrf">Cross-site request forgery</a></li>
+    <li>Securing Rails Applications: <a href="https://guides.rubyonrails.org/security.html#cross-site-request-forgery-csrf">Cross-Site Request Forgery (CSRF)</a></li>
+  </references>
+
+</qhelp>

ruby/ql/src/queries/security/cwe-352/CSRFProtectionDisabled.ql

@@ -0,0 +1,19 @@
+/**
+ * @name CSRF protection disabled
+ * @description Disabling CSRF protection makes the application vulnerable to
+ *              a Cross-Site Request Forgery (CSRF) attack.
+ * @kind problem
+ * @problem.severity warning
+ * @security-severity 8.8
+ * @precision high
+ * @id rb/csrf-protection-disabled
+ * @tags security
+ *       external/cwe/cwe-352
+ */
+
+import ruby
+import codeql.ruby.Concepts
+
+from CSRFProtectionSetting s
+where s.getVerificationSetting() = false
+select s, "Potential CSRF vulnerability due to forgery protection being disabled."

ruby/ql/src/queries/security/cwe-352/examples/UsersController.rb

@@ -0,0 +1,3 @@
+class UsersController < ApplicationController
+  skip_before_action :verify_authenticity_token
+end

ruby/ql/test/query-tests/security/cwe-352/CSRFProtectionDisabled.expected

@@ -0,0 +1,4 @@
+| railsapp/app/controllers/users_controller.rb:4:3:4:47 | call to skip_before_action | Potential CSRF vulnerability due to forgery protection being disabled. |
+| railsapp/config/application.rb:15:5:15:53 | call to allow_forgery_protection= | Potential CSRF vulnerability due to forgery protection being disabled. |
+| railsapp/config/environments/development.rb:5:3:5:51 | call to allow_forgery_protection= | Potential CSRF vulnerability due to forgery protection being disabled. |
+| railsapp/config/environments/production.rb:5:3:5:51 | call to allow_forgery_protection= | Potential CSRF vulnerability due to forgery protection being disabled. |

ruby/ql/test/query-tests/security/cwe-352/CSRFProtectionDisabled.qlref

@@ -0,0 +1 @@
+queries/security/cwe-352/CSRFProtectionDisabled.ql

ruby/ql/test/query-tests/security/cwe-352/railsapp/app/controllers/application_controller.rb

@@ -0,0 +1,2 @@
+class ApplicationController < ActionController::Base
+end

ruby/ql/test/query-tests/security/cwe-352/railsapp/app/controllers/users_controller.rb

@@ -0,0 +1,11 @@
+class UsersController < ApplicationController
+
+  # BAD: Disabling forgery protection may open the application to CSRF attacks
+  skip_before_action :verify_authenticity_token
+
+  def change_email
+    user = User.find_by(name: params[:user_name])
+    user.email = params[:new_email]
+    user.save!
+  end
+end

ruby/ql/test/query-tests/security/cwe-352/railsapp/config/application.rb

@@ -0,0 +1,17 @@
+require_relative 'boot'
+
+require 'rails/all'
+
+# Require the gems listed in Gemfile, including any gems
+# you've limited to :test, :development, or :production.
+Bundler.require(*Rails.groups)
+
+module Railsapp
+  class Application < Rails::Application
+    # Initialize configuration defaults for originally generated Rails version.
+    config.load_defaults 6.0
+
+    # BAD: Disabling forgery protection may open the application to CSRF attacks
+    config.action_controller.allow_forgery_protection = false
+  end
+end

ruby/ql/test/query-tests/security/cwe-352/railsapp/config/environment.rb

@@ -0,0 +1,5 @@
+# Load the Rails application.
+require_relative 'application'
+
+# Initialize the Rails application.
+Rails.application.initialize!

ruby/ql/test/query-tests/security/cwe-352/railsapp/config/environments/development.rb

@@ -0,0 +1,6 @@
+Rails.application.configure do
+  # Settings specified here will take precedence over those in config/application.rb.
+
+  # GOOD: disabling CSRF protection in the development environment should not be flagged
+  config.action_controller.allow_forgery_protection = false
+end

ruby/ql/test/query-tests/security/cwe-352/railsapp/config/environments/production.rb

@@ -0,0 +1,6 @@
+Rails.application.configure do
+  # Settings specified here will take precedence over those in config/application.rb.
+
+  # BAD: Disabling forgery protection may open the application to CSRF attacks
+  config.action_controller.allow_forgery_protection = false
+end

ruby/ql/test/query-tests/security/cwe-352/railsapp/config/environments/test.rb

@@ -0,0 +1,11 @@
+# The test environment is used exclusively to run your application's
+# test suite. You never need to work with it otherwise. Remember that
+# your test database is "scratch space" for the test suite and is wiped
+# and recreated between test runs. Don't rely on the data there!
+
+Rails.application.configure do
+  # Settings specified here will take precedence over those in config/application.rb.
+
+  # GOOD: disabling CSRF protection in the test environment should not be flagged
+  config.action_controller.allow_forgery_protection = false
+end

ruby/ql/test/query-tests/security/cwe-352/railsapp/test/controllers/users_controller_test.rb

@@ -0,0 +1,8 @@
+require "test_helper"
+
+class UsersControllerTest < ActiveSupport::TestCase
+  setup do
+    # GOOD: disabling CSRF protection in tests should not be flagged
+    config.action_controller.allow_forgery_protection = false
+  end
+end