CVE-2016-0721: Session Fixation (CWE-384)
value: Session fixation vulnerability in pcsd in pcs before 0.9.157.
CWE description: Authenticating a user, or otherwise establishing a new user session, without invalidating any existing session identifier gives an attacker the opportunity to steal authenticated sessions.
CVSS v3 base score: 8.1
Published: 2017-04-21T15:59Z
Repository: https://github.com/ClusterLabs/pcs
Language: Ruby
Files changed: pcsd/pcsd.conf, pcsd/pcsd.rb, pcsd/session.rb, pcsd/test/test_all_suite.rb, pcsd/test/test_session.rb
Commit date: 2016-02-15 15:57:36 +0100
Commit message: web UI auth: store session data on server side

Diff: diff --git a/pcsd/pcsd.conf b/pcsd/pcsd.conf
index 38c7ea25..5ac8bc76 100644
--- a/pcsd/pcsd.conf
+++ b/pcsd/pcsd.conf
@@ -1,6 +1,9 @@
 # pcsd configuration file
 # Set PCSD_DEBUG to true for advanced pcsd debugging information
 PCSD_DEBUG=false
-RACK_ENV=production
 # Set DISABLE_GUI to true to disable GUI frontend in pcsd
 DISABLE_GUI=false
+# Set web UI sesions lifetime
+SESSION_LIFETIME=3600
+# Do not change
+RACK_ENV=production
diff --git a/pcsd/pcsd.rb b/pcsd/pcsd.rb
index 71369651..7a00a8ba 100644
--- a/pcsd/pcsd.rb
+++ b/pcsd/pcsd.rb
@@ -17,10 +17,11 @@ require 'config.rb'
 require 'pcs.rb'
 require 'auth.rb'
 require 'wizard.rb'
 require 'cfgsync.rb'
 require 'permissions.rb'
+require 'session.rb'
 
 Dir["wizards/*.rb"].each {|file| require file}
 
 use Rack::CommonLogger
 
@@ -41,16 +42,23 @@ begin
 rescue Errno::ENOENT
   secret = generate_cookie_secret()
   File.open(COOKIE_FILE, 'w', 0700) {|f| f.write(secret)}
 end
 
-use Rack::Session::Cookie,
-  :expire_after => 60 * 60,
+session_lifetime = ENV['SESSION_LIFETIME'].to_i()
+session_lifetime = 60 * 60 unless session_lifetime > 0
+use SessionPoolLifetime,
+  :expire_after => session_lifetime,
   :secret => secret,
   :secure => true, # only send over HTTPS
   :httponly => true # don't provide to javascript
 
+# session storage instance
+# will be created by Rack later and fetched in "before" filter
+$session_storage = nil
+$session_storage_env = {}
+
 #use Rack::SSL
 
 if development?
   Dir["wizards/*.rb"].each {|file| also_reload file}
   also_reload 'resource.rb'
@@ -64,10 +72,17 @@ if development?
   also_reload 'cfgsync.rb'
 end
 
 before do
   @auth_user = nil
+
+  # get session storage instance from env
+  if not $session_storage and env[:__session_storage]
+    $session_storage = env[:__session_storage]
+    $session_storage_env = env
+  end
+
   if request.path != '/login' and not request.path == "/logout" and not request.path == '/remote/auth'
     protected! 
   end
   $cluster_name = get_cluster_name()
 end
@@ -114,10 +129,23 @@ $thread_cfgsync = Thread.new {
     }
     sleep(Cfgsync::ConfigSyncControl.sync_thread_interval())
   end
 }
 
+$thread_session_expired = Thread.new {
+  while true
+    sleep(60 * 5)
+    begin
+      if $session_storage
+        $session_storage.drop_expired($session_storage_env)
+      end
+    rescue => e
+      $logger.warn("Exception while removing expired sessions: #{e}")
+    end
+  end
+}
+
 helpers do
   def protected!
     gui_request = ( # these are URLs for web pages
       request.path == '/' or
       request.path == '/manage' or
@@ -332,11 +360,11 @@ end
 
 if not DISABLE_GUI
   get('/login'){ erb :login, :layout => :main }
 
   get '/logout' do 
-    session.clear
+    session.destroy
     erb :login, :layout => :main
   end
 
   post '/login' do
     auth_user = PCSAuth.loginByPassword(
diff --git a/pcsd/session.rb b/pcsd/session.rb
new file mode 100644
index 00000000..c2026328
--- /dev/null
+++ b/pcsd/session.rb
@@ -0,0 +1,71 @@
+require 'rack/session/pool'
+
+class SessionPoolLifetime < Rack::Session::Pool
+
+  def initialize(app, options={})
+    super
+    @pool_timestamp = Hash.new()
+  end
+
+  def call(env)
+    # save session storage to env so we can get it later
+    env[:__session_storage] = self
+    super
+  end
+
+  def get_session(env, sid)
+    with_lock(env) do
+      now = Time.now()
+      # delete the session if expired
+      if @default_options[:expire_after] and sid and @pool_timestamp[sid] and
+        @pool_timestamp[sid] < (now - @default_options[:expire_after])
+      then
+        delete_session(sid)
+      end
+      # create new session if nonexistent
+      unless sid and session = @pool[sid]
+        sid, session = generate_sid, {}
+        @pool.store sid, session
+      end
+      # bump session's access time
+      @pool_timestamp[sid] = now
+      [sid, session]
+    end
+  end
+
+  def set_session(env, session_id, new_session, options)
+    with_lock(env) do
+      @pool.store session_id, new_session
+      # bump session's access time
+      @pool_timestamp[session_id] = Time.now()
+      session_id
+    end
+  end
+
+  def destroy_session(env, session_id, options)
+    with_lock(env) do
+      delete_session(session_id)
+      generate_sid unless options[:drop]
+    end
+  end
+
+  def drop_expired(env)
+    return unless lifetime = @default_options[:expire_after]
+    with_lock(env) {
+      threshold = Time.now() - lifetime
+      @pool_timestamp.select { |sid, timestamp|
+        timestamp < threshold
+      }.keys.each { |sid|
+        delete_session(sid)
+      }
+    }
+  end
+
+  private
+
+  def delete_session(sid)
+    @pool.delete(sid)
+    @pool_timestamp.delete(sid)
+  end
+end
+
diff --git a/pcsd/test/test_all_suite.rb b/pcsd/test/test_all_suite.rb
index bc02ac84..804e702e 100644
--- a/pcsd/test/test_all_suite.rb
+++ b/pcsd/test/test_all_suite.rb
@@ -3,9 +3,10 @@ require 'test/unit'
 
 require 'test_corosyncconf.rb'
 require 'test_cluster.rb'
 require 'test_cluster_entity.rb'
 require 'test_auth.rb'
+require 'test_session.rb'
 require 'test_permissions.rb'
 require 'test_config.rb'
 require 'test_cfgsync.rb'
 require 'test_pcs.rb'
diff --git a/pcsd/test/test_session.rb b/pcsd/test/test_session.rb
new file mode 100644
index 00000000..e72bf012
--- /dev/null
+++ b/pcsd/test/test_session.rb
@@ -0,0 +1,71 @@
+require 'test/unit'
+
+require 'pcsd_test_utils.rb'
+require 'session.rb'
+
+class TestSessionPool < Test::Unit::TestCase
+
+  def setup()
+    @env = {
+      'rack.multithread' => true,
+    }
+  end
+
+  def fixture_get_pool(lifetime)
+    pool = SessionPoolLifetime.new(nil, {:expire_after => lifetime,})
+    (1..3).each { |i| pool.set_session(@env, "sid#{i}", {'value' => i}, {}) }
+    return pool
+  end
+
+  def test_drop_expired_on_get()
+    lifetime = 2
+    pool = fixture_get_pool(lifetime)
+    # touch sessions each second
+    lifetime.times {
+      sleep(1)
+      assert_equal({'value' => 1}, pool.get_session(@env, 'sid1')[1])
+      assert_equal({'value' => 3}, pool.get_session(@env, 'sid3')[1])
+    }
+    # after @lifetime passes the unused session gets removed on access
+    sleep(1)
+    assert_equal({'value' => 1}, pool.get_session(@env, 'sid1')[1])
+    assert_equal({'value' => 3}, pool.get_session(@env, 'sid3')[1])
+    assert_equal({}, pool.get_session(@env, 'sid2')[1])
+  end
+
+  def test_drop_expired_explicit()
+    lifetime = 2
+    pool = fixture_get_pool(lifetime)
+    # touch sessions each second (otherwise they will be removed on access)
+    lifetime.times {
+      sleep(1)
+      pool.get_session(@env, 'sid2')
+      pool.set_session(@env, 'sid3', {'value' => 33}, {})
+    }
+    sleep(1)
+
+    pool.drop_expired(@env)
+    assert_equal(
+      {
+        'sid2' => {'value' => 2,},
+        'sid3' => {'value' => 33,},
+      },
+      pool.pool
+    )
+  end
+
+  def test_no_lifetime()
+    pool = fixture_get_pool(nil)
+    sleep(1)
+    assert_equal({'value' => 1}, pool.get_session(@env, 'sid1')[1])
+    assert_equal({'value' => 2}, pool.get_session(@env, 'sid2')[1])
+    assert_equal({'value' => 3}, pool.get_session(@env, 'sid3')[1])
+    sleep(1)
+    pool.drop_expired(@env)
+    assert_equal({'value' => 1}, pool.get_session(@env, 'sid1')[1])
+    assert_equal({'value' => 2}, pool.get_session(@env, 'sid2')[1])
+    assert_equal({'value' => 3}, pool.get_session(@env, 'sid3')[1])
+  end
+
+end
+

Vulnerable code: RACK_ENV=production
use Rack::Session::Cookie,
  :expire_after => 60 * 60,
    session.clear
Fixed code: # Set web UI sesions lifetime
SESSION_LIFETIME=3600
# Do not change
RACK_ENV=production
require 'session.rb'
session_lifetime = ENV['SESSION_LIFETIME'].to_i()
session_lifetime = 60 * 60 unless session_lifetime > 0
use SessionPoolLifetime,
  :expire_after => session_lifetime,
# session storage instance
# will be created by Rack later and fetched in "before" filter
$session_storage = nil
$session_storage_env = {}


  # get session storage instance from env
  if not $session_storage and env[:__session_storage]
    $session_storage = env[:__session_storage]
    $session_storage_env = env
  end

$thread_session_expired = Thread.new {
  while true
    sleep(60 * 5)
    begin
      if $session_storage
        $session_storage.drop_expired($session_storage_env)
      end
    rescue => e
      $logger.warn("Exception while removing expired sessions: #{e}")
    end
  end
}

    session.destroy
require 'rack/session/pool'

class SessionPoolLifetime < Rack::Session::Pool

  def initialize(app, options={})
    super
    @pool_timestamp = Hash.new()
  end

  def call(env)
    # save session storage to env so we can get it later
    env[:__session_storage] = self
    super
  end

  def get_session(env, sid)
    with_lock(env) do
      now = Time.now()
      # delete the session if expired
      if @default_options[:expire_after] and sid and @pool_timestamp[sid] and
        @pool_timestamp[sid] < (now - @default_options[:expire_after])
      then
        delete_session(sid)
      end
      # create new session if nonexistent
      unless sid and session = @pool[sid]
        sid, session = generate_sid, {}
        @pool.store sid, session
      end
      # bump session's access time
      @pool_timestamp[sid] = now
      [sid, session]
    end
  end

  def set_session(env, session_id, new_session, options)
    with_lock(env) do
      @pool.store session_id, new_session
      # bump session's access time
      @pool_timestamp[session_id] = Time.now()
      session_id
    end
  end

  def destroy_session(env, session_id, options)
    with_lock(env) do
      delete_session(session_id)
      generate_sid unless options[:drop]
    end
  end

  def drop_expired(env)
    return unless lifetime = @default_options[:expire_after]
    with_lock(env) {
      threshold = Time.now() - lifetime
      @pool_timestamp.select { |sid, timestamp|
        timestamp < threshold
      }.keys.each { |sid|
        delete_session(sid)
      }
    }
  end

  private

  def delete_session(sid)
    @pool.delete(sid)
    @pool_timestamp.delete(sid)
  end
end

require 'test_session.rb'
require 'test/unit'

require 'pcsd_test_utils.rb'
require 'session.rb'

class TestSessionPool < Test::Unit::TestCase

  def setup()
    @env = {
      'rack.multithread' => true,
    }
  end

  def fixture_get_pool(lifetime)
    pool = SessionPoolLifetime.new(nil, {:expire_after => lifetime,})
    (1..3).each { |i| pool.set_session(@env, "sid#{i}", {'value' => i}, {}) }
    return pool
  end

  def test_drop_expired_on_get()
    lifetime = 2
    pool = fixture_get_pool(lifetime)
    # touch sessions each second
    lifetime.times {
      sleep(1)
      assert_equal({'value' => 1}, pool.get_session(@env, 'sid1')[1])
      assert_equal({'value' => 3}, pool.get_session(@env, 'sid3')[1])
    }
    # after @lifetime passes the unused session gets removed on access
    sleep(1)
    assert_equal({'value' => 1}, pool.get_session(@env, 'sid1')[1])
    assert_equal({'value' => 3}, pool.get_session(@env, 'sid3')[1])
    assert_equal({}, pool.get_session(@env, 'sid2')[1])
  end

  def test_drop_expired_explicit()
    lifetime = 2
    pool = fixture_get_pool(lifetime)
    # touch sessions each second (otherwise they will be removed on access)
    lifetime.times {
      sleep(1)
      pool.get_session(@env, 'sid2')
      pool.set_session(@env, 'sid3', {'value' => 33}, {})
    }
    sleep(1)

    pool.drop_expired(@env)
    assert_equal(
      {
        'sid2' => {'value' => 2,},
        'sid3' => {'value' => 33,},
      },
      pool.pool
    )
  end

  def test_no_lifetime()
    pool = fixture_get_pool(nil)
    sleep(1)
    assert_equal({'value' => 1}, pool.get_session(@env, 'sid1')[1])
    assert_equal({'value' => 2}, pool.get_session(@env, 'sid2')[1])
    assert_equal({'value' => 3}, pool.get_session(@env, 'sid3')[1])
    sleep(1)
    pool.drop_expired(@env)
    assert_equal({'value' => 1}, pool.get_session(@env, 'sid1')[1])
    assert_equal({'value' => 2}, pool.get_session(@env, 'sid2')[1])
    assert_equal({'value' => 3}, pool.get_session(@env, 'sid3')[1])
  end

end