CreateTeamDto.createdById is taken from the request body, so a caller can create a team owned by someone else

Aperta
  • Qualcosa non va?
#742 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
75/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
typescript
Ambito
api, backend, security

Direzione di ricerca

Start with TeamsService.create in src/teams/teams.service.ts, then read CreateTeamDto and the controller in the paths named in the issue. Check the existing team service specs and how member IDs are validated. Done means a request cannot choose an arbitrary team owner, and a service spec covers a body-supplied createdById that differs from the caller.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug security

Location — src/teams/teams.service.ts:38-43 (TeamsService.create), DTO field src/teams/dto/create-team.dto.ts:49-52 (CreateTeamDto.createdById), controller src/teams/teams.controller.ts:43.

Problem — The authenticated caller's id is used only as a fallback for a client-supplied value:

const team = await this.teamRepo.save(
  this.teamRepo.create({
    name: dto.name?.trim() ?? dto.name,
    // Fall back to the authenticated caller's id when the client omits createdById.
    createdById: dto.createdById ?? callerUserId,
  }),
);

CreateTeamDto.createdById is validated as @IsOptional() @IsUUID() and nothing else. There is no check that it equals user.userId from @CurrentUser(), and no check that the referenced user even exists (the existence loop above only validates members[].userId, not createdById). So any caller holding SPONSOR or MAINTAINER can set createdById to an arbitrary UUID.

Why it matters — Team.createdById is the only ownership field on the entity. Issue #357's fix made it the fallback owner precisely so a team cannot be created with no owner, and it is the field any ownership check on PATCH /teams/:id/splits or POST /teams/:id/assign/:bountyId must key on. Letting the client choose it means:

  • The audit trail is wrong: a team whose splits pay out real money is attributed to a user who never created it, and the real creator leaves no record anywhere.
  • It pre-breaks the ownership checks this field exists to support. Once a check of the form team.createdById === callerId is added, a caller can still hand ownership to an account they control, or — in the other direction — plant a team attributed to a victim so the victim appears responsible for its payout terms.
  • Passing an unknown UUID is also accepted; because Team.createdBy is onDelete: 'SET NULL' / nullable: true, there is no FK rejection path that reliably catches it at insert time for an id that does not exist, so the team can end up with a dangling owner id (the condition #357 set out to prevent).

Reproduction / failure path

  1. Authenticate as user A (SPONSOR). Note A's id.
  2. POST /teams with body {"name":"t","createdById":"<user B's uuid>","members":[{"userId":"<A>","percentage":100}]}.
  3. Returns 201 with createdById set to B. Nothing links the team to A.
  4. Repeat with a random UUID that belongs to no user — also accepted.

Suggested fix

  • Remove createdById from CreateTeamDto entirely and set createdById: callerUserId unconditionally in the service. The caller id is already available and is the only trustworthy source.
  • If an admin-on-behalf-of flow is genuinely needed, keep the field but accept it only when the caller holds UserRole.MAINTAINER, and validate the referenced user exists with the same batched existence check used for members; otherwise reject with ForbiddenException.
  • Add a service spec asserting that a body-supplied createdById different from the caller's id does not end up on the saved entity.
Lingua principale
TypeScript
Stelle
2
Fork
90
Merge medio
31m
PR unite (30g)
20

Preparare l'ambiente

  • Include un Dockerfile o un file Docker Compose
  • Nessun modello di pull request
  • Nessuna guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di MergeFi/backend

Tutte le issue di MergeFi/backend

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.