From e6dc27a7b55a975735f2fad5c198318f638c7415 Mon Sep 17 00:00:00 2001 From: Harry Maclean Date: Fri, 14 Oct 2022 18:55:05 +1300 Subject: [PATCH] Add content_mime_type, fix env/filtered_env --- .../ruby/frameworks/ActionController.qll | 27 +++++++--- .../frameworks/ActionController.expected | 41 +++++++++++++++ .../action_controller/input_access.rb | 50 +++++++++++++++++++ 3 files changed, 111 insertions(+), 7 deletions(-) create mode 100644 ruby/ql/test/library-tests/frameworks/action_controller/input_access.rb diff --git a/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll b/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll index 60eaa62c211..2f2829773d3 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll @@ -226,7 +226,7 @@ private module Request { [ "authorization", "script_name", "path_info", "user_agent", "referer", "referrer", "host_authority", "content_type", "host", "hostname", "accept_encoding", - "accept_language", "if_none_match", "if_none_match_etags" + "accept_language", "if_none_match", "if_none_match_etags", "content_mime_type" ] or // Request headers are prefixed with `HTTP_` to distinguish them from @@ -261,8 +261,7 @@ private module Request { */ private class HeaderTaintedCall extends RequestInputAccess { HeaderTaintedCall() { - this.getMethodName() = - ["media_type", "media_type", "media_type_params", "content_charset", "base_url"] + this.getMethodName() = ["media_type", "media_type_params", "content_charset", "base_url"] } override Http::Server::RequestInputKind getKind() { result = Http::Server::headerInputKind() } @@ -275,14 +274,28 @@ private module Request { override Http::Server::RequestInputKind getKind() { result = Http::Server::bodyInputKind() } } - /** A method call on `request` which returns the rack env. */ - private class EnvCall extends RequestInputAccess { - EnvCall() { - this.getMethodName() = ["env", "filtered_env"] and + /** + * A method call on `request` which returns the rack env. + * This is a hash containing all the information about the request. Values + * under keys starting with `HTTP_` are user-controlled. + */ + private class EnvCall extends RequestMethodCall { + EnvCall() { this.getMethodName() = ["env", "filtered_env"] } + } + + /** + * A read of a user-controlled parameter from the request env. + */ + private class EnvHttpAccess extends DataFlow::CallNode, Http::Server::RequestInputAccess::Range { + EnvHttpAccess() { + any(EnvCall c).(DataFlow::LocalSourceNode).flowsTo(this.getReceiver()) and + this.getMethodName() = "[]" and this.getArgument(0).asExpr().getExpr().getConstantValue().getString().regexpMatch("^HTTP_.+") } override Http::Server::RequestInputKind getKind() { result = Http::Server::headerInputKind() } + + override string getSourceType() { result = "ActionDispatch::Request#env[]" } } } diff --git a/ruby/ql/test/library-tests/frameworks/ActionController.expected b/ruby/ql/test/library-tests/frameworks/ActionController.expected index f5d8828fdc0..b1351bf76c8 100644 --- a/ruby/ql/test/library-tests/frameworks/ActionController.expected +++ b/ruby/ql/test/library-tests/frameworks/ActionController.expected @@ -1,4 +1,5 @@ actionControllerControllerClasses +| action_controller/input_access.rb:1:1:50:3 | UsersController | | action_controller/params_flow.rb:1:1:151:3 | MyController | | active_record/ActiveRecord.rb:23:1:39:3 | FooController | | active_record/ActiveRecord.rb:41:1:64:3 | BarController | @@ -12,6 +13,7 @@ actionControllerControllerClasses | app/controllers/tags_controller.rb:1:1:2:3 | TagsController | | app/controllers/users/notifications_controller.rb:2:3:5:5 | NotificationsController | actionControllerActionMethods +| action_controller/input_access.rb:2:3:49:5 | index | | action_controller/params_flow.rb:2:3:4:5 | m1 | | action_controller/params_flow.rb:6:3:8:5 | m2 | | action_controller/params_flow.rb:10:3:12:5 | m2 | @@ -223,6 +225,45 @@ paramsSources | app/controllers/foo/bars_controller.rb:22:10:22:15 | call to params | | app/views/foo/bars/show.html.erb:5:9:5:14 | call to params | httpInputAccesses +| action_controller/input_access.rb:3:5:3:18 | call to params | ActionDispatch::Request#params | +| action_controller/input_access.rb:4:5:4:22 | call to parameters | ActionDispatch::Request#parameters | +| action_controller/input_access.rb:5:5:5:15 | call to GET | ActionDispatch::Request#GET | +| action_controller/input_access.rb:6:5:6:16 | call to POST | ActionDispatch::Request#POST | +| action_controller/input_access.rb:7:5:7:28 | call to query_parameters | ActionDispatch::Request#query_parameters | +| action_controller/input_access.rb:8:5:8:30 | call to request_parameters | ActionDispatch::Request#request_parameters | +| action_controller/input_access.rb:9:5:9:31 | call to filtered_parameters | ActionDispatch::Request#filtered_parameters | +| action_controller/input_access.rb:11:5:11:25 | call to authorization | ActionDispatch::Request#authorization | +| action_controller/input_access.rb:12:5:12:23 | call to script_name | ActionDispatch::Request#script_name | +| action_controller/input_access.rb:13:5:13:21 | call to path_info | ActionDispatch::Request#path_info | +| action_controller/input_access.rb:14:5:14:22 | call to user_agent | ActionDispatch::Request#user_agent | +| action_controller/input_access.rb:15:5:15:19 | call to referer | ActionDispatch::Request#referer | +| action_controller/input_access.rb:16:5:16:20 | call to referrer | ActionDispatch::Request#referrer | +| action_controller/input_access.rb:17:5:17:26 | call to host_authority | ActionDispatch::Request#host_authority | +| action_controller/input_access.rb:18:5:18:24 | call to content_type | ActionDispatch::Request#content_type | +| action_controller/input_access.rb:19:5:19:16 | call to host | ActionDispatch::Request#host | +| action_controller/input_access.rb:20:5:20:20 | call to hostname | ActionDispatch::Request#hostname | +| action_controller/input_access.rb:21:5:21:27 | call to accept_encoding | ActionDispatch::Request#accept_encoding | +| action_controller/input_access.rb:22:5:22:27 | call to accept_language | ActionDispatch::Request#accept_language | +| action_controller/input_access.rb:23:5:23:25 | call to if_none_match | ActionDispatch::Request#if_none_match | +| action_controller/input_access.rb:24:5:24:31 | call to if_none_match_etags | ActionDispatch::Request#if_none_match_etags | +| action_controller/input_access.rb:25:5:25:29 | call to content_mime_type | ActionDispatch::Request#content_mime_type | +| action_controller/input_access.rb:27:5:27:21 | call to authority | ActionDispatch::Request#authority | +| action_controller/input_access.rb:28:5:28:16 | call to host | ActionDispatch::Request#host | +| action_controller/input_access.rb:29:5:29:26 | call to host_authority | ActionDispatch::Request#host_authority | +| action_controller/input_access.rb:30:5:30:26 | call to host_with_port | ActionDispatch::Request#host_with_port | +| action_controller/input_access.rb:31:5:31:20 | call to hostname | ActionDispatch::Request#hostname | +| action_controller/input_access.rb:32:5:32:25 | call to forwarded_for | ActionDispatch::Request#forwarded_for | +| action_controller/input_access.rb:33:5:33:26 | call to forwarded_host | ActionDispatch::Request#forwarded_host | +| action_controller/input_access.rb:34:5:34:16 | call to port | ActionDispatch::Request#port | +| action_controller/input_access.rb:35:5:35:26 | call to forwarded_port | ActionDispatch::Request#forwarded_port | +| action_controller/input_access.rb:37:5:37:22 | call to media_type | ActionDispatch::Request#media_type | +| action_controller/input_access.rb:38:5:38:29 | call to media_type_params | ActionDispatch::Request#media_type_params | +| action_controller/input_access.rb:39:5:39:27 | call to content_charset | ActionDispatch::Request#content_charset | +| action_controller/input_access.rb:40:5:40:20 | call to base_url | ActionDispatch::Request#base_url | +| action_controller/input_access.rb:42:5:42:16 | call to body | ActionDispatch::Request#body | +| action_controller/input_access.rb:43:5:43:20 | call to raw_post | ActionDispatch::Request#raw_post | +| action_controller/input_access.rb:45:5:45:30 | ...[...] | ActionDispatch::Request#env[] | +| action_controller/input_access.rb:47:5:47:39 | ...[...] | ActionDispatch::Request#env[] | | action_controller/params_flow.rb:3:10:3:15 | call to params | ActionController::Metal#params | | action_controller/params_flow.rb:7:10:7:15 | call to params | ActionController::Metal#params | | action_controller/params_flow.rb:11:10:11:15 | call to params | ActionController::Metal#params | diff --git a/ruby/ql/test/library-tests/frameworks/action_controller/input_access.rb b/ruby/ql/test/library-tests/frameworks/action_controller/input_access.rb new file mode 100644 index 00000000000..334e36d3f3c --- /dev/null +++ b/ruby/ql/test/library-tests/frameworks/action_controller/input_access.rb @@ -0,0 +1,50 @@ +class UsersController < ActionController::Base + def index + request.params + request.parameters + request.GET + request.POST + request.query_parameters + request.request_parameters + request.filtered_parameters + + request.authorization + request.script_name + request.path_info + request.user_agent + request.referer + request.referrer + request.host_authority + request.content_type + request.host + request.hostname + request.accept_encoding + request.accept_language + request.if_none_match + request.if_none_match_etags + request.content_mime_type + + request.authority + request.host + request.host_authority + request.host_with_port + request.hostname + request.forwarded_for + request.forwarded_host + request.port + request.forwarded_port + + request.media_type + request.media_type_params + request.content_charset + request.base_url + + request.body + request.raw_post + + request.env["HTTP_ACCEPT"] + request.env["NOT_USER_CONTROLLED"] + request.filtered_env["HTTP_ACCEPT"] + request.filtered_env["NOT_USER_CONTROLLED"] + end +end