From f8590ad45a247c09c7a37f8ee168dd8b0aa04a7e Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Wed, 26 Oct 2016 17:36:21 +0800 Subject: [PATCH] Revert "Fix Session replay secure issue that when Rails application use CookieStore." This reverts commit e129851fd982bdf5e8cc2c6bbf0d3b11a27bc248. --- CHANGELOG.md | 7 ---- Gemfile.lock | 2 +- lib/rucaptcha/configuration.rb | 2 +- lib/rucaptcha/controller_helpers.rb | 51 +++++++-------------------- lib/rucaptcha/version.rb | 2 +- spec/controller_helpers_spec.rb | 54 ++++++----------------------- spec/spec_helper.rb | 4 --- 7 files changed, 25 insertions(+), 97 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cadca14..6bfc9f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,3 @@ -1.0.1 ------ - -## Security Notes - -- Fix Session replay secure issue that when Rails application use CookieStore. - 1.0.0 ----- diff --git a/Gemfile.lock b/Gemfile.lock index 93492ae..ec66969 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - rucaptcha (1.0.1) + rucaptcha (1.0.0) railties (>= 3.2) GEM diff --git a/lib/rucaptcha/configuration.rb b/lib/rucaptcha/configuration.rb index d4d31c4..7a87cf7 100644 --- a/lib/rucaptcha/configuration.rb +++ b/lib/rucaptcha/configuration.rb @@ -11,7 +11,7 @@ module RuCaptcha attr_accessor :cache_limit # Color style, default: :colorful, allows: [:colorful, :black_white] attr_accessor :style - # rucaptcha expire time, default 2 minutes + # session[:_rucaptcha] expire time, default 2 minutes attr_accessor :expires_in end end diff --git a/lib/rucaptcha/controller_helpers.rb b/lib/rucaptcha/controller_helpers.rb index 1f67a7c..ceb9c76 100644 --- a/lib/rucaptcha/controller_helpers.rb +++ b/lib/rucaptcha/controller_helpers.rb @@ -6,55 +6,28 @@ module RuCaptcha helper_method :verify_rucaptcha? end - def rucaptcha_sesion_key_key - ['rucaptcha-session', session.id].join(':') - end - def generate_rucaptcha - code = RuCaptcha::Captcha.random_chars - Rails.cache.write(rucaptcha_sesion_key_key, { - code: code, - time: Time.now.to_i - }) + session[:_rucaptcha] = RuCaptcha::Captcha.random_chars + session[:_rucaptcha_at] = Time.now.to_i - RuCaptcha::Captcha.create(code) + RuCaptcha::Captcha.create(session[:_rucaptcha]) end def verify_rucaptcha?(resource = nil) - store_info = Rails.cache.read(rucaptcha_sesion_key_key) - # make sure move used key - Rails.cache.delete(rucaptcha_sesion_key_key) - - # Make sure session exist - if store_info.blank? - return add_rucaptcha_validation_error - end - - # Make sure not expire - if (Time.now.to_i - store_info[:time]) > RuCaptcha.config.expires_in - return add_rucaptcha_validation_error - end - - # Make sure parama have captcha + rucaptcha_at = session[:_rucaptcha_at].to_i captcha = (params[:_rucaptcha] || '').downcase.strip - if captcha.blank? - return add_rucaptcha_validation_error + + # Captcha chars in Session expire in 2 minutes + valid = false + if (Time.now.to_i - rucaptcha_at) <= RuCaptcha.config.expires_in + valid = captcha.present? && captcha == session.delete(:_rucaptcha) end - if captcha != store_info[:code] - return add_rucaptcha_validation_error + if resource && resource.respond_to?(:errors) + resource.errors.add(:base, t('rucaptcha.invalid')) unless valid end - true - end - - private - - def add_rucaptcha_validation_error - if defined?(resource) && resource && resource.respond_to?(:errors) - resource.errors.add(:base, t('rucaptcha.invalid')) - end - false + valid end end end diff --git a/lib/rucaptcha/version.rb b/lib/rucaptcha/version.rb index 96b3a67..12a51a3 100644 --- a/lib/rucaptcha/version.rb +++ b/lib/rucaptcha/version.rb @@ -1,3 +1,3 @@ module RuCaptcha - VERSION = '1.0.1' + VERSION = '1.0.0' end diff --git a/spec/controller_helpers_spec.rb b/spec/controller_helpers_spec.rb index bddf38c..f1db008 100644 --- a/spec/controller_helpers_spec.rb +++ b/spec/controller_helpers_spec.rb @@ -1,45 +1,23 @@ require 'spec_helper' -require 'securerandom' describe RuCaptcha do - class CustomSession - attr_accessor :id - - def initialize - self.id = SecureRandom.hex - end - end class Simple < ActionController::Base def session - @session ||= CustomSession.new + @session ||= {} end def params @params ||= {} end - - def custom_session - Rails.cache.read(self.rucaptcha_sesion_key_key) - end - - def clean_custom_session - Rails.cache.delete(self.rucaptcha_sesion_key_key) - end end let(:simple) { Simple.new } - describe '.rucaptcha_sesion_key_key' do - it 'should work' do - expect(simple.rucaptcha_sesion_key_key).to eq ['rucaptcha-session', simple.session.id].join(':') - end - end - describe '.generate_rucaptcha' do it 'should work' do expect(RuCaptcha::Captcha).to receive(:random_chars).and_return('abcd') expect(simple.generate_rucaptcha).not_to be_nil - expect(simple.custom_session[:code]).to eq('abcd') + expect(simple.session[:_rucaptcha]).to eq('abcd') end end @@ -51,7 +29,7 @@ describe RuCaptcha do end it 'should work when session[:_rucaptcha] is nil' do - simple.clean_custom_session + simple.session[:_rucaptcha] = nil simple.params[:_rucaptcha] = 'Abcd' expect(simple.verify_rucaptcha?).to eq(false) end @@ -59,18 +37,11 @@ describe RuCaptcha do context 'Correct chars in params' do it 'should work' do - Rails.cache.write(simple.rucaptcha_sesion_key_key, { - time: Time.now.to_i, - code: 'abcd' - }) + simple.session[:_rucaptcha_at] = Time.now.to_i + simple.session[:_rucaptcha] = 'abcd' simple.params[:_rucaptcha] = 'Abcd' expect(simple.verify_rucaptcha?).to eq(true) - expect(simple.custom_session).to eq nil - - Rails.cache.write(simple.rucaptcha_sesion_key_key, { - time: Time.now.to_i, - code: 'abcd' - }) + simple.session[:_rucaptcha] = 'abcd' simple.params[:_rucaptcha] = 'AbcD' expect(simple.verify_rucaptcha?).to eq(true) end @@ -78,22 +49,17 @@ describe RuCaptcha do describe 'Incorrect chars' do it 'should work' do - Rails.cache.write(simple.rucaptcha_sesion_key_key, { - time: Time.now.to_i - 60, - code: 'abcd' - }) + simple.session[:_rucaptcha_at] = Time.now.to_i - 60 + simple.session[:_rucaptcha] = 'abcd' simple.params[:_rucaptcha] = 'd123' expect(simple.verify_rucaptcha?).to eq(false) - expect(simple.custom_session).to eq nil end end describe 'Expires Session key' do it 'should work' do - Rails.cache.write(simple.rucaptcha_sesion_key_key, { - time: Time.now.to_i - 121, - code: 'abcd' - }) + simple.session[:_rucaptcha_at] = Time.now.to_i - 121 + simple.session[:_rucaptcha] = 'abcd' simple.params[:_rucaptcha] = 'abcd' expect(simple.verify_rucaptcha?).to eq(false) end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7549506..75c1ef6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -13,10 +13,6 @@ module Rails def root Pathname.new(File.join(File.dirname(__FILE__), '..')) end - - def cache - @cache ||= ActiveSupport::Cache::MemoryStore.new - end end end