Reported by @skeletonsec as GHSA-w29w-pj29-x7ww.
Deleting an account that owns files makes the admin choose who inherits
them. The picker narrows that list for a client-scoped staff member to their
own roster, and says why two methods up: "a client-scoped staff member is
not shown the name of somebody they can reach nothing of, and a picker is no
more a reason to hand one over than a listing is."
The write asked something else entirely — exists, active, and not the
account being deleted. All three are true of every account on the
installation. So a scoped staffer could name an id the picker had
deliberately kept off the list, and a roster client's files and folders
landed with a client on somebody else's roster: readable, editable and
deletable there, because a client owns what they uploaded and
visibleToClient() includes uploaded_by.
The entry doors scope the source account and always did — guardTarget goes
through canAssignClient. It is the destination nobody scoped.
candidates() and validate() now run one predicate, reachableTargets(),
rather than two that happened to agree. Two that agree by inspection is what
this was: the narrowing existed, was correct, and was only ever applied to
the list.
The refusal deliberately reads as "no such account". An out-of-roster id and
an id belonging to nobody now produce the same message, because a refusal
that distinguishes them lets a scoped staffer walk the id space and learn
which accounts exist outside their roster. That is why Rule::exists is gone
rather than kept alongside: one code path, one answer. A test pins the two
messages as identical instead of naming either.
Both the web screen and the API twin come through this one validate(), so
both are fixed by it — and the test file proves each separately rather than
assuming the sharing holds.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CNFU55Tkq6MuEQ73nbbBRx
`reassign_candidates` is the delete dialog's picker: every active account
in the installation, by name and by role label. The same list is shared
on the clients index, the users index, both edit screens and privacy
settings, and it was narrowed by nothing.
Two lines above it on the clients index sits the listing itself, narrowed
through `scope->clients($viewer)` with a comment saying why: "a
client-scoped staff member is not shown the name and email of somebody
they can reach nothing of". The picker beside it handed over every client
in the installation, plus every staff account and its role name. The
filter by `can('delete_clients')` happens in React, which decides what is
rendered, not what is sent.
So the client half of the candidate list goes through the same
StaffLibraryScope as the listing, and each screen sends the picker only to
a viewer holding the delete permission it exists for. Staff accounts are
not narrowed -- they are not narrowed anywhere else either -- and an
unscoped viewer's list is unchanged, because StaffLibraryScope::clients()
returns every client for them.
Privacy settings keeps the whole installation on purpose: that picker sets
the erasure default stored once for everybody, behind edit_settings, so
narrowing it by whoever happens to be editing would store the wrong
answer. The parameter is nullable for that one caller, and the docblock
says so.
Four tests. Without the fix three go red; the fourth is the guard that an
administrator still sees every active account.
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