mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-16 16:45:07 +00:00
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
146 lines
5.6 KiB
PHP
146 lines
5.6 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Modules\Identity\Http\Controllers;
|
|
|
|
use App\Http\Controllers\Controller;
|
|
use App\Models\User;
|
|
use App\Modules\Audit\Action;
|
|
use App\Modules\Audit\ActivityLogger;
|
|
use App\Modules\Identity\FirstAdministrator;
|
|
use App\Modules\Identity\Models\Role;
|
|
use App\Modules\Identity\Permissions\SystemRole;
|
|
use App\Modules\Identity\UserType;
|
|
use App\Modules\Platform\Onboarding\InstallationWelcome;
|
|
use App\Modules\Platform\Settings\Setting;
|
|
use App\Modules\Platform\Settings\Settings;
|
|
use Illuminate\Http\RedirectResponse;
|
|
use Illuminate\Http\Request;
|
|
use Illuminate\Validation\Rules\Password;
|
|
use Inertia\Inertia;
|
|
use Inertia\Response;
|
|
|
|
/**
|
|
* First-run setup: create the initial staff administrator. Only reachable
|
|
* while no staff user exists; afterwards the routes bounce home.
|
|
*/
|
|
class SetupController extends Controller
|
|
{
|
|
public function __construct(
|
|
private readonly Settings $settings,
|
|
private readonly ActivityLogger $activity,
|
|
private readonly InstallationWelcome $welcome,
|
|
) {}
|
|
|
|
public function show(): Response|RedirectResponse
|
|
{
|
|
if ($this->setupIsComplete()) {
|
|
return redirect()->route('home');
|
|
}
|
|
|
|
return Inertia::render('setup');
|
|
}
|
|
|
|
public function store(Request $request): RedirectResponse
|
|
{
|
|
if ($this->setupIsComplete()) {
|
|
return redirect()->route('home');
|
|
}
|
|
|
|
$validated = $request->validate([
|
|
'site_name' => ['required', 'string', 'max:255'],
|
|
'name' => ['required', 'string', 'max:255'],
|
|
'email' => ['required', 'string', 'lowercase', 'email', 'max:255'],
|
|
'password' => ['required', 'confirmed', Password::defaults()],
|
|
]);
|
|
|
|
// The check above is not enough on its own: it is a plain read, and
|
|
// between it and the insert a second setup request can do the same
|
|
// read and insert an administrator of its own. Everything this
|
|
// request writes therefore happens inside the claim, so a request
|
|
// that loses the race writes nothing at all — not the site name
|
|
// either. See FirstAdministrator.
|
|
$admin = FirstAdministrator::claim(
|
|
fn (): bool => ! $this->setupIsComplete(),
|
|
function () use ($validated): User {
|
|
$this->settings->set(Setting::SiteName, $validated['site_name']);
|
|
|
|
$admin = User::create([
|
|
'type' => UserType::Staff,
|
|
'active' => true,
|
|
'role_id' => Role::query()->where('name', SystemRole::SystemAdministrator->value)->value('id'),
|
|
'name' => $validated['name'],
|
|
'email' => $validated['email'],
|
|
'password' => $validated['password'],
|
|
]);
|
|
|
|
// forceFill, not part of the create() array:
|
|
// email_verified_at is deliberately absent from
|
|
// User::$fillable, so mass assignment dropped it in silence
|
|
// and this account was never marked verified. The first
|
|
// administrator typed their own address into the form in
|
|
// front of them; there is nobody to confirm it to. (Inert
|
|
// today, since MustVerifyEmail is not enabled on the model,
|
|
// but the column is what a later switch would read.)
|
|
$admin->forceFill(['email_verified_at' => now()])->save();
|
|
|
|
return $admin;
|
|
},
|
|
);
|
|
|
|
// Somebody else finished setup while this request was in flight.
|
|
// Theirs is the administrator that exists; this one is sent to the
|
|
// login screen like any other visitor to an installed site.
|
|
if ($admin === null) {
|
|
return redirect()->route('home');
|
|
}
|
|
|
|
// v1 logged installation as action 0; setup is a recorded action.
|
|
$this->activity->log(Action::SetupCompleted, $admin);
|
|
$this->activity->log(Action::UserCreated, $admin, $admin);
|
|
|
|
if ($this->settings->get(Setting::AdminNotificationEmails) === []) {
|
|
$this->settings->set(Setting::AdminNotificationEmails, [$admin->email]);
|
|
}
|
|
|
|
// They will be shown around the first time they sign in — which is
|
|
// the next thing that happens, since setup deliberately does not
|
|
// log anybody in.
|
|
$this->welcome->raise();
|
|
|
|
// Deliberately no auto-login: the new administrator proves their
|
|
// credentials at the login form, which also confirms they work.
|
|
return redirect()->route('setup.success')->with('setup_completed', true);
|
|
}
|
|
|
|
public function success(Request $request): Response|RedirectResponse
|
|
{
|
|
if (! $this->setupIsComplete()) {
|
|
return redirect()->route('setup');
|
|
}
|
|
|
|
if (! $request->session()->get('setup_completed')) {
|
|
return redirect()->route('login');
|
|
}
|
|
|
|
return Inertia::render('setup-success');
|
|
}
|
|
|
|
/**
|
|
* Trashed staff count, for the reason EnsureSetupIsComplete gives:
|
|
* this asks whether the installation was ever set up, and store()
|
|
* below is the door a stranger walks through if the answer is wrong.
|
|
* The middleware and this must agree — one of them saying "not set
|
|
* up" while the other says "set up" is either a redirect loop or an
|
|
* open form.
|
|
*
|
|
* @phpstan-impure asking twice can honestly give two answers, which is
|
|
* the entire reason store() asks a second time under a lock
|
|
*/
|
|
private function setupIsComplete(): bool
|
|
{
|
|
return User::query()->withTrashed()->where('type', UserType::Staff)->exists();
|
|
}
|
|
}
|