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
Reported by @choewonwoo1817 as GHSA-wgxf-v8cr-37mj, with a working
end-to-end reproducer against Keycloak.
`where('email', $address)` is not an exact match. It is whatever the
database says equality means, and the collation INSTALL.md tells people to
create — utf8mb4_unicode_ci — folds accents:
administrator@example.com = administrator@éxample.com -> 1
Those are two different domains. The second is xn--xample-9ua.com, which
somebody else can register and honestly verify at an OIDC provider. So an
attacker with no account here could sign in as themselves and be handed
the first account: SocialAuthenticator found it, linked their subject to
it permanently, and started a session. No password, no interaction from
the owner, an administrator session where that account was one.
Comparison now happens in PHP, in one place, on every driver. Case is
still folded because that is a real requirement — addresses are stored
lowercased and a provider may send any case — and mb_strtolower folds case
without folding accents, which is exactly the line to draw.
Three call sites move to it and two deliberately do not. Loose matching is
right when *refusing* and wrong when *selecting*: AvailableEmailRule and
ClientProvisioning ask "is this address free", where a collation that says
no to a near-miss refuses more registrations, which is the safe direction.
The three that ask "which account is this" are the social path, the login
form (where a password still gated it, so it was confusion rather than
takeover) and the erasure command (irreversible, and the wrong row is the
wrong person).
The test story is the part worth reading. The suite runs on SQLite, whose
`=` is byte-exact, so this defect does not exist there and never did —
which is how it survived six releases with everything green. A test
written the obvious way passes on unfixed code. So the comparison is
pinned by driver-independent tests that always run, and the chain is
proved by AccountLookupCollationTest, which skips unless the connection is
MySQL and carries the command to run it. Run against real MySQL with the
real collation: it fails on the old code and passes on the new.
The last two paths that passed email_verified_at into User::create() and
lost it: the setup screen, and projectsend:admin for a container that
comes up from environment variables. It is deliberately absent from
$fillable, so mass assignment drops it without a word, and both meant to
set it.
The intent is plain in both cases — the first administrator typed their
own address into the form in front of them, and whoever provisioned the
container supplied it themselves. 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: turning verification on would
have locked out the one account that cannot be helped by another
administrator.
Both are now pinned by a test that fails when the forceFill is removed.
StaffAccounts had already fixed this for staff and named the rest; with
client accounts done earlier today, that list is empty.
Also says on User::$fillable what absence from it buys and what it does
not. It stops a request smuggling a value in; it does not tell code that
meant to set the value that it failed. Four separate paths made the same
mistake against the same comment.
An account deleted by an administrator was soft-deleted with erase_after
null, so projectsend:purge-erasures — which filters on
whereNotNull('erase_after') — never reached it, and the unique index on
users.email kept the address reserved forever. Anyone re-creating the
account got "The email has already been taken", naming a conflict nothing
on any screen could show or clear (#1648).
Both halves of the issue's option 3:
Every deletion path now schedules the erasure. The stamp lives in
ErasureSchedule — self-deletion switched to it, and StaffAccounts::delete
(shared by the web screen and the API) and both client controllers call
it right before delete(). Same grace period, same purge, whoever deleted
the account. Deliberately no backfill for rows deleted before this
change: stamping them during an update would start a countdown to data
erasure that nobody chose at deletion time; the message below covers
them instead.
The staff creation paths swap unique:users,email for AvailableEmailRule,
which refuses exactly the same things but can explain the one refusal
the stock message can't: an address held by a deleted account now names
the date it becomes available, and one deleted before scheduling existed
points at projectsend:erase-account. A living account keeps the stock
message, and public registration keeps the stock rule — telling an
anonymous visitor the address belongs to a deleted account would confirm
it had an account here.
Setup ended by handing somebody a login form and an empty dashboard.
Everything this application can do was one menu away, and which menu was
theirs to discover.
The first time the administrator signs in to a new installation they now
land on a short ordered list of what is worth doing first — add a client,
upload a file, group the people who get the same things, choose how the
file lists and the email look, point it at a mail server, add the team,
check the scheduler — each a link straight to the screen that does it.
The list is filtered twice, and both filters matter. By permission,
because a link that answers 403 is worse than no link. And by edition:
a managed installation is not sent off to configure a mail server
somebody else runs, to create staff accounts that are not its to create,
or to check a scheduler it does not host. Those three drop out on Cloud
and the other five remain.
Two steps tick themselves, because the database can answer them: a client
exists, a file exists. Nothing else is checkable without guessing — a
theme that was never changed looks exactly like one chosen deliberately —
and a tick meaning "we assume so" is worse than no tick.
The invitation to the Discord is at the very bottom, after the list.
Somebody who has just installed this came with a job in mind, and opening
with a social invitation is the fastest way to lose them.
The marker is raised where a first administrator comes into existence —
the setup screen and `projectsend:admin`, so a container provisioned from
environment variables is welcomed too — and it is false by default, so an
installation that updates into this feature is not congratulated on an
install it finished a year ago.
RedirectToWhatsNew becomes RedirectToGreeting and answers for both: they
are the same interruption, and a second middleware on the same route
would have to know about the first to avoid arguing with it. Installing
wins; release notes for a version you never ran are the wrong greeting.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Client file sharing, rebuilt from the ground up: a private area per
client, resumable uploads, folders, groups and categories, sharing with
expiry dates and download limits, comments, file versions, an activity
log, a REST API, and sixteen languages.
This repository begins here. ProjectSend 2 was developed privately, and
that development history is not published — the previous generation
remains available, with its own history, at projectsend/legacy.
Free software under the GNU General Public License v2, or (at your
option) any later version.
ignacionelson