aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
authorLukasz Sarnacki <lukesarnacki@gmail.com>2014-01-23 16:31:52 +0100
committerLukasz Sarnacki <lukesarnacki@gmail.com>2014-01-28 20:29:38 +0100
commit69ab91ae9396f0101afd13871f179a7f779d3178 (patch)
tree5fea152a7fce596367503badf11bc21fe2e4e0c4 /actionpack
parentb9cd5a29dd4c6142b19c861fbf1a67452320b3dd (diff)
downloadrails-69ab91ae9396f0101afd13871f179a7f779d3178.tar.gz
rails-69ab91ae9396f0101afd13871f179a7f779d3178.tar.bz2
rails-69ab91ae9396f0101afd13871f179a7f779d3178.zip
Log which keys were set to nil in deep_munge
deep_munge solves CVE-2013-0155 security vulnerability, but its behaviour is definately confuisng. This commit adds logging to deep_munge. It logs keys for which values were set to nil. Also mentions in guides were added.
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/CHANGELOG.md8
-rw-r--r--actionpack/lib/action_controller/log_subscriber.rb9
-rw-r--r--actionpack/lib/action_dispatch/request/utils.rb13
3 files changed, 26 insertions, 4 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 23bb01d678..fc05ae3cec 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -11,6 +11,14 @@
*Maurizio De Santis*
+* Log which keys were affected by deep munge.
+
+ Deep munge solves CVE-2013-0155 security vulnerability, but its
+ behaviour is definately confusing, so now at least information
+ about for which keys values were set to nil is visible in logs.
+
+ *Ɓukasz Sarnacki*
+
* Automatically convert dashes to underscores for shorthand routes, e.g:
get '/our-work/latest'
diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb
index 9279d8bcea..823a1050b5 100644
--- a/actionpack/lib/action_controller/log_subscriber.rb
+++ b/actionpack/lib/action_controller/log_subscriber.rb
@@ -53,6 +53,15 @@ module ActionController
debug("Unpermitted parameters: #{unpermitted_keys.join(", ")}")
end
+ def deep_munge(event)
+ message = "Value for params[:#{event.payload[:keys].join('][:')}] was set"\
+ "to nil, because it was one of [], [null] or [null, null, ...]."\
+ "Go to http://guides.rubyonrails.org/security.html#unsafe-query-generation"\
+ "for more information."\
+
+ debug(message)
+ end
+
%w(write_fragment read_fragment exist_fragment?
expire_fragment expire_page write_page).each do |method|
class_eval <<-METHOD, __FILE__, __LINE__ + 1
diff --git a/actionpack/lib/action_dispatch/request/utils.rb b/actionpack/lib/action_dispatch/request/utils.rb
index a6dca9741c..9d4f1aa3c5 100644
--- a/actionpack/lib/action_dispatch/request/utils.rb
+++ b/actionpack/lib/action_dispatch/request/utils.rb
@@ -7,18 +7,23 @@ module ActionDispatch
class << self
# Remove nils from the params hash
- def deep_munge(hash)
+ def deep_munge(hash, keys = [])
return hash unless perform_deep_munge
hash.each do |k, v|
+ keys << k
case v
when Array
- v.grep(Hash) { |x| deep_munge(x) }
+ v.grep(Hash) { |x| deep_munge(x, keys) }
v.compact!
- hash[k] = nil if v.empty?
+ if v.empty?
+ hash[k] = nil
+ ActiveSupport::Notifications.instrument("deep_munge.action_controller", keys: keys)
+ end
when Hash
- deep_munge(v)
+ deep_munge(v, keys)
end
+ keys.pop
end
hash