This project is archived and is in readonly mode.

#6244 ✓committed
Prem Sichanugrist (sikachu)

Filter sensitive `QUERY_STRING` parameters in the log

Reported by Prem Sichanugrist (sikachu) | January 1st, 2011 @ 05:13 PM | in 3.x

On a year before, I've submitted a patch which move Action Controller's filter_parameter_logging into ActionDispatch::Http::ParametersFilter.filter_parameters. (https://github.com/rails/rails/commit/bd4f21f) My intention of doing that was actually the fact that I want to move those logics out from Action Controller to be able to access it somewhere else.

So, this patch is actually a sequel of that previous patch.

In real world, I'm seeing a lot of applications which involves sending some kind of secret information such as api_key or token as a QUERY_STRING, or GET parameter more precisely. While setting config.filter_parameters already filtered out the parameters hash in the log, it didn't filtered out the request's path.

Consider this log, given I configured config.filter_parameters += [:password, :secret]:

Started GET "/posts?secret=abcd&user%5Bname%5D=sikachu" for 127.0.0.1 at Sat Jan 01 22:29:18 +0700 2011
  Processing by PostsController#index as HTML
  Parameters: {"user"=>{"name"=>"sikachu"}, "secret"=>"[FILTERED]"}
  Post Load (0.5ms)  SELECT "posts".* FROM "posts" 
Rendered posts/index.html.erb within layouts/application (3.2ms)
Completed 200 OK in 24ms (Views: 17.7ms | ActiveRecord: 0.5ms)

You can see that on the first line it's exposing the value of secret. So, in this patch I just take that QUERY_STRING and filtered them out, resulting in:

Started GET "/posts?secret=[FILTERED]&user%5Bname%5D=sikachu" for 127.0.0.1 at Sun Jan 02 00:06:41 +0700 2011
  Processing by PostsController#index as HTML
  Parameters: {"user"=>{"name"=>"sikachu"}, "secret"=>"[FILTERED]"}
  Post Load (0.4ms)  SELECT "posts".* FROM "posts" 
Rendered posts/index.html.erb within layouts/application (0.9ms)
Completed 200 OK in 11ms (Views: 6.7ms | ActiveRecord: 0.4ms)

I think this would make your log file safer, if your server got compromised and someone trying to extract something out of your log file.

However, there's one known issue: This approach has been done by reconstructing the QUERY_STRING from request.GET hash. This would result in shifting of parameter position in Ruby version < 1.9, as it doesn't sort hash by insertion order. I don't know if it's crucial or not, but IMO not leaking secret information into log file is more crucial and people would move to 1.9.x one day anyway.

Please review it, and let me know if it's acceptable. Thank you :)

Comments and changes to this ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Tags

Referenced by

Pages