From 69ab91ae9396f0101afd13871f179a7f779d3178 Mon Sep 17 00:00:00 2001 From: Lukasz Sarnacki Date: Thu, 23 Jan 2014 16:31:52 +0100 Subject: 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. --- actionpack/CHANGELOG.md | 8 ++++++++ actionpack/lib/action_controller/log_subscriber.rb | 9 +++++++++ actionpack/lib/action_dispatch/request/utils.rb | 13 +++++++++---- 3 files changed, 26 insertions(+), 4 deletions(-) (limited to 'actionpack') 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 -- cgit v1.2.3