Feature #65032
closedtx_felogin: make 100% sure that password forgotten hash is unique [in all TYPO3 Versions]
0%
Description
Dear Team,
Problem:
--------
There is a minimum risk to have two identical forgotten hashes in database and when calling it as link to get a foreign account for which you could set a new password. This exists in all Versions.
Located at:
-----------
tx_felogin_pi1::generateAndSendHash($user)
Description:
------------
There, is no check if the generated hash is already existing!
Yes, the range from md5 is wide and the risk of two identical hashes in database is very low, but this is not enough for an enterprise cms. And the possible solutions are simple.
Possible solutions:
-------------------
1. solution
Add a unique key on table fe_user.felogin_forgotHash
If no hash is set the field must be NULL
If the hash is generated and the update fails with because of an duplicate entry, generate a new hash and try again.
2. solution
chain user uid and generated hash.
Just change existing code
$hash = md5(t3lib_div::generateRandomBytes(64));
to this
$hash = $user['uid'] . md5(t3lib_div::generateRandomBytes(64));
Code snippet
------------
code located at
tx_felogin_pi1::generateAndSendHash($user)
$hours = intval($this->conf['forgotLinkHashValidTime']) > 0 ? intval($this->conf['forgotLinkHashValidTime']) : 24;
$validEnd = time() + 3600 * $hours;
$validEndString = date($this->conf['dateFormat'], $validEnd);
$hash = md5(t3lib_div::generateRandomBytes(64));
$randHash = $validEnd . '|' . $hash;
$randHashDB = $validEnd . '|' . md5($hash);
//write hash to DB
$res = $GLOBALS['TYPO3_DB']->exec_UPDATEquery('fe_users', 'uid=' . $user['uid'], array('felogin_forgotHash' => $randHashDB));
Updated by Benni Mack about 9 years ago
- Target version changed from 7 LTS to 8 LTS
Updated by Helmut Hummel over 8 years ago
- Category changed from felogin to Security
Updated by Wouter Wolters over 8 years ago
- Status changed from New to Closed
Discussed this at a codesprint and we don't see a problem here.
If there really is a chance to get the same key when generating a random string of 64 bytes we will have other problems.
Furthermore the hash is only valid for a given timeframe (by default 24 hours).
The hash is saved for the given user only, so the uid of the user is in the link which fetched the hash only for that user.
I close this issue now as we as team don't see a need to fix something in this code.
If you think we are wrong here don't hesitate to contact us in the coredev channel on Slack.