@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Store the digest of the registration key, not the key itslef
Summary: Ref T1536. Like D6080, we don't need to store the registration key itself. This prevents a theoretical attacker who can read the database but not write to it from hijacking registrations.
Test Plan: Registered a new account.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6188
+10
-2
+3
-1
src/applications/auth/controller/PhabricatorAuthLoginController.php
+3
-1
src/applications/auth/controller/PhabricatorAuthLoginController.php
···
123
123
// key.
124
124
125
125
$registration_key = Filesystem::readRandomCharacters(32);
126
-
$account->setProperty('registrationKey', $registration_key);
126
+
$account->setProperty(
127
+
'registrationKey',
128
+
PhabricatorHash::digest($registration_key));
127
129
128
130
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
129
131
$account->save();
+7
-1
src/applications/auth/controller/PhabricatorAuthRegisterController.php
+7
-1
src/applications/auth/controller/PhabricatorAuthRegisterController.php
···
359
359
'Check that cookies are enabled and try again.'));
360
360
}
361
361
362
-
if ($registration_key != $account->getProperty('registrationKey')) {
362
+
// We store the digest of the key rather than the key itself to prevent a
363
+
// theoretical attacker with read-only access to the database from
364
+
// hijacking registration sessions.
365
+
366
+
$actual = $account->getProperty('registrationKey');
367
+
$expect = PhabricatorHash::digest($registration_key);
368
+
if ($actual !== $expect) {
363
369
return $this->renderError(
364
370
pht(
365
371
'Your browser submitted a different registration key than the one '.