Vikunja: Webhooks and link shares survive every revocation path, so a removed collaborator keeps a live feed

Why it is Yellow

• CVSS 6.5 medium
• On your watchlist: Go

Affected packages

References

• https://github.com/advisories/GHSA-hjx8-qv73-f7cm
• https://github.com/go-vikunja/vikunja/security/advisories/GHSA-hjx8-qv73-f7cm
• https://github.com/advisories/GHSA-hjx8-qv73-f7cm

Original description

## 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` (t