search

pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
307 stars 448 forks source link

Staled session IDs might lead to minor request failures #9859

Open jonasraoni opened 8 months ago

jonasraoni commented 8 months ago

Describe the bug Requests containing invalid/expired sessions might fail due to race-conditions, also it's also possible to tamper the session ID with random values, which might generate exceptions (log noise).

To Reproduce Steps to reproduce the behavior:

  1. Change the session ID cookie value manually at the browser (one with a long value to simulate the field maxlength issue)
  2. Launch multiple requests

Race-condition:

PHP Fatal error:  Uncaught PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '{session_id}' for key 'sessions.sessions_pkey' in /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Database/Connection.php:570
Stack trace:
#0 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Database/Connection.php(570): PDOStatement->execute()
#1 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Database/Connection.php(753): Illuminate\\Database\\Connection->Illuminate\\Database\\{closure}('INSERT INTO ses...', Array)
#2 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Database/Connection.php(720): Illuminate\\Database\\Connection->runQueryCallback('INSERT INTO ses...', Array, Object(Closure))
#3 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Database/Connection.php(577): Illuminate\\Database\\Connection->run('INSERT INTO ses...', Array, Object(Closure))
#4 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Database/DatabaseManager.php(469): Illuminate\\Database\\Connection->affectingStatement('INSERT INTO ses...', Array)
#5 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(338): Illuminate\\Database\\DatabaseManager->__call('affectingStatem...', Array)
#6 /var/www/html/lib/pkp/classes/db/DAO.php(170): Illuminate\\Support\\Facades\\Facade::__callStatic('affectingStatem...', Array)
#7 /var/www/html/lib/pkp/classes/session/SessionDAO.php(82): PKP\\db\\DAO->update('INSERT INTO ses...', Array)
#8 /var/www/html/lib/pkp/classes/session/SessionManager.php(375): PKP\\session\\SessionDAO->insertObject(Object(PKP\\session\\Session))
#9 /var/www/html/lib/pkp/classes/session/SessionManager.php(61): PKP\\session\\SessionManager->createSession()
#10 /var/www/html/lib/pkp/classes/session/SessionManager.php(70): PKP\\session\\SessionManager->__construct()
#11 /var/www/html/lib/pkp/classes/security/Validation.php(420): PKP\\session\\SessionManager::getManager()
#12 /var/www/html/lib/pkp/classes/core/PKPPageRouter.php(84): PKP\\security\\Validation::isLoggedIn()
#13 /var/www/html/lib/pkp/classes/core/Dispatcher.php(139): PKP\\core\\PKPPageRouter->isCacheable(Object(APP\\core\\Request))
#14 /var/www/html/lib/pkp/classes/core/PKPApplication.php(388): PKP\\core\\Dispatcher->dispatch(Object(APP\\core\\Request))
#15 /var/www/html/index.php(21): PKP\\core\\PKPApplication->execute()
#16 {main}

Tampering:

Uncaught PDOException: SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'session_id' at row 1 in /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Database/Connection.php:570
Stack trace:
#0 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Database/Connection.php(570): PDOStatement->execute()
#1 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Database/Connection.php(753): Illuminate\Database\Connection->Illuminate\Database\{closure}('INSERT INTO ses...', Array)
#2 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Database/Connection.php(720): Illuminate\Database\Connection->runQueryCallback('INSERT INTO ses...', Array, Object(Closure))
#3 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Database/Connection.php(577): Illuminate\Database\Connection->run('INSERT INTO ses...', Array, Object(Closure))
#4 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Database/DatabaseManager.php(469): Illuminate\Database\Connection->affectingStatement('INSERT INTO ses...', Array)
#5 /var/www/html/lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(338): Illuminate\Database\DatabaseManager->__call('affectingStatem...', Array)
#6 /var/www/html/lib/pkp/classes/db/DAO.php(170): Illuminate\Support\Facades\Facade::__callStatic('affectingStatem...', Array)
#7 /var/www/html/lib/pkp/classes/session/SessionDAO.php(82): PKP\db\DAO->update('INSERT INTO ses...', Array)
#8 /var/www/html/lib/pkp/classes/session/SessionManager.php(375): PKP\session\SessionDAO->insertObject(Object(PKP\session\Session))
#9 /var/www/html/lib/pkp/classes/session/SessionManager.php(61): PKP\session\SessionManager->createSession()
#10 /var/www/html/lib/pkp/classes/session/SessionManager.php(70): PKP\session\SessionManager->__construct()
#11 /var/www/html/lib/pkp/classes/security/Validation.php(420): PKP\session\SessionManager::getManager()
#12 /var/www/html/lib/pkp/classes/core/PKPPageRouter.php(84): PKP\security\Validation::isLoggedIn()
#13 /var/www/html/lib/pkp/classes/core/Dispatcher.php(139): PKP\core\PKPPageRouter->isCacheable(Object(APP\core\Request))
#14 /var/www/html/lib/pkp/classes/core/PKPApplication.php(388): PKP\core\Dispatcher->dispatch(Object(APP\core\Request))
#15 /var/www/html/index.php(21): PKP\core\PKPApplication->execute()
#16 {main}

What application are you using? OJS 3.4

asmecher commented 8 months ago

@jonasraoni, is this mooted by https://github.com/pkp/pkp-lib/issues/9566?

jonasraoni commented 8 months ago

@asmecher Yeah, I remembered about this PR, but if it's worth adding a fix for 3.4, I can push something small.