search

ulsdevteam / pkp-betterPassword

Plugin to implement password requirements for PKP OJS/OMP
GNU General Public License v2.0
7 stars 6 forks source link

Prevent leaking of user existence #1

Closed ctgraham closed 3 years ago

ctgraham commented 5 years ago

Because failure counts are based on user properties, the existence of a username is indicated by the transition to a locked account status. Best practices would obscure this so that usernames could not be confirmed by repeated login attempts.

https://github.com/ulsdevteam/pkp-betterPassword/blob/43dd2e61a30cea7bc9e08b48afb41669274b3488/BetterPasswordPlugin.inc.php#L234 https://github.com/ulsdevteam/pkp-betterPassword/blob/43dd2e61a30cea7bc9e08b48afb41669274b3488/BetterPasswordPlugin.inc.php#L266

ctgraham commented 4 years ago

Resolved by #9.

ctgraham commented 4 years ago

Missed conversion to BadpwFailedLoginsDAO: https://github.com/ulsdevteam/pkp-betterPassword/blob/59220f680b06625c1f11d3f1132924dbfc75682c/BetterPasswordPlugin.inc.php#L351-L357

This will also orphan the user settings here: https://github.com/ulsdevteam/pkp-betterPassword/blob/59220f680b06625c1f11d3f1132924dbfc75682c/BetterPasswordPlugin.inc.php#L373-L385

ctgraham commented 4 years ago

Saw this error in the wild when attempting to use 59220f6 on stable-3_2_1.

[Mon Nov 23 17:03:08.741836 2020] [php7:notice] [pid 4020] [client 10.195.106.146:53449] PHP Fatal error:  Uncaught Exception: DB Error: Duplicate entry '' for key 'PRIMARY' Query: INSERT INTO badpw_failedlogins(username,count,failed_login_time)VALUES (?,?,?) in  html/lib/pkp/classes/db/DAO.inc.php:703\nStack trace:\n#0  html/lib/pkp/classes/db/DAO.inc.php(231): DAO->handleError(Object(ADODB_mysqli), 'INSERT INTO bad...')\n#1  html/plugins/generic/betterPassword/classes/BadpwFailedLoginsDAO.inc.php(23): DAO->update('INSERT INTO bad...', Array)\n#2  html/plugins/generic/betterPassword/classes/BadpwFailedLoginsDAO.inc.php(61): BadpwFailedLoginsDAO->insertObject(Object(BadpwFailedLogins))\n#3  html/plugins/generic/betterPassword/BetterPasswordPlugin.inc.php(306): BadpwFailedLoginsDAO->getByUsername(NULL)\n#4  html/lib/pkp/classes/plugins/HookRegistry.inc.php(107): betterPasswordPlugin->handleTemplateDisplay('TemplateManage in  html/lib/pkp/classes/db/DAO.inc.php on line 703
ctgraham commented 3 years ago

Resolved here: https://github.com/ulsdevteam/pkp-betterPassword/blob/master/features/LimitRetry.inc.php