Skip to content

Commit 6d6f8ba

Browse files
committed
Ruby: Make CSRF query more sensitive
Generate an alert for every controller class that doesn't have or inherity a `protect_from_forgery` setting.
1 parent 49d826f commit 6d6f8ba

File tree

4 files changed

+21
-7
lines changed

4 files changed

+21
-7
lines changed

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,17 @@ import codeql.ruby.AST
1515
import codeql.ruby.Concepts
1616
import codeql.ruby.frameworks.ActionController
1717

18-
from ActionController::RootController c
19-
where
20-
not exists(ActionController::ProtectFromForgeryCall call |
21-
c.getSelf().flowsTo(call.getReceiver())
22-
)
23-
select c, "Potential CSRF vulnerability due to forgery protection not being enabled"
18+
/**
19+
* Holds if a call to `protect_from_forgery` is made in the controller class `definedIn`,
20+
* which is inherited by the controller class `child`.
21+
*/
22+
private predicate protectFromForgeryCall(
23+
ActionControllerClass definedIn, ActionControllerClass child,
24+
ActionController::ProtectFromForgeryCall call
25+
) {
26+
definedIn.getSelf().flowsTo(call.getReceiver()) and child = definedIn.getADescendent()
27+
}
28+
29+
from ActionControllerClass c
30+
where not protectFromForgeryCall(_, c, _)
31+
select c, "Potential CSRF vulnerability due to forgery protection not being enabled."
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +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 |
1+
| railsapp/app/controllers/alternative_root_controller.rb:1:1:3:3 | AlternativeRootController | Potential CSRF vulnerability due to forgery protection not being enabled. |
2+
| railsapp/app/controllers/tags_controller.rb:1:1:2:3 | TagsController | Potential CSRF vulnerability due to forgery protection not being enabled. |
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
class SubscriptionsController < AlternativeRootController
2+
protect_from_forgery with: :exception
3+
end
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
class TagsController < AlternativeRootController
2+
end

0 commit comments

Comments
 (0)