Project

General

Profile

Actions

Feature #65032

closed

tx_felogin: make 100% sure that password forgotten hash is unique [in all TYPO3 Versions]

Added by Michael Schramm over 9 years ago. Updated about 8 years ago.

Status:
Closed
Priority:
Must have
Assignee:
-
Category:
Security
Target version:
Start date:
2015-02-12
Due date:
% Done:

0%

Estimated time:
PHP Version:
Tags:
Complexity:
easy
Sprint Focus:

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));
Actions #1

Updated by Mathias Schreiber over 9 years ago

  • Category set to felogin
Actions #2

Updated by Benni Mack over 8 years ago

  • Target version changed from 7 LTS to 8 LTS
Actions #3

Updated by Helmut Hummel about 8 years ago

  • Category changed from felogin to Security
Actions #4

Updated by Wouter Wolters about 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.

Actions

Also available in: Atom PDF