Ruby: flag up protect_from_forgery calls without an exception strategy

Author: alexrford

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

@@ -297,3 +297,23 @@ class ActionControllerSkipForgeryProtectionCall extends CSRFProtectionSetting::R
 
   override boolean getVerificationSetting() { result = false }
 }
+
+/**
+ * A call to `protect_from_forgery`.
+ */
+private class ActionControllerProtectFromForgeryCall extends CSRFProtectionSetting::Range {
+  private ActionControllerContextCall callExpr;
+
+  ActionControllerProtectFromForgeryCall() {
+    callExpr = this.asExpr().getExpr() and
+    callExpr.getMethodName() = "protect_from_forgery"
+  }
+
+  private string getWithValueText() { result = callExpr.getKeywordArgument("with").getValueText() }
+
+  // Calls without `with: :exception` can allow for bypassing CSRF protection
+  // in some scenarios.
+  override boolean getVerificationSetting() {
+    if this.getWithValueText() = "exception" then result = true else result = false
+  }
+}

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

@@ -50,12 +50,23 @@
       <code>skip_before_action</code>.
     </p>
 
+    <p>
+      Care should be taken when using the Rails
+      <code>protect_from_forgery</code> method to prevent CSRF. The default
+      behaviour of this method is to null the session when an invalid CSRF token
+      is provided. This may not be sufficient to avoid a CSRF vulnerability -
+      for example if parts of the session are memoized. Calling
+      <code>protect_from_forgery with: :exception</code> can help to avoid this
+      by raising an exception on an invalid CSRF token instead.
+    </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>
+    <li>Veracode: <a href="https://www.veracode.com/blog/managing-appsec/when-rails-protectfromforgery-fails">When Rails' protect_from_forgery Fails</a>.</li>
   </references>
 
 </qhelp>

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

@@ -1,7 +1,7 @@
 /**
- * @name CSRF protection disabled
- * @description Disabling CSRF protection makes the application vulnerable to
- *              a Cross-Site Request Forgery (CSRF) attack.
+ * @name CSRF protection weakened or disabled
+ * @description Disabling or weakening CSRF protection may make the application
+ *              vulnerable to a Cross-Site Request Forgery (CSRF) attack.
  * @kind problem
  * @problem.severity warning
  * @security-severity 8.8
@@ -16,4 +16,4 @@ import codeql.ruby.Concepts
 
 from CSRFProtectionSetting s
 where s.getVerificationSetting() = false
-select s, "Potential CSRF vulnerability due to forgery protection being disabled."
+select s, "Potential CSRF vulnerability due to forgery protection being disabled or weakened."