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

Open
  • Is something wrong?
#742 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
75/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
typescript
Domain
api, backend, security

Research direction

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.

Written by the indexing model from the issue text.

Description

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.
Dominant language
TypeScript
Stars
2
Forks
90
Avg merge
31m
Merged PRs (30d)
20

Getting set up

  • Ships a Dockerfile or Docker Compose file
  • No pull request template
  • No contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from MergeFi/backend

All issues in MergeFi/backend

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.