diff --git a/ruby/ql/lib/codeql/ruby/frameworks/Grape.qll b/ruby/ql/lib/codeql/ruby/frameworks/Grape.qll index 857b849f425..a3aa2f684c7 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/Grape.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/Grape.qll @@ -12,6 +12,7 @@ private import codeql.ruby.typetracking.TypeTracking private import codeql.ruby.frameworks.Rails private import codeql.ruby.frameworks.internal.Rails private import codeql.ruby.dataflow.internal.DataFlowDispatch +private import codeql.ruby.dataflow.FlowSteps /** * Provides modeling for Grape, a REST-like API framework for Ruby. @@ -125,21 +126,17 @@ class GrapeParamsSource extends Http::Server::RequestInputAccess::Range { } /** - * A call to `params` from within a Grape API endpoint. + * A call to `params` from within a Grape API endpoint or helper method. */ private class GrapeParamsCall extends ParamsCallImpl { GrapeParamsCall() { - exists(GrapeEndpoint endpoint | - this.getParent+() = endpoint.getBody().asCallableAstNode() and - this.getMethodName() = "params" + // Simplified approach: find params calls that are descendants of Grape API class methods + exists(GrapeAPIClass api | + this.getMethodName() = "params" and + this.getParent+() = api.getADeclaration() ) - or - // Also handle cases where params is called on an instance of a Grape API class - this = grapeAPIInstance().getAMethodCall("params").asExpr().getExpr() } -} - -/** +}/** * A call to `headers` from within a Grape API endpoint. * Headers can also be a source of user input. */ @@ -195,4 +192,44 @@ private class GrapeRequestCall extends MethodCall { // Also handle cases where request is called on an instance of a Grape API class this = grapeAPIInstance().getAMethodCall("request").asExpr().getExpr() } +} + +/** + * A method defined within a `helpers` block in a Grape API class. + * These methods become available in endpoint contexts through Grape's DSL. + */ +private class GrapeHelperMethod extends Method { + private GrapeAPIClass apiClass; + + GrapeHelperMethod() { + exists(DataFlow::CallNode helpersCall | + helpersCall = apiClass.getAModuleLevelCall("helpers") and + this.getParent+() = helpersCall.getBlock().asExpr().getExpr() + ) + } + + /** + * Gets the API class that contains this helper method. + */ + GrapeAPIClass getAPIClass() { result = apiClass } +} + +/** + * Additional taint step to model dataflow from method arguments to parameters + * for Grape helper methods defined in `helpers` blocks. + * This bridges the gap where standard dataflow doesn't recognize the Grape DSL semantics. + */ +private class GrapeHelperMethodTaintStep extends AdditionalTaintStep { + override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { + exists(GrapeHelperMethod helperMethod, MethodCall call, int i | + // Find calls to helper methods from within Grape endpoints + call.getMethodName() = helperMethod.getName() and + exists(GrapeEndpoint endpoint | + call.getParent+() = endpoint.getBody().asExpr().getExpr() + ) and + // Map argument to parameter + nodeFrom.asExpr().getExpr() = call.getArgument(i) and + nodeTo.asParameter() = helperMethod.getParameter(i) + ) + } } \ No newline at end of file diff --git a/ruby/ql/test/library-tests/frameworks/grape/Grape.expected b/ruby/ql/test/library-tests/frameworks/grape/Grape.expected index 904cb36333a..7e792465911 100644 --- a/ruby/ql/test/library-tests/frameworks/grape/Grape.expected +++ b/ruby/ql/test/library-tests/frameworks/grape/Grape.expected @@ -22,4 +22,4 @@ grapeHeaders | app.rb:9:18:9:24 | call to headers | | app.rb:46:5:46:11 | call to headers | grapeRequest -| app.rb:25:12:25:18 | call to request | \ No newline at end of file +| app.rb:25:12:25:18 | call to request | diff --git a/ruby/ql/test/query-tests/security/cwe-089/ArelInjection.rb b/ruby/ql/test/query-tests/security/cwe-089/ArelInjection.rb index 30832894b9e..cf0769c0acd 100644 --- a/ruby/ql/test/query-tests/security/cwe-089/ArelInjection.rb +++ b/ruby/ql/test/query-tests/security/cwe-089/ArelInjection.rb @@ -15,4 +15,27 @@ class PotatoAPI < Grape::API sql = Arel.sql("SELECT * FROM users WHERE name = #{name}") sql = Arel::Nodes::SqlLiteral.new("SELECT * FROM users WHERE name = #{name}") end -end \ No newline at end of file +end + +class SimpleAPI < Grape::API + get '/test' do + x = params[:name] + Arel.sql("SELECT * FROM users WHERE name = #{x}") + end +end + + # Test helper method pattern in Grape helpers block + class TestAPI < Grape::API + helpers do + def vulnerable_helper(user_id) + # BAD: SQL statement constructed from user input passed as parameter + Arel.sql("SELECT * FROM users WHERE id = #{user_id}") + end + end + + get '/helper_test' do + # This should be detected as SQL injection via helper method + user_id = params[:user_id] + vulnerable_helper(user_id) + end + end \ No newline at end of file diff --git a/ruby/ql/test/query-tests/security/cwe-089/SqlInjection.expected b/ruby/ql/test/query-tests/security/cwe-089/SqlInjection.expected index b8b1350882d..0b14504058e 100644 --- a/ruby/ql/test/query-tests/security/cwe-089/SqlInjection.expected +++ b/ruby/ql/test/query-tests/security/cwe-089/SqlInjection.expected @@ -85,6 +85,14 @@ edges | ArelInjection.rb:13:5:13:8 | name | ArelInjection.rb:16:39:16:80 | "SELECT * FROM users WHERE nam..." | provenance | AdditionalTaintStep | | ArelInjection.rb:13:12:13:17 | call to params | ArelInjection.rb:13:12:13:29 | ...[...] | provenance | | | ArelInjection.rb:13:12:13:29 | ...[...] | ArelInjection.rb:13:5:13:8 | name | provenance | | +| ArelInjection.rb:22:5:22:5 | x | ArelInjection.rb:23:14:23:52 | "SELECT * FROM users WHERE nam..." | provenance | AdditionalTaintStep | +| ArelInjection.rb:22:9:22:14 | call to params | ArelInjection.rb:22:9:22:21 | ...[...] | provenance | | +| ArelInjection.rb:22:9:22:21 | ...[...] | ArelInjection.rb:22:5:22:5 | x | provenance | | +| ArelInjection.rb:30:29:30:35 | user_id | ArelInjection.rb:32:18:32:60 | "SELECT * FROM users WHERE id ..." | provenance | AdditionalTaintStep | +| ArelInjection.rb:38:7:38:13 | user_id | ArelInjection.rb:39:25:39:31 | user_id | provenance | | +| ArelInjection.rb:38:17:38:22 | call to params | ArelInjection.rb:38:17:38:32 | ...[...] | provenance | | +| ArelInjection.rb:38:17:38:32 | ...[...] | ArelInjection.rb:38:7:38:13 | user_id | provenance | | +| ArelInjection.rb:39:25:39:31 | user_id | ArelInjection.rb:30:29:30:35 | user_id | provenance | AdditionalTaintStep | | PgInjection.rb:6:5:6:8 | name | PgInjection.rb:13:5:13:8 | qry1 : String | provenance | AdditionalTaintStep | | PgInjection.rb:6:5:6:8 | name | PgInjection.rb:19:5:19:8 | qry2 : String | provenance | AdditionalTaintStep | | PgInjection.rb:6:5:6:8 | name | PgInjection.rb:31:5:31:8 | qry3 : String | provenance | AdditionalTaintStep | @@ -218,6 +226,16 @@ nodes | ArelInjection.rb:13:12:13:29 | ...[...] | semmle.label | ...[...] | | ArelInjection.rb:15:20:15:61 | "SELECT * FROM users WHERE nam..." | semmle.label | "SELECT * FROM users WHERE nam..." | | ArelInjection.rb:16:39:16:80 | "SELECT * FROM users WHERE nam..." | semmle.label | "SELECT * FROM users WHERE nam..." | +| ArelInjection.rb:22:5:22:5 | x | semmle.label | x | +| ArelInjection.rb:22:9:22:14 | call to params | semmle.label | call to params | +| ArelInjection.rb:22:9:22:21 | ...[...] | semmle.label | ...[...] | +| ArelInjection.rb:23:14:23:52 | "SELECT * FROM users WHERE nam..." | semmle.label | "SELECT * FROM users WHERE nam..." | +| ArelInjection.rb:30:29:30:35 | user_id | semmle.label | user_id | +| ArelInjection.rb:32:18:32:60 | "SELECT * FROM users WHERE id ..." | semmle.label | "SELECT * FROM users WHERE id ..." | +| ArelInjection.rb:38:7:38:13 | user_id | semmle.label | user_id | +| ArelInjection.rb:38:17:38:22 | call to params | semmle.label | call to params | +| ArelInjection.rb:38:17:38:32 | ...[...] | semmle.label | ...[...] | +| ArelInjection.rb:39:25:39:31 | user_id | semmle.label | user_id | | PgInjection.rb:6:5:6:8 | name | semmle.label | name | | PgInjection.rb:6:12:6:17 | call to params | semmle.label | call to params | | PgInjection.rb:6:12:6:24 | ...[...] | semmle.label | ...[...] | @@ -277,6 +295,8 @@ subpaths | ArelInjection.rb:7:39:7:80 | "SELECT * FROM users WHERE nam..." | ArelInjection.rb:4:12:4:17 | call to params | ArelInjection.rb:7:39:7:80 | "SELECT * FROM users WHERE nam..." | This SQL query depends on a $@. | ArelInjection.rb:4:12:4:17 | call to params | user-provided value | | ArelInjection.rb:15:20:15:61 | "SELECT * FROM users WHERE nam..." | ArelInjection.rb:13:12:13:17 | call to params | ArelInjection.rb:15:20:15:61 | "SELECT * FROM users WHERE nam..." | This SQL query depends on a $@. | ArelInjection.rb:13:12:13:17 | call to params | user-provided value | | ArelInjection.rb:16:39:16:80 | "SELECT * FROM users WHERE nam..." | ArelInjection.rb:13:12:13:17 | call to params | ArelInjection.rb:16:39:16:80 | "SELECT * FROM users WHERE nam..." | This SQL query depends on a $@. | ArelInjection.rb:13:12:13:17 | call to params | user-provided value | +| ArelInjection.rb:23:14:23:52 | "SELECT * FROM users WHERE nam..." | ArelInjection.rb:22:9:22:14 | call to params | ArelInjection.rb:23:14:23:52 | "SELECT * FROM users WHERE nam..." | This SQL query depends on a $@. | ArelInjection.rb:22:9:22:14 | call to params | user-provided value | +| ArelInjection.rb:32:18:32:60 | "SELECT * FROM users WHERE id ..." | ArelInjection.rb:38:17:38:22 | call to params | ArelInjection.rb:32:18:32:60 | "SELECT * FROM users WHERE id ..." | This SQL query depends on a $@. | ArelInjection.rb:38:17:38:22 | call to params | user-provided value | | PgInjection.rb:14:15:14:18 | qry1 | PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:14:15:14:18 | qry1 | This SQL query depends on a $@. | PgInjection.rb:6:12:6:17 | call to params | user-provided value | | PgInjection.rb:15:21:15:24 | qry1 | PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:15:21:15:24 | qry1 | This SQL query depends on a $@. | PgInjection.rb:6:12:6:17 | call to params | user-provided value | | PgInjection.rb:20:22:20:25 | qry2 | PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:20:22:20:25 | qry2 | This SQL query depends on a $@. | PgInjection.rb:6:12:6:17 | call to params | user-provided value |