add rack-protection, fixes #310

CHANGES

@@ -11,6 +11,9 @@
 
  * Added support for HTTP PATCH requests. (Konstantin Haase)
 
+ * Use rack-protection to defend against common opportunistic attacks.
+   (Konstantin Haase)
+
  * Support for Creole templates, Creole is a standardized wiki markup,
    supported by many wiki implementations. (Konstanin Haase)
 

Gemfile

@@ -8,6 +8,7 @@
 
 RUBY_ENGINE = 'ruby' unless defined? RUBY_ENGINE
 source :rubygems unless ENV['QUICK']
+gemspec
 
 gem 'rake'
 gem 'rack-test', '>= 0.5.6'

README.rdoc

@@ -104,6 +104,9 @@ Route patterns may have optional parameters:
     # matches "GET /posts" and any extension "GET /posts.json", "GET /posts.xml" etc.
   end
 
+By the way, unless you disable the path traversal attack protection (see below),
+the request path might be modified before matching against your routes.
+
 === Conditions
 
 Routes may include a variety of matching conditions, such as the user agent:
@@ -1238,6 +1241,23 @@ You can access those options via <tt>settings</tt>:
     ...
   end
 
+=== Configuring attack protection
+
+Sinatra is using
+{Rack::Protection}[https://github.com/rkh/rack-protection#readme] to defend
+you application against common, opportunistic attacks. You can easily disable
+this behavior (which should result in performance gains):
+
+  disable :protection
+
+To skip a single defense layer, set +protection+ to an options hash:
+
+  set :protection, :except => :path_traversal
+
+You can also hand in an array in order to disable a list of protections:
+
+  set :protections, :except => [:path_traversal, :session_hijacking]
+
 === Available Settings
 
 [absolute_redirects]   If disabled, Sinatra will allow relative redirects,
@@ -1290,6 +1310,9 @@ You can access those options via <tt>settings</tt>:
                        <tt>redirect '/foo'</tt> would behave like
                        <tt>redirect to('/foo')</tt>. Disabled per default.
 
+[protection]           Whether or not to enable web attack protections. See
+                       protection section above.
+
 [public_folder]        folder public files are served from
 
 [reload_templates]     whether or not to reload templates between requests.

lib/sinatra/base.rb

@@ -1,6 +1,7 @@
 # external dependencies
 require 'rack'
 require 'tilt'
+require "rack/protection"
 
 # stdlib dependencies
 require 'thread'
@@ -1308,8 +1309,9 @@ module Sinatra
         builder.use ShowExceptions       if show_exceptions?
         builder.use Rack::MethodOverride if method_override?
         builder.use Rack::Head
-        setup_logging  builder
+        setup_logging    builder
-        setup_sessions builder
+        setup_sessions   builder
+        setup_protection builder
       end
 
       def setup_middleware(builder)
@@ -1329,6 +1331,14 @@ module Sinatra
         end
       end
 
+      def setup_protection(builder)
+        return unless protection?
+        options = Hash === protection ? protection.dup : {}
+        options[:except] = Array options[:except]
+        options[:except] += [:session_hijacking, :remote_token] unless sessions?
+        builder.use Rack::Protection, options
+      end
+
       def setup_sessions(builder)
         return unless sessions?
         options = {}
@@ -1436,6 +1446,7 @@ module Sinatra
     set :show_exceptions, Proc.new { development? }
     set :sessions, false
     set :logging, false
+    set :protection, true
     set :method_override, false
     set :default_encoding, "utf-8"
     set :add_charset, %w[javascript xml xhtml+xml json].map { |t| "application/#{t}" }

sinatra.gemspec

@@ -12,6 +12,7 @@ Gem::Specification.new 'sinatra', Sinatra::VERSION do |s|
   s.extra_rdoc_files  = s.files.select { |p| p =~ /^README/ } << 'LICENSE'
   s.rdoc_options      = %w[--line-numbers --inline-source --title Sinatra --main README.rdoc]
 
-  s.add_dependency 'rack', '~> 1.3'
+  s.add_dependency 'rack',            '~> 1.3'
-  s.add_dependency 'tilt', '~> 1.3'
+  s.add_dependency 'rack-protection', '~> 1.0'
+  s.add_dependency 'tilt',            '~> 1.3'
 end

test/helpers_test.rb

@@ -380,7 +380,7 @@ class HelpersTest < Test::Unit::TestCase
         enable :sessions
 
         get '/' do
-          assert session.empty?
+          assert session[:foo].nil?
           session[:foo] = 'bar'
           redirect '/hi'
         end

test/settings_test.rb

@@ -490,4 +490,49 @@ class SettingsTest < Test::Unit::TestCase
       assert ! @application.lock?
     end
   end
+
+  describe 'protection' do
+    class MiddlewareTracker < Rack::Builder
+      def self.track
+        Rack.send :remove_const, :Builder
+        Rack.const_set :Builder, MiddlewareTracker
+        MiddlewareTracker.used.clear
+        yield
+      ensure
+        Rack.send :remove_const, :Builder
+        Rack.const_set :Builder, MiddlewareTracker.superclass
+      end
+
+      def self.used
+        @used ||= []
+      end
+
+      def use(middleware, *)
+        MiddlewareTracker.used << middleware
+        super
+      end
+    end
+
+    it 'sets up Rack::Protection' do
+      MiddlewareTracker.track do
+        Sinatra::Base.new
+        assert_include MiddlewareTracker.used, Rack::Protection
+      end
+    end
+
+    it 'sets up Rack::Protection::PathTraversal' do
+      MiddlewareTracker.track do
+        Sinatra::Base.new
+        assert_include MiddlewareTracker.used, Rack::Protection::PathTraversal
+      end
+    end
+
+    it 'does not set up Rack::Protection::PathTraversal when disabling it' do
+      MiddlewareTracker.track do
+        Sinatra.new { set :protection, :except => :path_traversal }.new
+        assert_include MiddlewareTracker.used, Rack::Protection
+        assert !MiddlewareTracker.used.include?(Rack::Protection::PathTraversal)
+      end
+    end
+  end
 end