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

オープン
  • 何か問題がありますか？
#742 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
3/5
見積もり時間
1〜2日
初心者へのやさしさ
75/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
typescript
領域
api, backend, security

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

説明

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.
主要言語
TypeScript
スター
2
フォーク
90
平均マージ
31分
マージ済み PR（30日）
20

環境構築

  • Dockerfile または Docker Compose ファイルあり
  • プルリクエストのテンプレートなし
  • コントリビューションガイドなし

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

MergeFi/backend のほかの issue

MergeFi/backend の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。