Permalink
Browse files

Merge [8176] to stable to fix session fixation attacks. Closes #10048

…[theflow, Koz]

git-svn-id: http://svn-commit.rubyonrails.org/rails/branches/1-2-stable@8177 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent 65539c9 commit 63e968794acb4435d44f8706d26f065bdcc79487 @NZKoz NZKoz committed Nov 21, 2007
@@ -36,24 +36,28 @@ def process_cgi(cgi, session_options = {}) #:nodoc:
end
class CgiRequest < AbstractRequest #:nodoc:
- attr_accessor :cgi, :session_options, :cookie_only
+ attr_accessor :cgi, :session_options
class SessionFixationAttempt < StandardError; end #:nodoc:
DEFAULT_SESSION_OPTIONS = {
:database_manager => CGI::Session::PStore,
:prefix => "ruby_sess.",
:session_path => "/",
+ :session_key => "_session_id",
:cookie_only => true
} unless const_defined?(:DEFAULT_SESSION_OPTIONS)
def initialize(cgi, session_options = {})
@cgi = cgi
@session_options = session_options
@env = @cgi.send(:env_table)
- @cookie_only = session_options.delete :cookie_only
super()
end
+ def cookie_only?
+ session_options_with_string_keys['cookie_only']
+ end
+
def query_string
if (qs = @cgi.query_string) && !qs.empty?
qs
@@ -114,7 +118,7 @@ def session
@session = Hash.new
else
stale_session_check! do
- if @cookie_only && request_parameters[session_options_with_string_keys['session_key']]
+ if cookie_only? && request_parameters[session_options_with_string_keys['session_key']]
raise SessionFixationAttempt
end
case value = session_options_with_string_keys['new_session']
@@ -0,0 +1,92 @@
+require File.dirname(__FILE__) + '/../abstract_unit'
+
+class SessionFixationTest < Test::Unit::TestCase
+ class MockCGI < CGI #:nodoc:
+ attr_accessor :stdoutput, :env_table
+
+ def initialize(env, data = '')
+ self.env_table = env
+ self.stdoutput = StringIO.new
+ super(StringIO.new(data))
+ end
+ end
+
+ class TestController < ActionController::Base
+ session :session_key => '_myapp_session_id', :secret => 'secret', :except => :default_session_key
+ session :cookie_only => false, :only => :allow_session_fixation
+
+ def default_session_key
+ render :text => "default_session_key"
+ end
+
+ def custom_session_key
+ render :text => "custom_session_key: #{params[:id]}"
+ end
+
+ def allow_session_fixation
+ render :text => "allow_session_fixation"
+ end
+
+ def rescue_action(e) raise end
+ end
+
+ def setup
+ @controller = TestController.new
+ end
+
+ def test_should_be_able_to_make_a_successful_request
+ cgi = mock_cgi_for_request_to(:custom_session_key, :id => 1)
+
+ assert_nothing_raised do
+ @controller.send(:process, mock_request(cgi), ActionController::CgiResponse.new(cgi))
+ end
+ assert_equal 'custom_session_key: 1', @controller.response.body
+ assert_not_nil @controller.session
+ end
+
+ def test_should_catch_session_fixation_attempt
+ cgi = mock_cgi_for_request_to(:custom_session_key, :_myapp_session_id => 42)
+
+ assert_raises ActionController::CgiRequest::SessionFixationAttempt do
+ @controller.send(:process, mock_request(cgi), ActionController::CgiResponse.new(cgi))
+ end
+ assert_nil @controller.session
+ end
+
+ def test_should_not_catch_session_fixation_attempt_when_cookie_only_setting_is_disabled
+ cgi = mock_cgi_for_request_to(:allow_session_fixation, :_myapp_session_id => 42)
+
+ assert_nothing_raised do
+ @controller.send(:process, mock_request(cgi), ActionController::CgiResponse.new(cgi))
+ end
+ assert !@controller.response.body.blank?
+ assert_not_nil @controller.session
+ end
+
+ def test_should_catch_session_fixation_attempt_with_default_session_key
+ ActionController::Base.session_store = :p_store # using the default session_key is not possible with cookie store
+ cgi = mock_cgi_for_request_to(:default_session_key, :_session_id => 42)
+
+ assert_raises ActionController::CgiRequest::SessionFixationAttempt do
+ @controller.send(:process, mock_request(cgi) , ActionController::CgiResponse.new(cgi))
+ end
+ assert @controller.response.body.blank?
+ assert_nil @controller.session
+ end
+
+private
+
+ def mock_cgi_for_request_to(action, params = {})
+ MockCGI.new({
+ "REQUEST_METHOD" => "GET",
+ "QUERY_STRING" => "action=#{action}&#{params.to_query}",
+ "REQUEST_URI" => "/",
+ "SERVER_PORT" => "80",
+ "HTTP_HOST" => "testdomain.com" }, '')
+ end
+
+ def mock_request(cgi)
+ ActionController::CgiRequest.new(cgi, {})
+ end
+
+end

0 comments on commit 63e9687

Please sign in to comment.