[Regression on closed #273] assignReward's double-payout guard is keyed on (pool, recipient) instead of (pool, issue) — the same issue can still be paid twice

Aperta
  • Qualcosa non va?
#458 5 commenti 0 reazioni 1 assegnatario Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@MrOmale ci sta già lavorando.

Dal 21/10/2026.

  • #472 di @Proxima84-code — aperta
  • #473 di @Ultra-Tech-code — aperta
  • #479 di @PINYOPATTANAWASANPORN — aperta
  • #715 di @MrOmale — aperta

Valutazione

Questa issue non è ancora stata valutata.

Descrizione

bug security Stellar Wave very hard

Problem

#273 ("no guard against being called twice for the same issueId") was closed with this check in MaintenancePoolService.assignReward():

// Guard against double/triple payout for the same issue (#273).
const existingPayment = await this.paymentRepo.findOne({
  where: { recipientId: recipientId ?? null },
});
const existingPoolPayment = await this.paymentRepo
  .createQueryBuilder('payment')
  .innerJoin('payment.escrow', 'escrow')
  .where('escrow.maintenancePoolId = :poolId', { poolId: id })
  .andWhere('payment.recipientId = :recipientId', { recipientId: recipientId ?? null })
  .getOne();
if (existingPoolPayment) {
  throw new ConflictException(`Issue ${issueId} has already received a reward from pool ${id}`);
}

issueId doesn't appear in either query, and Payment has no issue column at all. The guard checks who was paid from this pool, not which issue was rewarded:

  1. The same issue can still be paid twice. Call assignReward(pool, issue#1, …, recipientA) and then assignReward(pool, issue#1, …, recipientB), and both succeed. This is the double payout #273 asked to prevent.
  2. Anonymous payouts are never guarded. When recipientId is omitted, the query becomes payment.recipientId = NULL, which is never true in SQL. So repeated rewards for the same issue with no recipientId always pass.
  3. Legitimate rewards are rejected. A maintainer who fixes two different maintenance issues gets a 409 on the second one, with a misleading message saying that issue was already rewarded.
  4. existingPayment is dead code. It's an unscoped query across every payment in the system, whose result is never read, and it costs an extra DB round trip per call.

Suggested fix

  • Record the rewarded issue on the payment, for example a nullable Payment.maintenanceIssueId (FK to issues), plus a unique index on (escrowId, maintenanceIssueId).
  • Guard on (pool escrow, issueId), and let the unique index close the concurrent-call race.
  • Remove the unused existingPayment query.
  • Add tests: the same issue with two recipients gets a 409, and two issues for the same recipient both succeed.

Related: #273, #274, #276.

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.