Skip to content

Commit 73ca595

Browse files
hmachmac
committed
Ruby: Model ActionDispatch::Response
1 parent ae0c9b7 commit 73ca595

File tree

7 files changed

+183
-7
lines changed

7 files changed

+183
-7
lines changed

‎ruby/ql/lib/codeql/ruby/Concepts.qll

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,37 @@ module Http {
467467
override RequestInputKind getKind() { result = parameterInputKind() }
468468
}
469469

470+
/**
471+
* A data flow node that writes data to a header in a HTTP response.
472+
*
473+
* Extend this class to refine existing API models. If you want to model new APIs,
474+
* extend `HeaderWriteAccess::Range` instead.
475+
*/
476+
class HeaderWriteAccess extends DataFlow::Node instanceof HeaderWriteAccess::Range {
477+
/** Gets the name of the header that is written to. */
478+
string getName() { result = super.getName() }
479+
480+
/** Gets the value that is written to the header. */
481+
DataFlow::Node getValue() { result = super.getValue() }
482+
}
483+
484+
/** Provides a class for modeling new HTTP header writes. */
485+
module HeaderWriteAccess {
486+
/**
487+
* A data flow node that writes data to the a header in a HTTP response.
488+
*
489+
* Extend this class to model new APIs. If you want to refine existing API models,
490+
* extend `HeaderWriteAccess` instead.
491+
*/
492+
abstract class Range extends DataFlow::Node {
493+
/** Gets the name of the header that is written to. */
494+
abstract string getName();
495+
496+
/** Gets the value that is written to the header. */
497+
abstract DataFlow::Node getValue();
498+
}
499+
}
500+
470501
/**
471502
* A data-flow node that creates a HTTP response on a server.
472503
*

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

Lines changed: 96 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -516,8 +516,11 @@ private class ActionControllerProtectFromForgeryCall extends CsrfProtectionSetti
516516
*/
517517
private class SendFile extends FileSystemAccess::Range, DataFlow::CallNode {
518518
SendFile() {
519-
this.asExpr().getExpr() instanceof ActionControllerContextCall and
520-
this.getMethodName() = "send_file"
519+
this.getMethodName() = "send_file" and
520+
(
521+
this.asExpr().getExpr() instanceof ActionControllerContextCall or
522+
this.getReceiver().asExpr().getExpr() instanceof Response::ResponseCall
523+
)
521524
}
522525

523526
override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
@@ -642,3 +645,94 @@ private module ParamsSummaries {
642645
}
643646
}
644647
}
648+
649+
/**
650+
* Provides modeling for `ActionDispatch::Response`, which represents an HTTP
651+
* response.
652+
*/
653+
private module Response {
654+
class ResponseCall extends ActionControllerContextCall {
655+
ResponseCall() { this.getMethodName() = "response" }
656+
}
657+
658+
class BodyWrite extends DataFlow::CallNode, Http::Server::HttpResponse::Range {
659+
BodyWrite() {
660+
this.getReceiver().asExpr().getExpr() instanceof ResponseCall and
661+
this.getMethodName() = "body="
662+
}
663+
664+
override DataFlow::Node getBody() { result = this.getArgument(0) }
665+
666+
override DataFlow::Node getMimetypeOrContentTypeArg() { none() }
667+
668+
override string getMimetypeDefault() { result = "text/http" }
669+
}
670+
671+
class SendFileCall extends DataFlow::CallNode, Http::Server::HttpResponse::Range {
672+
SendFileCall() {
673+
this.getReceiver().asExpr().getExpr() instanceof ResponseCall and
674+
this.getMethodName() = "send_file"
675+
}
676+
677+
override DataFlow::Node getBody() { result = this.getArgument(0) }
678+
679+
override DataFlow::Node getMimetypeOrContentTypeArg() { none() }
680+
681+
override string getMimetypeDefault() { result = "application/octet-stream" }
682+
}
683+
684+
class HeaderWrite extends DataFlow::CallNode, Http::Server::HeaderWriteAccess::Range {
685+
HeaderWrite() {
686+
// response.header[key] = val
687+
// response.headers[key] = val
688+
exists(MethodCall headerCall |
689+
headerCall.getMethodName() = ["header", "headers"] and
690+
headerCall.getReceiver() instanceof ResponseCall
691+
|
692+
this.getReceiver().asExpr().getExpr() = headerCall and
693+
this.getMethodName() = "[]="
694+
)
695+
or
696+
// response.set_header(key) = val
697+
// response[header] = val
698+
// response.add_header(key, val)
699+
this.getReceiver().asExpr().getExpr() instanceof ResponseCall and
700+
this.getMethodName() = ["set_header", "[]=", "add_header"]
701+
}
702+
703+
override string getName() {
704+
result = this.getArgument(0).asExpr().getConstantValue().getString()
705+
}
706+
707+
override DataFlow::Node getValue() { result = this.getArgument(1) }
708+
}
709+
710+
class SpecificHeaderWrite extends DataFlow::CallNode, Http::Server::HeaderWriteAccess::Range {
711+
SpecificHeaderWrite() {
712+
// response.<method> = val
713+
this.getReceiver().asExpr().getExpr() instanceof ResponseCall and
714+
this.getMethodName() =
715+
[
716+
"location=", "cache_control=", "_cache_control=", "etag=", "charset=", "content_type=",
717+
"date=", "last_modified=", "weak_etag=", "strong_etag="
718+
]
719+
}
720+
721+
override string getName() {
722+
this.getMethodName() = "location=" and result = "location"
723+
or
724+
this.getMethodName() = ["_cache_control=", "cache_control="] and result = "cache-control"
725+
or
726+
this.getMethodName() = ["etag=", "weak_etag=", "strong_etag="] and result = "etag"
727+
or
728+
// sets the charset part of the content-type header
729+
this.getMethodName() = ["charset=", "content_type="] and result = "content-type"
730+
or
731+
this.getMethodName() = "date=" and result = "date"
732+
or
733+
this.getMethodName() = "last_modified=" and result = "last-modified"
734+
}
735+
736+
override DataFlow::Node getValue() { result = this.getArgument(0) }
737+
}
738+
}

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ actionControllerControllerClasses
66
| active_record/ActiveRecord.rb:66:1:98:3 | BazController |
77
| active_record/ActiveRecord.rb:100:1:108:3 | AnnotatedController |
88
| active_storage/active_storage.rb:39:1:45:3 | PostsController |
9-
| app/controllers/comments_controller.rb:1:1:14:3 | CommentsController |
9+
| app/controllers/comments_controller.rb:1:1:40:3 | CommentsController |
1010
| app/controllers/foo/bars_controller.rb:3:1:46:3 | BarsController |
1111
| app/controllers/photos_controller.rb:1:1:4:3 | PhotosController |
1212
| app/controllers/posts_controller.rb:1:1:10:3 | PostsController |
@@ -61,8 +61,8 @@ actionControllerActionMethods
6161
| active_record/ActiveRecord.rb:101:3:103:5 | index |
6262
| active_record/ActiveRecord.rb:105:3:107:5 | unsafe_action |
6363
| active_storage/active_storage.rb:40:3:44:5 | create |
64-
| app/controllers/comments_controller.rb:2:3:10:5 | index |
65-
| app/controllers/comments_controller.rb:12:3:13:5 | show |
64+
| app/controllers/comments_controller.rb:2:3:36:5 | index |
65+
| app/controllers/comments_controller.rb:38:3:39:5 | show |
6666
| app/controllers/foo/bars_controller.rb:5:3:7:5 | index |
6767
| app/controllers/foo/bars_controller.rb:9:3:18:5 | show_debug |
6868
| app/controllers/foo/bars_controller.rb:20:3:24:5 | show |
@@ -370,3 +370,19 @@ getAssociatedControllerClasses
370370
controllerTemplateFiles
371371
| 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 |
372372
| 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 |
373+
headerWriteAccesses
374+
| app/controllers/comments_controller.rb:15:5:15:35 | call to []= | Content-Type | app/controllers/comments_controller.rb:15:39:15:49 | ... = ... |
375+
| app/controllers/comments_controller.rb:16:5:16:46 | call to set_header | Content-Length | app/controllers/comments_controller.rb:16:43:16:45 | 100 |
376+
| app/controllers/comments_controller.rb:17:5:17:39 | call to []= | X-Custom-Header | app/controllers/comments_controller.rb:17:43:17:46 | ... = ... |
377+
| app/controllers/comments_controller.rb:18:5:18:39 | call to []= | X-Another-Custom-Header | app/controllers/comments_controller.rb:18:43:18:47 | ... = ... |
378+
| app/controllers/comments_controller.rb:19:5:19:49 | call to add_header | X-Yet-Another | app/controllers/comments_controller.rb:19:42:19:49 | "indeed" |
379+
| app/controllers/comments_controller.rb:25:5:25:21 | call to location= | location | app/controllers/comments_controller.rb:25:25:25:36 | ... = ... |
380+
| app/controllers/comments_controller.rb:26:5:26:26 | call to cache_control= | cache-control | app/controllers/comments_controller.rb:26:30:26:36 | ... = ... |
381+
| app/controllers/comments_controller.rb:27:5:27:27 | call to _cache_control= | cache-control | app/controllers/comments_controller.rb:27:31:27:37 | ... = ... |
382+
| app/controllers/comments_controller.rb:28:5:28:17 | call to etag= | etag | app/controllers/comments_controller.rb:28:21:28:27 | ... = ... |
383+
| app/controllers/comments_controller.rb:29:5:29:20 | call to charset= | content-type | app/controllers/comments_controller.rb:29:24:29:30 | ... = ... |
384+
| app/controllers/comments_controller.rb:30:5:30:25 | call to content_type= | content-type | app/controllers/comments_controller.rb:30:29:30:35 | ... = ... |
385+
| app/controllers/comments_controller.rb:32:5:32:17 | call to date= | date | app/controllers/comments_controller.rb:32:21:32:30 | ... = ... |
386+
| app/controllers/comments_controller.rb:33:5:33:26 | call to last_modified= | last-modified | app/controllers/comments_controller.rb:33:30:33:43 | ... = ... |
387+
| app/controllers/comments_controller.rb:34:5:34:22 | call to weak_etag= | etag | app/controllers/comments_controller.rb:34:26:34:32 | ... = ... |
388+
| app/controllers/comments_controller.rb:35:5:35:24 | call to strong_etag= | etag | app/controllers/comments_controller.rb:35:28:35:34 | ... = ... |

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ private import codeql.ruby.frameworks.ActionController
33
private import codeql.ruby.frameworks.Rails
44
private import codeql.ruby.frameworks.ActionView
55
private import codeql.ruby.Concepts
6+
private import codeql.ruby.DataFlow
67

78
query predicate actionControllerControllerClasses(ActionControllerControllerClass cls) { any() }
89

@@ -31,3 +32,9 @@ query predicate getAssociatedControllerClasses(ActionControllerControllerClass c
3132
query predicate controllerTemplateFiles(ActionControllerControllerClass cls, ErbFile templateFile) {
3233
controllerTemplateFile(cls, templateFile)
3334
}
35+
36+
query predicate headerWriteAccesses(
37+
Http::Server::HeaderWriteAccess a, string name, DataFlow::Node value
38+
) {
39+
name = a.getName() and value = a.getValue()
40+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ actionDispatchRoutes
3636
actionDispatchControllerMethods
3737
| app/config/routes.rb:2:3:8:5 | call to resources | app/controllers/posts_controller.rb:2:3:3:5 | index |
3838
| app/config/routes.rb:2:3:8:5 | call to resources | app/controllers/posts_controller.rb:5:3:6:5 | show |
39-
| app/config/routes.rb:3:5:6:7 | call to resources | app/controllers/comments_controller.rb:2:3:10:5 | index |
40-
| app/config/routes.rb:3:5:6:7 | call to resources | app/controllers/comments_controller.rb:12:3:13:5 | show |
39+
| app/config/routes.rb:3:5:6:7 | call to resources | app/controllers/comments_controller.rb:2:3:36:5 | index |
40+
| app/config/routes.rb:3:5:6:7 | call to resources | app/controllers/comments_controller.rb:38:3:39:5 | show |
4141
| app/config/routes.rb:7:5:7:37 | call to post | app/controllers/posts_controller.rb:8:3:9:5 | upvote |
4242
| app/config/routes.rb:27:3:27:48 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
4343
| app/config/routes.rb:28:3:28:50 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ renderToCalls
2424
linkToCalls
2525
| app/views/foo/bars/show.html.erb:33:5:33:41 | call to link_to |
2626
httpResponses
27+
| app/controllers/comments_controller.rb:11:5:11:17 | call to body= | app/controllers/comments_controller.rb:11:21:11:34 | ... = ... | text/http |
28+
| app/controllers/comments_controller.rb:21:5:21:37 | call to send_file | app/controllers/comments_controller.rb:21:24:21:36 | "my-file.ext" | application/octet-stream |
2729
| 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 |
2830
| 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 |
2931
| 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 |

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,32 @@ def index
77
request.query_parameters
88
request.request_parameters
99
request.filtered_parameters
10+
11+
response.body = "some content"
12+
13+
response.status = 200
14+
15+
response.header["Content-Type"] = "text/html"
16+
response.set_header("Content-Length", 100)
17+
response.headers["X-Custom-Header"] = "hi"
18+
response["X-Another-Custom-Header"] = "yes"
19+
response.add_header "X-Yet-Another", "indeed"
20+
21+
response.send_file("my-file.ext")
22+
23+
response.request
24+
25+
response.location = "http://..." # relevant for url redirect query
26+
response.cache_control = "value"
27+
response._cache_control = "value"
28+
response.etag = "value"
29+
response.charset = "value" # sets the charset part of the content-type header
30+
response.content_type = "value" # sets the main part of the content-type header
31+
32+
response.date = Date.today
33+
response.last_modified = Date.yesterday
34+
response.weak_etag = "value"
35+
response.strong_etag = "value"
1036
end
1137

1238
def show

0 commit comments

Comments
 (0)