mirror of
https://github.com/samsonjs/rack-attack.git
synced 2026-03-25 09:25:49 +00:00
Normalize request paths when using Rails' ActionDispatch
The issue
---
When using rack-attack with a rails app, developers expect the request
path to be normalized. In particular, trailing slashes are stripped so
a request path "/login/" becomes "/login" by the time you're in
ActionController.
Since Rack::Attack runs before ActionDispatch, the request path is not
yet normalized. This can cause throttles and blacklists to not work as
expected.
E.g., a throttle:
throttle('logins', ...) {|req| req.path == "/login" }
would not match a request to '/login/', though Rails would route
'/login/' to the same '/login' action.
The solution
---
This patch looks if ActionDispatch's request normalization is loaded,
and if so, uses it to normalize the path before processing throttles,
blacklists, etc.
If it's not loaded, the request path is not modified.
Credit
---
Thanks to Andres Riancho at Include Security for reporting this issue.
This commit is contained in:
parent
bbf8a488ab
commit
76c2e31430
10 changed files with 67 additions and 1 deletions
|
|
@ -3,5 +3,6 @@
|
|||
source "https://rubygems.org"
|
||||
|
||||
gem "activesupport", "~> 3.2.0"
|
||||
gem "actionpack", "~> 3.2.0"
|
||||
|
||||
gemspec :path => "../"
|
||||
|
|
|
|||
|
|
@ -3,5 +3,6 @@
|
|||
source "https://rubygems.org"
|
||||
|
||||
gem "activesupport", "~> 4.0.0"
|
||||
gem "actionpack", "~> 4.0.0"
|
||||
|
||||
gemspec :path => "../"
|
||||
|
|
|
|||
|
|
@ -3,5 +3,6 @@
|
|||
source "https://rubygems.org"
|
||||
|
||||
gem "activesupport", "~> 4.1.0"
|
||||
gem "actionpack", "~> 4.1.0"
|
||||
|
||||
gemspec :path => "../"
|
||||
|
|
|
|||
|
|
@ -2,6 +2,6 @@
|
|||
|
||||
source "https://rubygems.org"
|
||||
|
||||
gem "activesupport", "~> 4.2.1"
|
||||
gem "actionpack", "~> 4.2.1"
|
||||
|
||||
gemspec :path => "../"
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ require 'forwardable'
|
|||
|
||||
class Rack::Attack
|
||||
autoload :Cache, 'rack/attack/cache'
|
||||
autoload :PathNormalizer, 'rack/attack/path_normalizer'
|
||||
autoload :Check, 'rack/attack/check'
|
||||
autoload :Throttle, 'rack/attack/throttle'
|
||||
autoload :Whitelist, 'rack/attack/whitelist'
|
||||
|
|
@ -91,6 +92,7 @@ class Rack::Attack
|
|||
end
|
||||
|
||||
def call(env)
|
||||
env['PATH_INFO'] = PathNormalizer.normalize_path(env['PATH_INFO'])
|
||||
req = Rack::Attack::Request.new(env)
|
||||
|
||||
if whitelisted?(req)
|
||||
|
|
|
|||
27
lib/rack/attack/path_normalizer.rb
Normal file
27
lib/rack/attack/path_normalizer.rb
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
class Rack::Attack
|
||||
|
||||
# When using Rack::Attack with a Rails app, developers expect the request path
|
||||
# to be normalized. In particular, trailing slashes are stripped.
|
||||
# (See http://git.io/v0rrR for implementation.)
|
||||
#
|
||||
# Look for an ActionDispatch utility class that Rails folks would expect
|
||||
# to normalize request paths. If unavailable, use a fallback class that
|
||||
# doesn't normalize the path (as a non-Rails rack app developer expects).
|
||||
|
||||
module FallbackPathNormalizer
|
||||
def self.normalize_path(path)
|
||||
path
|
||||
end
|
||||
end
|
||||
|
||||
PathNormalizer = if defined?(::ActionDispatch::Journey::Router::Utils)
|
||||
# For Rails 4+ apps
|
||||
::ActionDispatch::Journey::Router::Utils
|
||||
elsif defined?(::Journey::Router::Utils)
|
||||
# for Rails 3.2
|
||||
::Journey::Router::Utils
|
||||
else
|
||||
FallbackPathNormalizer
|
||||
end
|
||||
|
||||
end
|
||||
|
|
@ -28,6 +28,7 @@ Gem::Specification.new do |s|
|
|||
s.add_development_dependency 'rake'
|
||||
s.add_development_dependency 'appraisal'
|
||||
s.add_development_dependency 'activesupport', '>= 3.0.0'
|
||||
s.add_development_dependency 'actionpack', '>= 3.0.0'
|
||||
s.add_development_dependency 'redis-activesupport'
|
||||
s.add_development_dependency 'dalli'
|
||||
s.add_development_dependency 'connection_pool'
|
||||
|
|
|
|||
17
spec/rack_attack_path_normalizer_spec.rb
Normal file
17
spec/rack_attack_path_normalizer_spec.rb
Normal file
|
|
@ -0,0 +1,17 @@
|
|||
require_relative 'spec_helper'
|
||||
|
||||
describe Rack::Attack::PathNormalizer do
|
||||
subject { Rack::Attack::PathNormalizer }
|
||||
|
||||
it 'should have a normalize_path method' do
|
||||
subject.normalize_path('/foo').must_equal '/foo'
|
||||
end
|
||||
|
||||
describe 'FallbackNormalizer' do
|
||||
subject { Rack::Attack::FallbackPathNormalizer }
|
||||
|
||||
it '#normalize_path does not change the path' do
|
||||
subject.normalize_path('').must_equal ''
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -3,6 +3,17 @@ require_relative 'spec_helper'
|
|||
describe 'Rack::Attack' do
|
||||
allow_ok_requests
|
||||
|
||||
describe 'normalizing paths' do
|
||||
before do
|
||||
Rack::Attack.blacklist("banned_path") {|req| req.path == '/foo' }
|
||||
end
|
||||
|
||||
it 'blocks requests with trailing slash' do
|
||||
get '/foo/'
|
||||
last_response.status.must_equal 403
|
||||
end
|
||||
end
|
||||
|
||||
describe 'blacklist' do
|
||||
before do
|
||||
@bad_ip = '1.2.3.4'
|
||||
|
|
|
|||
|
|
@ -5,6 +5,11 @@ require "minitest/autorun"
|
|||
require "minitest/pride"
|
||||
require "rack/test"
|
||||
require 'active_support'
|
||||
require 'action_dispatch'
|
||||
|
||||
# Load Journey for Rails 3.2
|
||||
require 'journey' if ActionPack::VERSION::MAJOR == 3
|
||||
|
||||
require "rack/attack"
|
||||
|
||||
begin
|
||||
|
|
|
|||
Loading…
Reference in a new issue