mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-12 06:48:55 +00:00
ignacionelson
5fb17388cd
Reported by @Drescargot as GHSA-r3hg-3fxw-rcmr, in two halves. The groups listing never narrowed at all. Every other action in that controller is guarded with allowsGroupChange(), and index() — web and API alike — built a bare Group::query(), so a client-scoped staff member was shown every group on the installation with its name, description and member count. StaffLibraryScope::groups() is that narrowing, and assignableGroupIds() now reads from it rather than restating the same rule a second time, which is how the two drifted apart to begin with. The second half is the one that mattered. allowsGroupChange() asked only groupReachesNoFurther() — "is anything shared with this group outside my library" — which a group with nothing shared with it yet passes vacuously. So a scoped staff member could rename, delete or publish a group whose every member was somebody else's client. Publishing is the sharp end: whatever is shared with the group afterwards is reachable without signing in. The reporter suggested putting the membership check inside groupReachesNoFurther(). Tried, and it breaks two things. That predicate is shared with allowsGroupMembership(), where a group nobody has joined must stay usable so its creator can add the first member. And "every member must be mine" is the obvious reading of the rule and is wrong: it turns GHSA-whmp-p9hv-r7j7's narrowing — a mixed group's edit screen loads and simply does not name the stranger — back into a 404, undoing that fix. Four tests from it fail that way. So the check sits in allowsGroupChange() alone, and asks whether the group is wholly somebody else's rather than whether it is wholly theirs. A mixed group stays workable and is still covered by the reach check; an empty one stays nameable by whoever just made it; a group with members and none of them theirs is refused.