From f31a483898298d0c71d45b2ad54b707aaa7c63fd Mon Sep 17 00:00:00 2001 From: Harry Maclean Date: Tue, 10 Oct 2023 15:37:32 +0100 Subject: [PATCH] Ruby: reduce duplicate alerts for csrf query Only generate an alert on the top-most vulnerable Rails controller in the controller tree. --- .../queries/security/cwe-352/CSRFProtectionNotEnabled.ql | 6 ++++-- .../security/cwe-352/CSRFProtectionNotEnabled.expected | 1 - 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ruby/ql/src/queries/security/cwe-352/CSRFProtectionNotEnabled.ql b/ruby/ql/src/queries/security/cwe-352/CSRFProtectionNotEnabled.ql index cd7961d2f9a08..f3631eb455605 100644 --- a/ruby/ql/src/queries/security/cwe-352/CSRFProtectionNotEnabled.ql +++ b/ruby/ql/src/queries/security/cwe-352/CSRFProtectionNotEnabled.ql @@ -18,7 +18,7 @@ import codeql.ruby.frameworks.Gemfile /** * Holds if a call to `protect_from_forgery` is made in the controller class `definedIn`, - * which is inherited by the controller class `child`. + * which is inherited by the controller class `child`. These classes may be the same. */ private predicate protectFromForgeryCall( ActionControllerClass definedIn, ActionControllerClass child, @@ -45,5 +45,7 @@ where railsPreVersion3() or not any(MethodCall m).getMethodName() = ["csrf_meta_tags", "csrf_meta_tag"] - ) + ) and + // Only generate alerts for the topmost controller in the tree. + not exists(ActionControllerClass parent | c = parent.getAnImmediateDescendent()) select c, "Potential CSRF vulnerability due to forgery protection not being enabled." diff --git a/ruby/ql/test/query-tests/security/cwe-352/CSRFProtectionNotEnabled.expected b/ruby/ql/test/query-tests/security/cwe-352/CSRFProtectionNotEnabled.expected index 52e2b1aaa4b83..50da7dc076686 100644 --- a/ruby/ql/test/query-tests/security/cwe-352/CSRFProtectionNotEnabled.expected +++ b/ruby/ql/test/query-tests/security/cwe-352/CSRFProtectionNotEnabled.expected @@ -1,2 +1 @@ | railsapp/app/controllers/alternative_root_controller.rb:1:1:3:3 | AlternativeRootController | Potential CSRF vulnerability due to forgery protection not being enabled. | -| railsapp/app/controllers/tags_controller.rb:1:1:2:3 | TagsController | Potential CSRF vulnerability due to forgery protection not being enabled. |