mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-16 16:45:07 +00:00
ignacionelson
83a8fe2288
Reported by @ry2811 as GHSA-w3w9-prpw-qx77, with a working two-worker reproducer. Setup asked the database whether any staff user existed, and created one some time later, in a separate statement with nothing joining the two. So two POSTs arriving together both read "no staff" and both inserted a System Administrator. Different addresses do not collide; `users.email` is the only unique key and it has nothing to say about there being one first administrator. The gap is not narrow. Between the check and the insert sits password hashing at BCRYPT_ROUNDS=12, which is slow on purpose, so the window is hundreds of milliseconds wide and observable without trying. What makes this worth fixing is not that a stranger can set up an unconfigured installation — first-run setup is open to whoever reaches it first, and always was. It is that racing the operator is *quiet*. The operator's own request also succeeds, also redirects to /setup/success, and the installation they get looks exactly like the one they expected. The second administrator is discovered later or not at all, and closing setup afterwards does not revoke it. FirstAdministrator::claim() makes it one operation. The row it locks is the System Administrator role, because the obvious candidate cannot work: there are no staff rows on a fresh install and a lock over an empty result serialises nothing. That role row is written by the roles migration and rewritten on every boot, so it is always there to be locked. The second caller waits on it, and by the time it has the lock the first caller's user is committed and visible to the re-check it then makes. Everything the request writes moved inside the claim, including the site name. A request that loses now writes nothing at all, rather than renaming the installation on its way to the login screen. `projectsend:admin --if-none` had the same shape and is fixed the same way — two containers coming up against one database is the version of this that needs no attacker. The early check stays where it is so an unattended boot does not prompt for a password it is about to discard; it is simply asked again under the lock. Both tests fail on the unfixed code. They stage the interleaving rather than attempting real concurrency, creating the winning administrator from a query listener after the request has made its first check — which is exactly the window, and the re-check is the only thing that closes it. The lock itself is invisible to them: the suite runs SQLite, where lockForUpdate() compiles to nothing. That half was verified against MySQL 8.4 by running the reporter's race for real, two processes through the full HTTP kernel: two administrators before, one after, repeatably. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CNFU55Tkq6MuEQ73nbbBRx
66 lines
2.6 KiB
PHP
66 lines
2.6 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Modules\Identity;
|
|
|
|
use App\Models\User;
|
|
use App\Modules\Identity\Models\Role;
|
|
use App\Modules\Identity\Permissions\EnsureSystemRoles;
|
|
use App\Modules\Identity\Permissions\SystemRole;
|
|
use Illuminate\Support\Facades\DB;
|
|
|
|
/**
|
|
* Creating the very first administrator is a claim, not a check followed
|
|
* by an insert.
|
|
*
|
|
* "Has this installation been set up" is answered by asking whether any
|
|
* staff row exists, and an empty result has nothing in it to lock. So two
|
|
* unauthenticated setup requests arriving together both read "no staff",
|
|
* both spend a quarter of a second hashing a password, and both insert a
|
|
* System Administrator. The operator's own setup succeeds and looks
|
|
* entirely normal, which is the point: a stranger walks away with a
|
|
* second, permanent administrator account and nothing says so.
|
|
* (GHSA-w3w9-prpw-qx77, reported by @ry2811.)
|
|
*
|
|
* What gets locked is the System Administrator role row. It is the thing
|
|
* being claimed; it is written by the roles migration and rewritten on
|
|
* every boot, so unlike the staff rows it is always there to be locked.
|
|
* The second caller waits on it, and by the time it has the lock the
|
|
* first caller's user row is committed and visible — so its own re-check,
|
|
* asked inside the claim this time, sees an installation that is already
|
|
* set up and creates nothing.
|
|
*
|
|
* Locking the staff query itself would not do. There are no matching rows
|
|
* on a fresh install, and a lock over nothing serialises nothing.
|
|
*/
|
|
final class FirstAdministrator
|
|
{
|
|
/**
|
|
* Create the initial administrator, or nothing if somebody else got
|
|
* there first.
|
|
*
|
|
* @param callable(): bool $stillNeeded asked again with the claim held
|
|
* @param callable(): User $create runs only if it is still needed
|
|
* @return User|null null when the claim was lost
|
|
*/
|
|
public static function claim(callable $stillNeeded, callable $create): ?User
|
|
{
|
|
// The lock needs a row to bite on. This is idempotent and already
|
|
// runs on every boot; asking again costs one query on the one
|
|
// request in the life of an installation that comes through here,
|
|
// and means a database somehow missing its roles gets them back
|
|
// rather than quietly racing.
|
|
(new EnsureSystemRoles)->ensure();
|
|
|
|
return DB::transaction(function () use ($stillNeeded, $create): ?User {
|
|
Role::query()
|
|
->where('name', SystemRole::SystemAdministrator->value)
|
|
->lockForUpdate()
|
|
->value('id');
|
|
|
|
return $stillNeeded() ? $create() : null;
|
|
});
|
|
}
|
|
}
|