From e129851fd982bdf5e8cc2c6bbf0d3b11a27bc248 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Fri, 14 Oct 2016 17:33:34 +0800 Subject: [PATCH] Fix Session replay secure issue that when Rails application use CookieStore. --- CHANGELOG.md | 7 ++++ Gemfile.lock | 2 +- lib/rucaptcha/configuration.rb | 2 +- lib/rucaptcha/controller_helpers.rb | 53 +++++++++++++++++++++------- lib/rucaptcha/version.rb | 2 +- spec/controller_helpers_spec.rb | 54 +++++++++++++++++++++++------ spec/spec_helper.rb | 4 +++ 7 files changed, 98 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bfc9f2..cadca14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +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 ec66969..93492ae 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - rucaptcha (1.0.0) + rucaptcha (1.0.1) railties (>= 3.2) GEM diff --git a/lib/rucaptcha/configuration.rb b/lib/rucaptcha/configuration.rb index 7a87cf7..d4d31c4 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 - # session[:_rucaptcha] expire time, default 2 minutes + # 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 ceb9c76..1f67a7c 100644 --- a/lib/rucaptcha/controller_helpers.rb +++ b/lib/rucaptcha/controller_helpers.rb @@ -6,28 +6,55 @@ module RuCaptcha helper_method :verify_rucaptcha? end - def generate_rucaptcha - session[:_rucaptcha] = RuCaptcha::Captcha.random_chars - session[:_rucaptcha_at] = Time.now.to_i + def rucaptcha_sesion_key_key + ['rucaptcha-session', session.id].join(':') + end - RuCaptcha::Captcha.create(session[:_rucaptcha]) + def generate_rucaptcha + code = RuCaptcha::Captcha.random_chars + Rails.cache.write(rucaptcha_sesion_key_key, { + code: code, + time: Time.now.to_i + }) + + RuCaptcha::Captcha.create(code) end def verify_rucaptcha?(resource = nil) - rucaptcha_at = session[:_rucaptcha_at].to_i + 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 captcha = (params[:_rucaptcha] || '').downcase.strip - - # 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) + if captcha.blank? + return add_rucaptcha_validation_error end - if resource && resource.respond_to?(:errors) - resource.errors.add(:base, t('rucaptcha.invalid')) unless valid + if captcha != store_info[:code] + return add_rucaptcha_validation_error end - valid + 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 end end end diff --git a/lib/rucaptcha/version.rb b/lib/rucaptcha/version.rb index 12a51a3..96b3a67 100644 --- a/lib/rucaptcha/version.rb +++ b/lib/rucaptcha/version.rb @@ -1,3 +1,3 @@ module RuCaptcha - VERSION = '1.0.0' + VERSION = '1.0.1' end diff --git a/spec/controller_helpers_spec.rb b/spec/controller_helpers_spec.rb index f1db008..bddf38c 100644 --- a/spec/controller_helpers_spec.rb +++ b/spec/controller_helpers_spec.rb @@ -1,23 +1,45 @@ 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 ||= {} + @session ||= CustomSession.new 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.session[:_rucaptcha]).to eq('abcd') + expect(simple.custom_session[:code]).to eq('abcd') end end @@ -29,7 +51,7 @@ describe RuCaptcha do end it 'should work when session[:_rucaptcha] is nil' do - simple.session[:_rucaptcha] = nil + simple.clean_custom_session simple.params[:_rucaptcha] = 'Abcd' expect(simple.verify_rucaptcha?).to eq(false) end @@ -37,11 +59,18 @@ describe RuCaptcha do context 'Correct chars in params' do it 'should work' do - simple.session[:_rucaptcha_at] = Time.now.to_i - simple.session[:_rucaptcha] = 'abcd' + Rails.cache.write(simple.rucaptcha_sesion_key_key, { + time: Time.now.to_i, + code: 'abcd' + }) simple.params[:_rucaptcha] = 'Abcd' expect(simple.verify_rucaptcha?).to eq(true) - simple.session[:_rucaptcha] = 'abcd' + expect(simple.custom_session).to eq nil + + Rails.cache.write(simple.rucaptcha_sesion_key_key, { + time: Time.now.to_i, + code: 'abcd' + }) simple.params[:_rucaptcha] = 'AbcD' expect(simple.verify_rucaptcha?).to eq(true) end @@ -49,17 +78,22 @@ describe RuCaptcha do describe 'Incorrect chars' do it 'should work' do - simple.session[:_rucaptcha_at] = Time.now.to_i - 60 - simple.session[:_rucaptcha] = 'abcd' + Rails.cache.write(simple.rucaptcha_sesion_key_key, { + time: Time.now.to_i - 60, + code: '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 - simple.session[:_rucaptcha_at] = Time.now.to_i - 121 - simple.session[:_rucaptcha] = 'abcd' + Rails.cache.write(simple.rucaptcha_sesion_key_key, { + time: Time.now.to_i - 121, + code: '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 75c1ef6..7549506 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -13,6 +13,10 @@ module Rails def root Pathname.new(File.join(File.dirname(__FILE__), '..')) end + + def cache + @cache ||= ActiveSupport::Cache::MemoryStore.new + end end end