From ee2d0f30b7699248c2ace02c12ce7223102b6077 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Thu, 27 Mar 2014 16:53:09 +0000 Subject: URL Encode the path parameter for render_exception MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a request is made and path is something like /%d3 we rescue this with a custom 404 template. This gets unescaped as {"path"=>"\323"}. In the case of a RouteNotFound, ApplicationController#render_exception renders the general/exception_caught template in to the default layout, which renders the general/_locale_switcher partial. This partial calls url_for – sending the full params hash as the argument – so that a user may return to the existing page in their chosen locale. The problem is that url_for tries to construct the url with the hash {:action=>"not_found", :controller=>"general", :path=>"\323"}. ApplicationController#sanitize_params re-encodes the path parameter so that it can be passed through to url_for without trouble. --- app/controllers/application_controller.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'app/controllers/application_controller.rb') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 370e8e15c..410778d9a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -131,6 +131,7 @@ class ApplicationController < ActionController::Base case exception when ActiveRecord::RecordNotFound, RouteNotFound @status = 404 + sanitize_path(params) when PermissionDenied @status = 403 else @@ -441,6 +442,15 @@ class ApplicationController < ActionController::Base `git log -1 --format="%H"`.strip end + # URL Encode the path parameter for use in render_exception + # + # params - the params Hash + # + # Returns a Hash + def sanitize_path(params) + params.merge!(:path => Rack::Utils.escape(params[:path])) if params.key?(:path) + end + # URL generating functions are needed by all controllers (for redirects), # views (for links) and mailers (for use in emails), so include them into # all of all. -- cgit v1.2.3 From b0fbe646f386b866791fbb321d6fa183bb4a6517 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Thu, 10 Apr 2014 12:53:06 +0100 Subject: Rescue from non-numeric page parameter exceptions will_paginate intentionally throws an ArgumentError when a non-numeric page parameter is used. Conveniently, they tag it with WillPaginate::InvalidPage, so here we rescue with a 404. --- app/controllers/application_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/controllers/application_controller.rb') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 410778d9a..ba086cfa3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -129,7 +129,7 @@ class ApplicationController < ActionController::Base @exception_class = exception.class.to_s @exception_message = exception.message case exception - when ActiveRecord::RecordNotFound, RouteNotFound + when ActiveRecord::RecordNotFound, RouteNotFound, WillPaginate::InvalidPage @status = 404 sanitize_path(params) when PermissionDenied -- cgit v1.2.3 From 8d3b3044fb4a606b76a03abbb71064bcb4875704 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Mon, 14 Apr 2014 10:03:10 +0100 Subject: Rescue from IpSpoofAttackError when using remote IP Some proxies seem to be setting the Client-IP HTTP header to 127.0.0.1. Rails checks that Client-IP is contained in X-Forwarded-For and raises the error. We decided to rescue in this individual case rather than adding a middleware to strip Client-IP (http://writeheavy.com/2011/07/31/when-its-ok-to-turn-of-rails-ip-spoof-checking.html#well_thats_stupid_can_we_turn_it_off) so that we don't introduce unexpected behaviour. If we start to do anything more with request.remote_ip, then we should look at doing so. See http://blog.gingerlime.com/2012/rails-ip-spoofing-vulnerabilities-and-protection for an in-depth look at this issue. --- app/controllers/application_controller.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'app/controllers/application_controller.rb') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 410778d9a..b6547d1d0 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -432,7 +432,11 @@ class ApplicationController < ActionController::Base def country_from_ip country = "" if !AlaveteliConfiguration::gaze_url.empty? - country = quietly_try_to_open("#{AlaveteliConfiguration::gaze_url}/gaze-rest?f=get_country_from_ip;ip=#{request.remote_ip}") + begin + country = quietly_try_to_open("#{AlaveteliConfiguration::gaze_url}/gaze-rest?f=get_country_from_ip;ip=#{request.remote_ip}") + rescue ActionDispatch::RemoteIp::IpSpoofAttackError + country = AlaveteliConfiguration::iso_country_code + end end country = AlaveteliConfiguration::iso_country_code if country.empty? return country -- cgit v1.2.3