Fix Session replay secure issue that when Rails application use CookieStore.

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
 -----
 

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    rucaptcha (1.0.0)
+    rucaptcha (1.0.1)
       railties (>= 3.2)
 
 GEM

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

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

lib/rucaptcha/version.rb

@@ -1,3 +1,3 @@
 module RuCaptcha
-  VERSION = '1.0.0'
+  VERSION = '1.0.1'
 end

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

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