Skip to content

Commit 7ef6ffb

Browse files
hmachmac
committed
Ruby: Recognise Rails render calls as HTTP responses
1 parent 79bae0c commit 7ef6ffb

File tree

7 files changed

+92
-15
lines changed

7 files changed

+92
-15
lines changed

‎ruby/ql/lib/change-notes/2022-08-16-action-controller-response-body.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Calls to `render` in Rails controllers and views are now recognised as HTTP
5+
response bodies.

‎ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,26 +149,26 @@ class CookiesSource extends HTTP::Server::RequestInputAccess::Range {
149149
override string getSourceType() { result = "ActionController::Metal#cookies" }
150150
}
151151

152-
// A call to `cookies` from within a controller.
152+
/** A call to `cookies` from within a controller. */
153153
private class ActionControllerCookiesCall extends ActionControllerContextCall, CookiesCall { }
154154

155-
// A call to `params` from within a controller.
155+
/** A call to `params` from within a controller. */
156156
private class ActionControllerParamsCall extends ActionControllerContextCall, ParamsCall { }
157157

158-
// A call to `render` from within a controller.
158+
/** A call to `render` from within a controller. */
159159
private class ActionControllerRenderCall extends ActionControllerContextCall, RenderCall { }
160160

161-
// A call to `render_to` from within a controller.
161+
/** A call to `render_to` from within a controller. */
162162
private class ActionControllerRenderToCall extends ActionControllerContextCall, RenderToCall { }
163163

164-
// A call to `html_safe` from within a controller.
164+
/** A call to `html_safe` from within a controller. */
165165
private class ActionControllerHtmlSafeCall extends HtmlSafeCall {
166166
ActionControllerHtmlSafeCall() {
167167
this.getEnclosingModule() instanceof ActionControllerControllerClass
168168
}
169169
}
170170

171-
// A call to `html_escape` from within a controller.
171+
/** A call to `html_escape` from within a controller. */
172172
private class ActionControllerHtmlEscapeCall extends HtmlEscapeCall {
173173
ActionControllerHtmlEscapeCall() {
174174
this.getEnclosingModule() instanceof ActionControllerControllerClass

‎ruby/ql/lib/codeql/ruby/frameworks/ActionView.qll

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,55 @@ abstract class RenderCall extends MethodCall {
123123
// TODO: implicit renders in controller actions
124124
}
125125

126-
// A call to the `render` method within the context of a template.
126+
/**
127+
* A call to `render`, `render_to_body` or `render_to_string`, seen as an
128+
* `HttpResponse`.
129+
*/
130+
private class RenderCallAsHttpResponse extends DataFlow::CallNode, HTTP::Server::HttpResponse::Range {
131+
RenderCallAsHttpResponse() {
132+
this.asExpr().getExpr() instanceof RenderCall or
133+
this.asExpr().getExpr() instanceof RenderToCall
134+
}
135+
136+
// `render` is a very polymorphic method - all of these are valid calls:
137+
// render @user
138+
// render "path/to/template"
139+
// render html: "<html></html>"
140+
// render json: { "some" => "hash" }
141+
// render body: "some text"
142+
override DataFlow::Node getBody() {
143+
// A positional argument, e.g.
144+
// render @user
145+
// render "path/to/template"
146+
result = this.getArgument(_) and
147+
not result.asExpr() instanceof ExprNodes::PairCfgNode
148+
or
149+
result = this.getKeywordArgument(["html", "json", "body", "inline", "plain", "js", "file"])
150+
}
151+
152+
override DataFlow::Node getMimetypeOrContentTypeArg() {
153+
result = this.getKeywordArgument("content_type")
154+
}
155+
156+
override string getMimetype() {
157+
exists(this.getKeywordArgument("json")) and result = "application/json"
158+
or
159+
exists(this.getKeywordArgument("plain")) and result = "text/plain"
160+
or
161+
exists(this.getKeywordArgument("html")) and result = "text/html"
162+
or
163+
exists(this.getKeywordArgument("xml")) and result = "application/xml"
164+
or
165+
exists(this.getKeywordArgument("js")) and result = "text/javascript"
166+
or
167+
not exists(this.getKeywordArgument(["json", "plain", "html", "xml", "js"])) and
168+
result = super.getMimetype()
169+
}
170+
171+
override string getMimetypeDefault() { result = "text/html" }
172+
}
173+
174+
/** A call to the `render` method within the context of a template. */
127175
private class ActionViewRenderCall extends RenderCall, ActionViewContextCall { }
128176

129177
/**

‎ruby/ql/test/library-tests/frameworks/ActionController.expected

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ actionControllerControllerClasses
44
| active_record/ActiveRecord.rb:66:1:94:3 | BazController |
55
| active_record/ActiveRecord.rb:96:1:104:3 | AnnotatedController |
66
| app/controllers/comments_controller.rb:1:1:7:3 | CommentsController |
7-
| app/controllers/foo/bars_controller.rb:3:1:39:3 | BarsController |
7+
| app/controllers/foo/bars_controller.rb:3:1:46:3 | BarsController |
88
| app/controllers/photos_controller.rb:1:1:4:3 | PhotosController |
99
| app/controllers/posts_controller.rb:1:1:10:3 | PostsController |
1010
| app/controllers/users/notifications_controller.rb:2:3:5:5 | NotificationsController |
@@ -28,6 +28,7 @@ actionControllerActionMethods
2828
| app/controllers/foo/bars_controller.rb:20:3:24:5 | show |
2929
| app/controllers/foo/bars_controller.rb:26:3:28:5 | go_back |
3030
| app/controllers/foo/bars_controller.rb:30:3:32:5 | go_back_2 |
31+
| app/controllers/foo/bars_controller.rb:34:3:39:5 | show_2 |
3132
| app/controllers/photos_controller.rb:2:3:3:5 | show |
3233
| app/controllers/posts_controller.rb:2:3:3:5 | index |
3334
| app/controllers/posts_controller.rb:5:3:6:5 | show |
@@ -103,8 +104,8 @@ redirectToCalls
103104
| app/controllers/foo/bars_controller.rb:31:5:31:56 | call to redirect_back |
104105
actionControllerHelperMethods
105106
getAssociatedControllerClasses
106-
| app/controllers/foo/bars_controller.rb:3:1:39:3 | BarsController | app/views/foo/bars/_widget.html.erb:0:0:0:0 | app/views/foo/bars/_widget.html.erb |
107-
| app/controllers/foo/bars_controller.rb:3:1:39:3 | BarsController | app/views/foo/bars/show.html.erb:0:0:0:0 | app/views/foo/bars/show.html.erb |
107+
| app/controllers/foo/bars_controller.rb:3:1:46:3 | BarsController | app/views/foo/bars/_widget.html.erb:0:0:0:0 | app/views/foo/bars/_widget.html.erb |
108+
| app/controllers/foo/bars_controller.rb:3:1:46:3 | BarsController | app/views/foo/bars/show.html.erb:0:0:0:0 | app/views/foo/bars/show.html.erb |
108109
controllerTemplateFiles
109-
| app/controllers/foo/bars_controller.rb:3:1:39:3 | BarsController | app/views/foo/bars/_widget.html.erb:0:0:0:0 | app/views/foo/bars/_widget.html.erb |
110-
| app/controllers/foo/bars_controller.rb:3:1:39:3 | BarsController | app/views/foo/bars/show.html.erb:0:0:0:0 | app/views/foo/bars/show.html.erb |
110+
| app/controllers/foo/bars_controller.rb:3:1:46:3 | BarsController | app/views/foo/bars/_widget.html.erb:0:0:0:0 | app/views/foo/bars/_widget.html.erb |
111+
| app/controllers/foo/bars_controller.rb:3:1:46:3 | BarsController | app/views/foo/bars/show.html.erb:0:0:0:0 | app/views/foo/bars/show.html.erb |

‎ruby/ql/test/library-tests/frameworks/ActionView.expected

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,19 @@ rawCalls
1414
renderCalls
1515
| app/controllers/foo/bars_controller.rb:6:5:6:37 | call to render |
1616
| app/controllers/foo/bars_controller.rb:23:5:23:76 | call to render |
17-
| app/controllers/foo/bars_controller.rb:37:5:37:17 | call to render |
17+
| app/controllers/foo/bars_controller.rb:35:5:35:33 | call to render |
18+
| app/controllers/foo/bars_controller.rb:38:5:38:50 | call to render |
19+
| app/controllers/foo/bars_controller.rb:44:5:44:17 | call to render |
1820
| app/views/foo/bars/show.html.erb:31:5:31:89 | call to render |
1921
renderToCalls
2022
| app/controllers/foo/bars_controller.rb:15:16:15:97 | call to render_to_string |
23+
| app/controllers/foo/bars_controller.rb:36:12:36:67 | call to render_to_string |
2124
linkToCalls
2225
| app/views/foo/bars/show.html.erb:33:5:33:41 | call to link_to |
26+
httpResponses
27+
| app/controllers/foo/bars_controller.rb:15:16:15:97 | call to render_to_string | app/controllers/foo/bars_controller.rb:15:33:15:47 | "foo/bars/show" | text/html |
28+
| app/controllers/foo/bars_controller.rb:23:5:23:76 | call to render | app/controllers/foo/bars_controller.rb:23:12:23:26 | "foo/bars/show" | text/html |
29+
| app/controllers/foo/bars_controller.rb:35:5:35:33 | call to render | app/controllers/foo/bars_controller.rb:35:18:35:33 | call to [] | application/json |
30+
| app/controllers/foo/bars_controller.rb:36:12:36:67 | call to render_to_string | app/controllers/foo/bars_controller.rb:36:29:36:33 | @user | application/json |
31+
| app/controllers/foo/bars_controller.rb:38:5:38:50 | call to render | app/controllers/foo/bars_controller.rb:38:12:38:22 | call to backtrace | text/plain |
32+
| app/controllers/foo/bars_controller.rb:44:5:44:17 | call to render | app/controllers/foo/bars_controller.rb:44:12:44:17 | "show" | text/html |

‎ruby/ql/test/library-tests/frameworks/ActionView.ql

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
import codeql.ruby.frameworks.ActionController
2-
import codeql.ruby.frameworks.ActionView
1+
private import codeql.ruby.frameworks.ActionController
2+
private import codeql.ruby.frameworks.ActionView
3+
private import codeql.ruby.Concepts
4+
private import codeql.ruby.DataFlow
35

46
query predicate htmlSafeCalls(HtmlSafeCall c) { any() }
57

@@ -10,3 +12,7 @@ query predicate renderCalls(RenderCall c) { any() }
1012
query predicate renderToCalls(RenderToCall c) { any() }
1113

1214
query predicate linkToCalls(LinkToCall c) { any() }
15+
16+
query predicate httpResponses(HTTP::Server::HttpResponse r, DataFlow::Node body, string mimeType) {
17+
r.getBody() = body and r.getMimetype() = mimeType
18+
}

‎ruby/ql/test/library-tests/frameworks/app/controllers/foo/bars_controller.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ def go_back_2
3131
redirect_back fallback_location: { action: "index" }
3232
end
3333

34+
def show_2
35+
render json: { some: "data" }
36+
body = render_to_string @user, content_type: "application/json"
37+
rescue => e
38+
render e.backtrace, content_type: "text/plain"
39+
end
40+
3441
private
3542

3643
def unreachable_action

0 commit comments

Comments
 (0)