From 059c4d38ad97b4972f0be1e4e878fbd32ad3b10c Mon Sep 17 00:00:00 2001
From: thiggy1342 <thiggy1342@github.com>
Date: Sat, 18 Jun 2022 18:26:45 +0000
Subject: [PATCH] refine query to use appropriate types

---
 .../ManuallyCheckHttpVerb.ql                  | 26 ++++++++++---------
 .../ManuallyCheckHttpVerb.expected            |  0
 2 files changed, 14 insertions(+), 12 deletions(-)
 create mode 100644 ruby/ql/test/query-tests/security/manually-check-http-verb/ManuallyCheckHttpVerb.expected

diff --git a/ruby/ql/src/experimental/manually-check-http-verb/ManuallyCheckHttpVerb.ql b/ruby/ql/src/experimental/manually-check-http-verb/ManuallyCheckHttpVerb.ql
index ac34ad34088..7396e75b920 100644
--- a/ruby/ql/src/experimental/manually-check-http-verb/ManuallyCheckHttpVerb.ql
+++ b/ruby/ql/src/experimental/manually-check-http-verb/ManuallyCheckHttpVerb.ql
@@ -12,15 +12,14 @@
 import ruby
 import codeql.ruby.DataFlow
 
-class ManuallyCheckHttpVerb extends DataFlow::CallNode {
-  ManuallyCheckHttpVerb() {
+class HttpVerbMethod extends MethodCall {
+  HttpVerbMethod() {
     this instanceof CheckGetRequest or
     this instanceof CheckPostRequest or
     this instanceof CheckPatchRequest or
     this instanceof CheckPostRequest or
     this instanceof CheckDeleteRequest or
-    this instanceof CheckHeadRequest or
-    this instanceof CheckRequestMethodFromEnv
+    this instanceof CheckHeadRequest
   }
 }
 
@@ -53,30 +52,33 @@ class GetRequestMethodFromEnv extends ElementReference {
   }
 }
 
-class CheckGetRequest extends DataFlow::CallNode {
+class CheckGetRequest extends MethodCall {
   CheckGetRequest() { this.getMethodName() = "get?" }
 }
 
-class CheckPostRequest extends DataFlow::CallNode {
+class CheckPostRequest extends MethodCall {
   CheckPostRequest() { this.getMethodName() = "post?" }
 }
 
-class CheckPutRequest extends DataFlow::CallNode {
+class CheckPutRequest extends MethodCall {
   CheckPutRequest() { this.getMethodName() = "put?" }
 }
 
-class CheckPatchRequest extends DataFlow::CallNode {
+class CheckPatchRequest extends MethodCall {
   CheckPatchRequest() { this.getMethodName() = "patch?" }
 }
 
-class CheckDeleteRequest extends DataFlow::CallNode {
+class CheckDeleteRequest extends MethodCall {
   CheckDeleteRequest() { this.getMethodName() = "delete?" }
 }
 
-class CheckHeadRequest extends DataFlow::CallNode {
+class CheckHeadRequest extends MethodCall {
   CheckHeadRequest() { this.getMethodName() = "head?" }
 }
 
-from ManuallyCheckHttpVerb check
-select check,
+from CheckRequestMethodFromEnv env, AstNode node
+where
+  node instanceof HttpVerbMethod or
+  node = env.asExpr().getExpr()
+select node,
   "Manually checking HTTP verbs is an indication that multiple requests are routed to the same controller action. This could lead to bypassing necessary authorization methods and other protections, like CSRF protection. Prefer using different controller actions for each HTTP method and relying Rails routing to handle mappting resources and verbs to specific methods."
diff --git a/ruby/ql/test/query-tests/security/manually-check-http-verb/ManuallyCheckHttpVerb.expected b/ruby/ql/test/query-tests/security/manually-check-http-verb/ManuallyCheckHttpVerb.expected
new file mode 100644
index 00000000000..e69de29bb2d