Skip to content

Commit 3252bd3

Browse files
committed
Enhance Grape framework with additional data flow modeling and helper method support
1 parent 738ab6f commit 3252bd3

File tree

4 files changed

+92
-12
lines changed

4 files changed

+92
-12
lines changed

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

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ private import codeql.ruby.typetracking.TypeTracking
1212
private import codeql.ruby.frameworks.Rails
1313
private import codeql.ruby.frameworks.internal.Rails
1414
private import codeql.ruby.dataflow.internal.DataFlowDispatch
15+
private import codeql.ruby.dataflow.FlowSteps
1516

1617
/**
1718
* Provides modeling for Grape, a REST-like API framework for Ruby.
@@ -125,21 +126,17 @@ class GrapeParamsSource extends Http::Server::RequestInputAccess::Range {
125126
}
126127

127128
/**
128-
* A call to `params` from within a Grape API endpoint.
129+
* A call to `params` from within a Grape API endpoint or helper method.
129130
*/
130131
private class GrapeParamsCall extends ParamsCallImpl {
131132
GrapeParamsCall() {
132-
exists(GrapeEndpoint endpoint |
133-
this.getParent+() = endpoint.getBody().asCallableAstNode() and
134-
this.getMethodName() = "params"
133+
// Simplified approach: find params calls that are descendants of Grape API class methods
134+
exists(GrapeAPIClass api |
135+
this.getMethodName() = "params" and
136+
this.getParent+() = api.getADeclaration()
135137
)
136-
or
137-
// Also handle cases where params is called on an instance of a Grape API class
138-
this = grapeAPIInstance().getAMethodCall("params").asExpr().getExpr()
139138
}
140-
}
141-
142-
/**
139+
}/**
143140
* A call to `headers` from within a Grape API endpoint.
144141
* Headers can also be a source of user input.
145142
*/
@@ -195,4 +192,44 @@ private class GrapeRequestCall extends MethodCall {
195192
// Also handle cases where request is called on an instance of a Grape API class
196193
this = grapeAPIInstance().getAMethodCall("request").asExpr().getExpr()
197194
}
195+
}
196+
197+
/**
198+
* A method defined within a `helpers` block in a Grape API class.
199+
* These methods become available in endpoint contexts through Grape's DSL.
200+
*/
201+
private class GrapeHelperMethod extends Method {
202+
private GrapeAPIClass apiClass;
203+
204+
GrapeHelperMethod() {
205+
exists(DataFlow::CallNode helpersCall |
206+
helpersCall = apiClass.getAModuleLevelCall("helpers") and
207+
this.getParent+() = helpersCall.getBlock().asExpr().getExpr()
208+
)
209+
}
210+
211+
/**
212+
* Gets the API class that contains this helper method.
213+
*/
214+
GrapeAPIClass getAPIClass() { result = apiClass }
215+
}
216+
217+
/**
218+
* Additional taint step to model dataflow from method arguments to parameters
219+
* for Grape helper methods defined in `helpers` blocks.
220+
* This bridges the gap where standard dataflow doesn't recognize the Grape DSL semantics.
221+
*/
222+
private class GrapeHelperMethodTaintStep extends AdditionalTaintStep {
223+
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
224+
exists(GrapeHelperMethod helperMethod, MethodCall call, int i |
225+
// Find calls to helper methods from within Grape endpoints
226+
call.getMethodName() = helperMethod.getName() and
227+
exists(GrapeEndpoint endpoint |
228+
call.getParent+() = endpoint.getBody().asExpr().getExpr()
229+
) and
230+
// Map argument to parameter
231+
nodeFrom.asExpr().getExpr() = call.getArgument(i) and
232+
nodeTo.asParameter() = helperMethod.getParameter(i)
233+
)
234+
}
198235
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,4 @@ grapeHeaders
2222
| app.rb:9:18:9:24 | call to headers |
2323
| app.rb:46:5:46:11 | call to headers |
2424
grapeRequest
25-
| app.rb:25:12:25:18 | call to request |
25+
| app.rb:25:12:25:18 | call to request |

ruby/ql/test/query-tests/security/cwe-089/ArelInjection.rb

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,27 @@ class PotatoAPI < Grape::API
1515
sql = Arel.sql("SELECT * FROM users WHERE name = #{name}")
1616
sql = Arel::Nodes::SqlLiteral.new("SELECT * FROM users WHERE name = #{name}")
1717
end
18-
end
18+
end
19+
20+
class SimpleAPI < Grape::API
21+
get '/test' do
22+
x = params[:name]
23+
Arel.sql("SELECT * FROM users WHERE name = #{x}")
24+
end
25+
end
26+
27+
# Test helper method pattern in Grape helpers block
28+
class TestAPI < Grape::API
29+
helpers do
30+
def vulnerable_helper(user_id)
31+
# BAD: SQL statement constructed from user input passed as parameter
32+
Arel.sql("SELECT * FROM users WHERE id = #{user_id}")
33+
end
34+
end
35+
36+
get '/helper_test' do
37+
# This should be detected as SQL injection via helper method
38+
user_id = params[:user_id]
39+
vulnerable_helper(user_id)
40+
end
41+
end

ruby/ql/test/query-tests/security/cwe-089/SqlInjection.expected

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,14 @@ edges
8585
| ArelInjection.rb:13:5:13:8 | name | ArelInjection.rb:16:39:16:80 | "SELECT * FROM users WHERE nam..." | provenance | AdditionalTaintStep |
8686
| ArelInjection.rb:13:12:13:17 | call to params | ArelInjection.rb:13:12:13:29 | ...[...] | provenance | |
8787
| ArelInjection.rb:13:12:13:29 | ...[...] | ArelInjection.rb:13:5:13:8 | name | provenance | |
88+
| ArelInjection.rb:22:5:22:5 | x | ArelInjection.rb:23:14:23:52 | "SELECT * FROM users WHERE nam..." | provenance | AdditionalTaintStep |
89+
| ArelInjection.rb:22:9:22:14 | call to params | ArelInjection.rb:22:9:22:21 | ...[...] | provenance | |
90+
| ArelInjection.rb:22:9:22:21 | ...[...] | ArelInjection.rb:22:5:22:5 | x | provenance | |
91+
| ArelInjection.rb:30:29:30:35 | user_id | ArelInjection.rb:32:18:32:60 | "SELECT * FROM users WHERE id ..." | provenance | AdditionalTaintStep |
92+
| ArelInjection.rb:38:7:38:13 | user_id | ArelInjection.rb:39:25:39:31 | user_id | provenance | |
93+
| ArelInjection.rb:38:17:38:22 | call to params | ArelInjection.rb:38:17:38:32 | ...[...] | provenance | |
94+
| ArelInjection.rb:38:17:38:32 | ...[...] | ArelInjection.rb:38:7:38:13 | user_id | provenance | |
95+
| ArelInjection.rb:39:25:39:31 | user_id | ArelInjection.rb:30:29:30:35 | user_id | provenance | AdditionalTaintStep |
8896
| PgInjection.rb:6:5:6:8 | name | PgInjection.rb:13:5:13:8 | qry1 : String | provenance | AdditionalTaintStep |
8997
| PgInjection.rb:6:5:6:8 | name | PgInjection.rb:19:5:19:8 | qry2 : String | provenance | AdditionalTaintStep |
9098
| PgInjection.rb:6:5:6:8 | name | PgInjection.rb:31:5:31:8 | qry3 : String | provenance | AdditionalTaintStep |
@@ -218,6 +226,16 @@ nodes
218226
| ArelInjection.rb:13:12:13:29 | ...[...] | semmle.label | ...[...] |
219227
| ArelInjection.rb:15:20:15:61 | "SELECT * FROM users WHERE nam..." | semmle.label | "SELECT * FROM users WHERE nam..." |
220228
| ArelInjection.rb:16:39:16:80 | "SELECT * FROM users WHERE nam..." | semmle.label | "SELECT * FROM users WHERE nam..." |
229+
| ArelInjection.rb:22:5:22:5 | x | semmle.label | x |
230+
| ArelInjection.rb:22:9:22:14 | call to params | semmle.label | call to params |
231+
| ArelInjection.rb:22:9:22:21 | ...[...] | semmle.label | ...[...] |
232+
| ArelInjection.rb:23:14:23:52 | "SELECT * FROM users WHERE nam..." | semmle.label | "SELECT * FROM users WHERE nam..." |
233+
| ArelInjection.rb:30:29:30:35 | user_id | semmle.label | user_id |
234+
| ArelInjection.rb:32:18:32:60 | "SELECT * FROM users WHERE id ..." | semmle.label | "SELECT * FROM users WHERE id ..." |
235+
| ArelInjection.rb:38:7:38:13 | user_id | semmle.label | user_id |
236+
| ArelInjection.rb:38:17:38:22 | call to params | semmle.label | call to params |
237+
| ArelInjection.rb:38:17:38:32 | ...[...] | semmle.label | ...[...] |
238+
| ArelInjection.rb:39:25:39:31 | user_id | semmle.label | user_id |
221239
| PgInjection.rb:6:5:6:8 | name | semmle.label | name |
222240
| PgInjection.rb:6:12:6:17 | call to params | semmle.label | call to params |
223241
| PgInjection.rb:6:12:6:24 | ...[...] | semmle.label | ...[...] |
@@ -277,6 +295,8 @@ subpaths
277295
| 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 |
278296
| 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 |
279297
| 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 |
298+
| 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 |
299+
| 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 |
280300
| 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 |
281301
| 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 |
282302
| 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 |

0 commit comments

Comments
 (0)