[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

Abierto
  • ¿Algo no está bien?
#458 5 comentarios 0 reacciones 1 asignado Ver en GitHub

Los mantenedores suelen responder en 1 día

@MrOmale ya está trabajando en esto.

Desde el 21/10/2026.

  • #472 de @Proxima84-code — abierto
  • #473 de @Ultra-Tech-code — abierto
  • #479 de @PINYOPATTANAWASANPORN — abierto
  • #715 de @MrOmale — abierto

Evaluación

Este issue todavía no se ha evaluado.

Descripción

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.

Lenguaje dominante
TypeScript
Estrellas
2
Forks
90
Merge medio
31 min
PR fusionados (30 d)
20

Preparar el entorno

  • Incluye un Dockerfile o un archivo de Docker Compose
  • Sin plantilla de pull request
  • Sin guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de MergeFi/backend

Todos los issues de MergeFi/backend

Issues similares

Más issues de TypeScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.