Convert from GrapeHelperMethodTaintStep extends AdditionalTaintStep to a simplified GrapeHelperMethodTarget extends AdditionalCallTarget

Author: felickz

ruby/ql/lib/change-notes/2025-09-15-grape-framework-support.md

@@ -1,4 +1,4 @@
 ---
 category: feature
 ---
-* Initial modeling for the Ruby Grape framework in `Grape.qll` have been added to detect API endpoints, parameters, and headers within Grape API classes.
+* Initial modeling for the Ruby Grape framework in `Grape.qll` has been added to detect API endpoints, parameters, and headers within Grape API classes.

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

@@ -3,6 +3,7 @@
  */
 
 private import codeql.ruby.AST
+private import codeql.ruby.CFG
 private import codeql.ruby.Concepts
 private import codeql.ruby.controlflow.CfgNodes
 private import codeql.ruby.DataFlow
@@ -301,32 +302,20 @@ private class GrapeHelperMethod extends Method {
 }
 
 /**
- * Additional taint step to model dataflow from method arguments to parameters
- * and from return values back to call sites for Grape helper methods defined in `helpers` blocks.
- * This bridges the gap where standard dataflow doesn't recognize the Grape DSL semantics.
+ * Additional call-target to resolve helper method calls defined in `helpers` blocks.
+ *
+ * This class is responsible for resolving calls to helper methods defined in
+ * `helpers` blocks, allowing the dataflow framework to accurately track
+ * the flow of information between these methods and their call sites.
  */
-private class GrapeHelperMethodTaintStep extends AdditionalTaintStep {
-  override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
-    // Map arguments to parameters for helper method calls
-    exists(GrapeHelperMethod helperMethod, MethodCall call, int i |
-      // Find calls to helper methods from within Grape endpoints or other helper methods
-      call.getMethodName() = helperMethod.getName() and
-      exists(GrapeApiClass api | call.getParent+() = api.getADeclaration()) and
-      // Map argument to parameter
-      nodeFrom.asExpr().getExpr() = call.getArgument(i) and
-      nodeTo.asParameter() = helperMethod.getParameter(i)
-    )
-    or
-    // Model implicit return values: the last expression in a helper method flows to the call site
-    exists(GrapeHelperMethod helperMethod, MethodCall helperCall, Expr lastExpr |
-      // Find calls to helper methods from within Grape endpoints or other helper methods
-      helperCall.getMethodName() = helperMethod.getName() and
-      exists(GrapeApiClass api | helperCall.getParent+() = api.getADeclaration()) and
-      // Get the last expression in the helper method (Ruby's implicit return)
-      lastExpr = helperMethod.getLastStmt() and
-      // Flow from the last expression in the helper method to the call site
-      nodeFrom.asExpr().getExpr() = lastExpr and
-      nodeTo.asExpr().getExpr() = helperCall
+private class GrapeHelperMethodTarget extends AdditionalCallTarget {
+  override DataFlowCallable viableTarget(CfgNodes::ExprNodes::CallCfgNode call) {
+    // Find calls to helper methods from within Grape endpoints or other helper methods
+    exists(GrapeHelperMethod helperMethod, MethodCall mc |
+      result.asCfgScope() = helperMethod and
+      mc = call.getAstNode() and
+      mc.getMethodName() = helperMethod.getName() and
+      mc.getParent+() = helperMethod.getApiClass().getADeclaration()
     )
   }
 }

ruby/ql/test/library-tests/frameworks/grape/Flow.expected

@@ -1,10 +1,15 @@
 models
 edges
 | app.rb:103:13:103:18 | call to params | app.rb:103:13:103:70 | call to select | provenance |  |
-| app.rb:103:13:103:70 | call to select | app.rb:149:21:149:31 | call to user_params | provenance | AdditionalTaintStep |
-| app.rb:103:13:103:70 | call to select | app.rb:165:21:165:31 | call to user_params | provenance | AdditionalTaintStep |
-| app.rb:107:13:107:32 | call to source | app.rb:143:18:143:43 | call to vulnerable_helper | provenance | AdditionalTaintStep |
-| app.rb:111:13:111:33 | call to source | app.rb:150:25:150:37 | call to simple_helper | provenance | AdditionalTaintStep |
+| app.rb:103:13:103:18 | call to params | app.rb:103:13:103:70 | call to select : [collection] [element] | provenance |  |
+| app.rb:103:13:103:70 | call to select | app.rb:149:21:149:31 | call to user_params | provenance |  |
+| app.rb:103:13:103:70 | call to select | app.rb:165:21:165:31 | call to user_params | provenance |  |
+| app.rb:103:13:103:70 | call to select : [collection] [element] | app.rb:149:21:149:31 | call to user_params : [collection] [element] | provenance |  |
+| app.rb:103:13:103:70 | call to select : [collection] [element] | app.rb:165:21:165:31 | call to user_params : [collection] [element] | provenance |  |
+| app.rb:107:13:107:32 | call to source | app.rb:143:18:143:43 | call to vulnerable_helper | provenance |  |
+| app.rb:107:13:107:32 | call to source | app.rb:143:18:143:43 | call to vulnerable_helper | provenance |  |
+| app.rb:111:13:111:33 | call to source | app.rb:150:25:150:37 | call to simple_helper | provenance |  |
+| app.rb:111:13:111:33 | call to source | app.rb:150:25:150:37 | call to simple_helper | provenance |  |
 | app.rb:126:9:126:15 | user_id | app.rb:133:14:133:20 | user_id | provenance |  |
 | app.rb:126:19:126:24 | call to params | app.rb:126:19:126:34 | ...[...] | provenance |  |
 | app.rb:126:19:126:34 | ...[...] | app.rb:126:9:126:15 | user_id | provenance |  |
@@ -17,20 +22,31 @@ edges
 | app.rb:129:19:129:25 | call to cookies | app.rb:129:19:129:38 | ...[...] | provenance |  |
 | app.rb:129:19:129:38 | ...[...] | app.rb:129:9:129:15 | session | provenance |  |
 | app.rb:143:9:143:14 | result | app.rb:144:14:144:19 | result | provenance |  |
+| app.rb:143:9:143:14 | result | app.rb:144:14:144:19 | result | provenance |  |
+| app.rb:143:18:143:43 | call to vulnerable_helper | app.rb:143:9:143:14 | result | provenance |  |
 | app.rb:143:18:143:43 | call to vulnerable_helper | app.rb:143:9:143:14 | result | provenance |  |
 | app.rb:149:9:149:17 | user_data | app.rb:151:14:151:22 | user_data | provenance |  |
+| app.rb:149:9:149:17 | user_data : [collection] [element] | app.rb:151:14:151:22 | user_data | provenance |  |
 | app.rb:149:21:149:31 | call to user_params | app.rb:149:9:149:17 | user_data | provenance |  |
+| app.rb:149:21:149:31 | call to user_params : [collection] [element] | app.rb:149:9:149:17 | user_data : [collection] [element] | provenance |  |
 | app.rb:150:9:150:21 | simple_result | app.rb:152:14:152:26 | simple_result | provenance |  |
+| app.rb:150:9:150:21 | simple_result | app.rb:152:14:152:26 | simple_result | provenance |  |
+| app.rb:150:25:150:37 | call to simple_helper | app.rb:150:9:150:21 | simple_result | provenance |  |
 | app.rb:150:25:150:37 | call to simple_helper | app.rb:150:9:150:21 | simple_result | provenance |  |
 | app.rb:159:13:159:19 | user_id | app.rb:160:18:160:24 | user_id | provenance |  |
 | app.rb:159:23:159:28 | call to params | app.rb:159:23:159:33 | ...[...] | provenance |  |
 | app.rb:159:23:159:33 | ...[...] | app.rb:159:13:159:19 | user_id | provenance |  |
 | app.rb:165:9:165:17 | user_data | app.rb:166:14:166:22 | user_data | provenance |  |
+| app.rb:165:9:165:17 | user_data : [collection] [element] | app.rb:166:14:166:22 | user_data | provenance |  |
 | app.rb:165:21:165:31 | call to user_params | app.rb:165:9:165:17 | user_data | provenance |  |
+| app.rb:165:21:165:31 | call to user_params : [collection] [element] | app.rb:165:9:165:17 | user_data : [collection] [element] | provenance |  |
 nodes
 | app.rb:103:13:103:18 | call to params | semmle.label | call to params |
 | app.rb:103:13:103:70 | call to select | semmle.label | call to select |
+| app.rb:103:13:103:70 | call to select : [collection] [element] | semmle.label | call to select : [collection] [element] |
 | app.rb:107:13:107:32 | call to source | semmle.label | call to source |
+| app.rb:107:13:107:32 | call to source | semmle.label | call to source |
+| app.rb:111:13:111:33 | call to source | semmle.label | call to source |
 | app.rb:111:13:111:33 | call to source | semmle.label | call to source |
 | app.rb:126:9:126:15 | user_id | semmle.label | user_id |
 | app.rb:126:19:126:24 | call to params | semmle.label | call to params |
@@ -48,20 +64,30 @@ nodes
 | app.rb:135:14:135:17 | auth | semmle.label | auth |
 | app.rb:136:14:136:20 | session | semmle.label | session |
 | app.rb:143:9:143:14 | result | semmle.label | result |
+| app.rb:143:9:143:14 | result | semmle.label | result |
 | app.rb:143:18:143:43 | call to vulnerable_helper | semmle.label | call to vulnerable_helper |
+| app.rb:143:18:143:43 | call to vulnerable_helper | semmle.label | call to vulnerable_helper |
+| app.rb:144:14:144:19 | result | semmle.label | result |
 | app.rb:144:14:144:19 | result | semmle.label | result |
 | app.rb:149:9:149:17 | user_data | semmle.label | user_data |
+| app.rb:149:9:149:17 | user_data : [collection] [element] | semmle.label | user_data : [collection] [element] |
 | app.rb:149:21:149:31 | call to user_params | semmle.label | call to user_params |
+| app.rb:149:21:149:31 | call to user_params : [collection] [element] | semmle.label | call to user_params : [collection] [element] |
 | app.rb:150:9:150:21 | simple_result | semmle.label | simple_result |
+| app.rb:150:9:150:21 | simple_result | semmle.label | simple_result |
+| app.rb:150:25:150:37 | call to simple_helper | semmle.label | call to simple_helper |
 | app.rb:150:25:150:37 | call to simple_helper | semmle.label | call to simple_helper |
 | app.rb:151:14:151:22 | user_data | semmle.label | user_data |
 | app.rb:152:14:152:26 | simple_result | semmle.label | simple_result |
+| app.rb:152:14:152:26 | simple_result | semmle.label | simple_result |
 | app.rb:159:13:159:19 | user_id | semmle.label | user_id |
 | app.rb:159:23:159:28 | call to params | semmle.label | call to params |
 | app.rb:159:23:159:33 | ...[...] | semmle.label | ...[...] |
 | app.rb:160:18:160:24 | user_id | semmle.label | user_id |
 | app.rb:165:9:165:17 | user_data | semmle.label | user_data |
+| app.rb:165:9:165:17 | user_data : [collection] [element] | semmle.label | user_data : [collection] [element] |
 | app.rb:165:21:165:31 | call to user_params | semmle.label | call to user_params |
+| app.rb:165:21:165:31 | call to user_params : [collection] [element] | semmle.label | call to user_params : [collection] [element] |
 | app.rb:166:14:166:22 | user_data | semmle.label | user_data |
 subpaths
 testFailures
@@ -71,7 +97,9 @@ testFailures
 | app.rb:135:14:135:17 | auth | app.rb:128:16:128:22 | call to headers | app.rb:135:14:135:17 | auth | $@ | app.rb:128:16:128:22 | call to headers | call to headers |
 | app.rb:136:14:136:20 | session | app.rb:129:19:129:25 | call to cookies | app.rb:136:14:136:20 | session | $@ | app.rb:129:19:129:25 | call to cookies | call to cookies |
 | app.rb:144:14:144:19 | result | app.rb:107:13:107:32 | call to source | app.rb:144:14:144:19 | result | $@ | app.rb:107:13:107:32 | call to source | call to source |
+| app.rb:144:14:144:19 | result | app.rb:107:13:107:32 | call to source | app.rb:144:14:144:19 | result | $@ | app.rb:107:13:107:32 | call to source | call to source |
 | app.rb:151:14:151:22 | user_data | app.rb:103:13:103:18 | call to params | app.rb:151:14:151:22 | user_data | $@ | app.rb:103:13:103:18 | call to params | call to params |
 | app.rb:152:14:152:26 | simple_result | app.rb:111:13:111:33 | call to source | app.rb:152:14:152:26 | simple_result | $@ | app.rb:111:13:111:33 | call to source | call to source |
+| app.rb:152:14:152:26 | simple_result | app.rb:111:13:111:33 | call to source | app.rb:152:14:152:26 | simple_result | $@ | app.rb:111:13:111:33 | call to source | call to source |
 | app.rb:160:18:160:24 | user_id | app.rb:159:23:159:28 | call to params | app.rb:160:18:160:24 | user_id | $@ | app.rb:159:23:159:28 | call to params | call to params |
 | app.rb:166:14:166:22 | user_data | app.rb:103:13:103:18 | call to params | app.rb:166:14:166:22 | user_data | $@ | app.rb:103:13:103:18 | call to params | call to params |

ruby/ql/test/library-tests/frameworks/grape/app.rb

@@ -141,15 +141,15 @@ def simple_helper
         # Test helper method parameter passing dataflow
         user_id = params[:user_id]
         result = vulnerable_helper(user_id)
-        sink result # $ hasTaintFlow=paramHelper
+        sink result # $ hasValueFlow=paramHelper
     end
 
     post '/users' do
         # Test helper method return dataflow
         user_data = user_params
         simple_result = simple_helper
         sink user_data # $ hasTaintFlow
-        sink simple_result # $ hasTaintFlow=simpleHelper
+        sink simple_result # $ hasValueFlow=simpleHelper
     end
 
     # Test route_param block pattern