GHSA-hjx8-qv73-f7cm Vikunja: Webhooks and link shares survive every revocation path, so a removed collaborator keeps a live feed

• Go code.vikunja.io/api: >= 0.22.0, <= 2.6.0 (no fix yet)

Summary A collaborator removed from a project keeps a live, automatic feed of that project's contents, because nothing on any revocation path deletes the webhook they created while they had access. Vikunja already has a revocation-cleanup routine that deletes other derived rows for exactly this reason. Its set is `{task_assignees, subscriptions}`. `webhooks` and `link_shares` — the only two rows that carry a live channel into the project — are not in it, and the routine is wired to one of four revocation paths. Details `pkg/models/teams.go:411` — `cleanupTaskMembersAfterTeamRemoval`, added in `9358954c9`, whose commit message states the invariant: *"cleanup team memberships, assignments and subscriptions when users lose access to a project"*. ```go canRead, _, permErr := project.CanRead(s, &user.User{ID: memberID}) ... if !canRead { projectsToCleanup = append(projectsToCleanup, projectID) } ... _, err = s.In("task_id", taskIDs).And("user_id = ?", memberID). Delete(&TaskAssginee{}) _, err = s.In("entity_id", taskIDs). Where("entity_type = ? AND user_id = ?", SubscriptionEntityTask, memberID). Delete(&Subscription{}) _, err = s.In("entity_id", projectsToCleanup). Where("entity_type = ? AND user_id = ?", SubscriptionEntityProject, memberID). Delete(&Subscription{}) ``` So you have already decided that losing read access must delete rows the departing user left behind, and that the test is a live `project.CanRead`. The gap is which rows are in the set. And there is a second level, which is the sharper one. That routine has exactly one caller: ``` pkg/models/listeners.go:1642 err = cleanupTaskMembersAfterTeamRemoval(s, event.Team.ID, event.Member.ID) ``` against four revocation paths: ``` pkg/models/project_team.go:151 TeamProject.Delete pkg/models/project_users.go:141 ProjectUser.Delete <- grep -c cleanup inside: 0 pkg/models/project_users.go:248 ProjectUser.Update <- the downgrade path pkg/models/team_members.go:91 TeamMember.Delete <- the only one wired up ``` Removing a user directly from a project — the ordinary operation — dispatches nothing at all. PoC Released `vikunja/vikunja:2.6.0` Docker image. The harness was proved first by reproducing a known-fixed advisory: `GHSA-qfwc` refuses the read-only member and returns the hash to the owner. Every run carries its own negative control. The strongest form is on the team-removal path, where your cleanup routine does fire: ``` == OWNER removes collab from the TEAM == -- did the cleanup routine run? -- assignees now: [] <- POSITIVE CONTROL: it ran collab direct read of task: HTTP 403 <- NEGATIVE CONTROL: access is gone -- webhook still delivering? -- WEBHOOK RECEIVED task_title='post-team-removal secret' task_desc='cleanup ran, webhook did not' -- link share still redeemable? -- [{"title":"post-team-removal secret","description":"cleanup ran, webhook did not"}] ``` The two controls are the argument: your own routine executed and removed the assignee rows, and the collaborator's direct read is refused with 403 — and the webhook created before removal still delivers the contents of a task created *after* it. On the `ProjectUser.Delete` path the same thing happens with no cleanup running at all. One lab accommodation, stated plainly: non-routable outbound IPs were enabled so a loopback sink was reachable. In a default deployment an attacker simply uses a public URL, so this changes nothing about reachability. Impact A former collaborator receives task titles and descriptions for the whole project, continuously, after their access has been revoked — including content created after revocation. The honest counter-argument, which we would rather state than have you find. Both artefacts stay visible and auditable to the owner afterwards: `GET /projects/{p}/webhooks` still shows `{"created_by":"collab"}` and `GET /projects/{p}/shares` still shows `{"shared_by":"collab"}`. An administrator reviewing project settings after an offboarding will find them. That is genuinely weaker than a silent channel, and it argues for the low end of Medium. The precondition is also real: the attacker held write access, so they could have taken a snapshot before leaving. The only thing new here is access to content created after revocation — which is precisely the line you drew yourselves in `GHSA-jp29-jrxc-92vf`, *"favourites readable after revocation"*. We think it holds. It is also the entirety of the claim, and we are not dressing it up as more. Affected range All releases from `v0.22.0` through `v2.6.0`, and current `main`. Determined by checking the code at each released tag: ``` v0.22.0 … v0.24.6 webhooks.go=yes cleanupRoutine=0 v1.0.0 … v2.6.0 webhooks.go=yes cleanupRoutine=1 ``` Project webhooks arrive in `ad7d485eb` (2023-10-17), first released in v0.22.0. The cleanup routine arrives in `9358954c9` (2025-10-09), first released in v1.0.0, and has never included webhooks or link shares. Still present on `main` at `d822e1c58`: the routine contains only `Delete(&TaskAssginee{})` and two `Delete(&Subscription{})`, and `ProjectUser.Delete` contains zero dispatches. Reproduced at v2.6.0; earlier versions asserted from source rather than run, and we are saying which is which. Recommended Fix Two changes, and they are independent — the first is the smaller one, the second is the one that closes the class. Add the two row types to the cleanup set. Alongside the existing `task_assignees` and `subscriptions` deletions, delete `webhooks` and `link_shares` whose `created_by` / `shared_by` is the departing user and whose project is in `projectsToCleanup`. The existing live `project.CanRead` test is already the right predicate. Dispatch the cleanup from all four revocation paths, not only team-member removal. `ProjectUser.Delete` and `ProjectUser.Update` (the downgrade case) are the common operations and currently fire nothing; `TeamProject.Delete` unshares a whole project from a team and is equally a revocation. A cheaper alternative for the second half, if reworking dispatch is unattractive: check `CanRead` for the webhook's `created_by` at delivery time, and for the share's `shared_by` at redemption time. That converts a cleanup problem into an authorization check on a path that already has the user id to hand. Severity `CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:N/A:N` — 6.5 Medium. This is the identical vector you assigned to your own webhook advisory, `GHSA-7c2g-p23p-4jg3` (webhook BasicAuth credentials exposed to read-only members), copied rather than argued. `PR:L` because the attacker must have been provisioned with project write at some point; `C:H` because the channel carries full task titles and descriptions for the whole project continuously — the same `C:H` you assigned there for a credential leak of narrower scope. The auditability caveat above argues for the low end of Medium and we would not contest a lower score. The link share is strictly more capable, and we are not leading with it The same gap leaves a link share created by the departing collaborator redeemable after revocation, and a link share carries read and write — HTTP 201 demonstrated. On the same reasoning that would be `C:H/I:H` = 8.1 High. We are deliberately not proposing that, and leading with the webhook instead, because a link share has a real *"it is designed to be handed out"* defence and the webhook does not. That judgement is yours to make, and if you decide the share is the more serious half we will not argue.