Vulnerability GHSA-hjx8-qv73-f7cm

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".

		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.