Rails Ruby on Rails

Comments and changes to this ticket

• 

  Neeraj Singh April 19th, 2011 @ 11:07 PM

  • Importance changed from “” to “Low”

  Which version of Rails you are using? I tested it with one of my apps which is using Rails 3.0.3 and this is what I am getting. Notice that password is filtered.

  (rdb:1) eval ActionDispatch::Request.new(env).filtered_env
  ....
  "rack.request.form_input"=>#<StringIO:0x107ef78a8>, "HTTP_ACCEPT_CHARSET"=>"ISO-8859-1,utf-8;q=0.7,*;q=0.3", "SERVER_PORT"=>"3000", "action_controller.instance"=>#<SessionsController:0x107e9b2b0 ...>, "rack.session.options"=>{:httponly=>true, :expire_after=>nil, :domain=>nil, :path=>"/", :id=>"92c6cfe0e146cf646c06d13641ec6af8", :secure=>false}, "REQUEST_METHOD"=>"POST", "HTTP_ORIGIN"=>"http://localhost:3000", "rack.request.query_string"=>"", "rack.request.form_hash"=>{"commit"=>"Login", "authenticity_token"=>"NJIXco+zVXb9bYJOb3ethlOOY9Y0vW7skoBZhsPjFYI=", "utf8"=>"\342\234\223", "password"=>"welcome", "email"=>"neeraj.cmu@gmail.com"}, "action_dispatch.request.content_type"=>#<Mime::Type:0x101980880 @synonyms=[], @symbol=:url_encoded_form, @string="application/x-www-form-urlencoded">, "QUERY_STRING"=>"", "rack.input"=>#<StringIO:0x107ef78a8>, "HTTP_CONNECTION"=>"keep-alive", "HTTP_ACCEPT_ENCODING"=>"gzip,deflate,sdch", "GATEWAY_INTERFACE"=>"CGI/1.2"}, @_config=#<OrderedHash {}>, @_status=200, @_action_name="create", @lookup_context=#<ActionView::LookupContext:0x107e9adb0 @view_paths=[#<ActionView::FileSystemResolver:0x108be3788 @cached={}, @caching=false, @path="/Users/nsingh/dev/personal/teams99_new/app/views">, #<ActionView::FileSystemResolver:0x108be4778 @cached={}, @caching=false, @path="/Users/nsingh/dev/personal/teams99_new/vendor/ruby/1.8/gems/handy-0.0.15/app/views">, #<ActionView::FileSystemResolver:0x108c0e938 @cached={}, @caching=false, @path="/Users/nsingh/dev/personal/teams99_new/vendor/ruby/1.8/gems/admin_data-1.1.10/app/views">], @details_key=nil, @skip_default_locale=false, @details={:locale=>[:en, :en], :formats=>[:html], :handlers=>[:rhtml, :rxml, :erb, :builder, :haml, :rjs]}, @frozen_formats=false>>, "HTTP_ORIGIN"=>"http://localhost:3000", "GATEWAY_INTERFACE"=>"CGI/1.2", "HTTP_ACCEPT_ENCODING"=>"gzip,deflate,sdch", "HTTP_CONNECTION"=>"keep-alive", "rack.input"=>#<StringIO:0x107ef78a8>, "QUERY_STRING"=>"", "action_dispatch.request.content_type"=>#<Mime::Type:0x101980880 @synonyms=[], @symbol=:url_encoded_form, @string="application/x-www-form-urlencoded">, "rack.request.form_hash"=>{"commit"=>"Login", "utf8"=>"\342\234\223", "authenticity_token"=>"NJIXco+zVXb9bYJOb3ethlOOY9Y0vW7skoBZhsPjFYI=", "password"=>"[FILTERED]", "email"=>"neeraj@example.com"}, "rack.request.query_string"=>""}
  

• You flagged this item as spam.
  

  Sebastian Martinez April 19th, 2011 @ 11:15 PM

  I'm using Rails 3.0.3 too.

  In the response you posted, the password is not filtered in some places but in others it's not.
  The password you used is 'welcome'.
  Check "rack.request.form_hash".
  I am also seeing it without being filtered on "action_dispatch.request.request_parameters" that you omitted.

• 

  Neeraj Singh April 20th, 2011 @ 03:46 AM

  I investigated a bit and this is what I found.

  The output looks like this https://gist.github.com/930236

  Look at #8 the key is (action_dispatch.request.parameters)
  and the password is filtered at line #16

  Now look at line #33 the key is same (action_dispatch.request.parameters) . Notice that password is not filtered at line #41.

  Question is why?

  The second key (action_dispatch.request.parameters) has a parent key named (action_dispatch.remote_ip) line #28.

  When rails gets the key and value for this case it looks like this.
  key: action_dispatch.remote_ip

  value : [something] . However notice that the class of this value is not Hash. In this case value.is_a?(Hash) return false since the class of value is ActionDispatch::RemoteIp::RemoteIpGetter.

  The filtering code looks like this

                if regexps.find { |r| key =~ r }                value = '[FILTERED]'
                elsif value.is_a?(Hash)
                  value = filter(value)
                elsif value.is_a?(Array)
                  value = value.map { |v| v.is_a?(Hash) ? filter(v) : v }
                else ..
  

  Notice that in this case value does not satisfy any of the above cases and hence the value is left as it is and the password value is exposed.

  What's the fix?

  I don't have a good solution. It is not practically possible to look inside all different kinds of objects and replace the value. I can think of a solution where the values are not printed for the classes like the one mentioned above in normal mode. If you are debugging something then in the debug mode full value should be printed.

• You flagged this item as spam.
  

  Steve Schwartz April 20th, 2011 @ 04:44 AM

  I don't know much about the context of the filter code you posted, but I'm assuming it logs the ActionDispatch::RemoteIp::RemoteIpGetter by calling to_s on it. Could something like this work?

  elsif value.respond_to?(:to_s)
    value = value.to_s.gsub!(/(#{key}:\s*)([^\s,$]+)/, "#{$1}#{filter($2)}")
  

• 

  Neeraj Singh April 20th, 2011 @ 10:02 AM

  @steve

  to_s is currently not being called on ActionDispatch::RemoteIp::RemoteIpGetter. Rather the value is left intact. In other words value is not being altered at all.

  Secondly after invoking to_s it is hard to be precise about key and value and each class can have its own implementation of to_s.

• You flagged this item as spam.
  

  Steve Schwartz April 26th, 2011 @ 12:57 AM

  @neeraj, I think you misunderstood. I was referring to when ActionDispatch::RemoteIp::RemoteIpGetter is actually getting output (i.e. with print or puts or whatever), at which time the to_s is called on it.

  But yes, the code you had excerpted was from within the ActionDispatch::Http::ParameterFilter class, which happens before the to_s method ever gets called, so that wouldn't be solvable from here. I would imagine a fix for this issue would have to go straight in the filtered_env method itself.

• You flagged this item as spam.
  

  Steve Schwartz April 26th, 2011 @ 12:59 AM

  Oh, and concerning the difficulty in being precise about key/value for each class's own to_s method, yes I agree, but when it comes to filtering sensitive info, something is better than nothing.