From 82fc62ca71fb37eec456d65dc1cb40f3d9151b9a Mon Sep 17 00:00:00 2001 From: Aaron Lahey Date: Sat, 28 Aug 2021 11:41:08 -0500 Subject: [PATCH] Use static message when raising HTTP request parameter parse errors When parsing HTTP request parameters, Rails delegates to a set of parsing strategies based on the MIME type. If any of these strategies raises an error Rails rescues it and raises an instance of `ActionDispatch::Http::Parameters::ParseError` with the same message as the underlying error. However, in the presence of malformed JSON, the default parameter parser for the `application/json` MIME type raises a `JSON:ParserError` with a message containing the entire malformed JSON string (the request body in this context). By raising a new error with this same message Rails inadvertently ends up logging the full HTTP request body at the `fatal` level. This request body could contain sensitive information or could be intentionally crafted to be extremely large. This commit sets the `ActionDispatch::Http::Parameters::ParseError` message to a static message which mirrors that of the corresponding `debug` log. --- actionpack/CHANGELOG.md | 8 ++++++++ actionpack/lib/action_dispatch/http/parameters.rb | 6 +++--- .../test/dispatch/request/json_params_parsing_test.rb | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index bd22674d49..f2ca11c88f 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,11 @@ +* Use a static error message when raising `ActionDispatch::Http::Parameters::ParseError` + to avoid inadvertently logging the HTTP request body at the `fatal` level when it contains + malformed JSON. + + Fixes #41145 + + *Aaron Lahey* + * Add `Middleware#delete!` to delete middleware or raise if not found. `Middleware#delete!` works just like `Middleware#delete` but will diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 1cc3abdd99..d2aaf7c936 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -17,8 +17,8 @@ module ActionDispatch # Raised when raw data from the request cannot be parsed by the parser # defined for request's content MIME type. class ParseError < StandardError - def initialize - super($!.message) + def initialize(message = $!.message) + super(message) end end @@ -93,7 +93,7 @@ module ActionDispatch strategy.call(raw_post) rescue # JSON or Ruby code block errors. log_parse_error_once - raise ParseError + raise ParseError, "Error occurred while parsing request parameters" end end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index bbf98912f3..7dc467e36d 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -80,7 +80,7 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest post "/parse", params: json, headers: { "CONTENT_TYPE" => "application/json", "action_dispatch.show_exceptions" => false } end assert_equal JSON::ParserError, exception.cause.class - assert_equal exception.cause.message, exception.message + assert_equal "Error occurred while parsing request parameters", exception.message ensure $stderr = STDERR end