TeamsService.assignToBounty has no bounty-status or ownership guard, allowing payout hijack via last-second team assignment #41

Overview

TeamsService.assignToBounty has no restriction on which bounty it can be called against, and no check on the bounty's current state:

// src/teams/teams.service.ts:52-59
async assignToBounty(teamId: string, bountyId: string): Promise<Bounty> {
  await this.findOne(teamId); // ensures team exists
  const bounty = await this.bountyRepo.findOne({ where: { id: bountyId } });
  if (!bounty) throw new NotFoundException(`Bounty ${bountyId} not found`);
  bounty.teamId = teamId;
  return this.bountyRepo.save(bounty);
}

No status check, no ownership check (does the caller own this team? does the caller sponsor this bounty? is the caller even the claimant?), nothing. TeamsController.assign (teams.controller.ts:21-24) exposes this as POST /teams/:id/assign/:bountyId with no guard on top (see the companion "no auth at all" issue) — but even once that's fixed, this method's own logic has no business rule preventing misuse by an authenticated-but-unrelated caller.

The consequence is severe because of how BountiesService.markMergedAndRelease picks a payout path — team takes unconditional priority over the individual claimant:

// src/bounties/bounties.service.ts:101-130
if (bounty.teamId) {
  const team = await this.teamRepo.findOne({ where: { id: bounty.teamId }, relations: { splits: true } });
  if (team && team.splits.length > 0) {
    // ... splitRelease to every member's stellarAddress
  }
} else if (bounty.claimedById) {
  // ... release to the single claimant
}

Walk the attack end to end: a bounty is FUNDED, contributor X claims it (bounty.claimedById = X, bounty.teamId still null), opens a PR, and the PR is approved — the bounty is sitting in IN_REVIEW, one merge away from paying X. At any point before that merge webhook fires, anyone who can call assignToBounty (today: literally anyone, since there's no guard; after the companion auth fix: any authenticated user, since there's still no ownership check here) creates a team whose splits point at wallets they control, and calls POST /teams/:teamId/assign/:bountyId. The instant the PR merges, markMergedAndRelease sees bounty.teamId set, ignores claimedById entirely, and pays the attacker's team instead of X — X did the work, reviewed and merged; the attacker's team gets the funds. assignToBounty doesn't even require the bounty to be unclaimed, unfunded, or in any particular state — it works identically whether the bounty is OPEN, FUNDED, CLAIMED, IN_REVIEW, or even already MERGED (harmlessly a no-op at that point only because release already fired, or actively dangerous if it hasn't — see the companion "stuck MERGED bounty" issue for a state where a MERGED bounty's release hasn't happened yet and is retried later).

Requirements

• Add a status guard to assignToBounty: team assignment should only be permitted while the bounty is in a state where no individual has yet been committed to as the payout target — realistically OPEN or FUNDED, before claim() has run. Reject with a clear error once the bounty is CLAIMED or later.
• Add an ownership check once auth lands (companion issue): only the bounty's sponsorId (or a maintainer role) should be able to attach a team to it — not an arbitrary authenticated user, and certainly not the team's own creator alone (a malicious team creator shouldn't be able to self-assign their team to someone else's bounty).
• Consider whether claimedById and teamId should be mutually exclusive at the data-model level (e.g. clearing claimedById when a team is assigned, or rejecting team assignment if claimedById is already set) so the "which path does markMergedAndRelease take" decision isn't silently resolved by whichever field happens to be non-null with team taking undocumented precedence.
• Add tests covering: team assignment rejected once a bounty is claimed; team assignment rejected from a non-owning caller; the existing happy path (team assigned to an open/funded bounty before anyone claims it) continues to work.

Acceptance Criteria

• assignToBounty rejects attaching a team to a bounty that has already been claimed (or later) unless an explicit, separately-justified override path is added.
• assignToBounty requires the caller to own/sponsor the bounty (or hold a maintainer role) once authentication exists.
• A test demonstrates the full attack path described above is blocked: claim as X, attempt team-hijack before merge, assert rejection, merge, assert payout still goes to X.
• The precedence rule between teamId and claimedById in markMergedAndRelease is either made structurally impossible to misuse or explicitly documented and tested for every combination (neither set, only teamId, only claimedById, both set).

Additional Notes

Precise references:

• src/teams/teams.service.ts:52-59 — the vulnerable method.
• src/teams/teams.controller.ts:21-24 — the unguarded route.
• src/bounties/bounties.service.ts:101-130 — confirms team takes unconditional precedence over claimedById with no reconciliation.
• src/bounties/bounty-state-machine.ts:11-36 — confirms the bounty's own status transitions (OPEN → FUNDED → CLAIMED → IN_REVIEW → MERGED → PAID) are exactly what assignToBounty should be gated on, but currently isn't gated on at all — this is the one bounty-mutating write path in the entire codebase that doesn't go through assertTransition or any status check, despite changing something that directly determines where money goes.

Edge cases:

• A bounty can currently have both claimedById and teamId set simultaneously (nothing clears one when the other is set) — worth an explicit test matrix, not just the primary attack scenario, since the current code's behavior in that state (team silently wins) is itself surprising and worth locking down with a test regardless of the auth fix.
• If the fix clears claimedById when a team is assigned (one reasonable design), make sure that's not itself abusable to un-claim a bounty out from under a legitimate claimant by a non-owner — hence the ownership check being a hard requirement, not optional hardening.

Test/reproduction plan:

1. Create bounty, fund it, have user X claim it (claimedById = X).
2. As an unrelated authenticated user, create a team with attacker-controlled member addresses, call assignToBounty(team.id, bounty.id).
3. Pre-fix: succeeds, bounty.teamId set.
4. Merge the bounty's PR (markMergedAndRelease); pre-fix: escrow splits to the attacker's team, not X.
5. Post-fix: step 2 is rejected (either status-guard or ownership-guard, whichever lands) and step 4 still pays X directly.

Cross-references: compounds directly with the "no auth at all" issue (this method has zero protection today) and the "client-supplied identity" issue (same root pattern: nothing ties a mutation to who's actually allowed to make it). Also interacts with the "stuck MERGED bounty" issue — a bounty stuck in MERGED with a pending, not-yet-successful release is exactly the kind of window where a team-hijack attempt could still land before a retried release fires, once retries are added.