diff --git a/ruby/ql/lib/change-notes/2025-09-15-grape-framework-support.md b/ruby/ql/lib/change-notes/2025-09-15-grape-framework-support.md index 258da40d36c..08ceed887f2 100644 --- a/ruby/ql/lib/change-notes/2025-09-15-grape-framework-support.md +++ b/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. \ No newline at end of file +* Initial modeling for the Ruby Grape framework in `Grape.qll` has been added to detect API endpoints, parameters, and headers within Grape API classes. \ No newline at end of file diff --git a/ruby/ql/lib/codeql/ruby/frameworks/Grape.qll b/ruby/ql/lib/codeql/ruby/frameworks/Grape.qll index a1646b8654c..31632e01948 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/Grape.qll +++ b/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() ) } } diff --git a/ruby/ql/test/library-tests/frameworks/grape/Flow.expected b/ruby/ql/test/library-tests/frameworks/grape/Flow.expected index 0fd19d4eace..c104b36afb2 100644 --- a/ruby/ql/test/library-tests/frameworks/grape/Flow.expected +++ b/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 | diff --git a/ruby/ql/test/library-tests/frameworks/grape/app.rb b/ruby/ql/test/library-tests/frameworks/grape/app.rb index 6fbb184cab9..81f46482687 100644 --- a/ruby/ql/test/library-tests/frameworks/grape/app.rb +++ b/ruby/ql/test/library-tests/frameworks/grape/app.rb @@ -141,7 +141,7 @@ class UserAPI < Grape::API # 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 @@ -149,7 +149,7 @@ class UserAPI < Grape::API 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