Skip to content

EscrowService.refund()/release() check the caller's role but never bind the check to the specific escrow's sponsor — any SPONSOR can force-refund any other sponsor's escrow #299

Description

@abayomiwav

EscrowController.refund() (src/escrow/escrow.controller.ts) is gated by @Roles(UserRole.MAINTAINER, UserRole.SPONSOR) via RolesGuard. Reading src/auth/guards/roles.guard.ts, RolesGuard.canActivate only checks requiredRoles.some((role) => user.roles.includes(role)) — it has zero concept of which resource is being acted on. It never compares the authenticated caller's id to anything on the target Escrow.

EscrowService.refund(escrowId) (src/escrow/escrow.service.ts) takes only escrowId — no caller identity is threaded through at all — and unconditionally refunds to escrow.fundedByAddress, the original funder. So any authenticated user holding the SPONSOR role (not just the sponsor who actually funded this specific escrow, tracked on escrow.sponsorId) can call POST /escrow/:id/refund for any LOCKED escrow in the system and force it back to REFUNDED — cancelling a rival sponsor's active bounty/milestone escrow, even one where a contributor has already done the work and is waiting on release(). Funds don't go to the attacker (they return to the rightful original funder), so this is a griefing/availability attack rather than theft, but it lets one sponsor account unilaterally kill any other sponsor's in-flight payout.

This is distinct from #40 (DTOs trusting client-supplied recipient/funder ids) and #41 (TeamsService.assignToBounty missing an ownership guard on team assignment) — this is specifically the escrow refund/release authorization path lacking an escrow.sponsorId === callingUser.id (or equivalent maintainer-of-repo) check. Note that right now this is latent rather than actively exploitable, because (per the separate "nothing grants SPONSOR/MAINTAINER role" finding) every user is created with only roles: [UserRole.CONTRIBUTOR] and role assignment is currently unreachable through the app's own API. But the moment that's wired up (or a seed script/admin panel grants roles, which is exactly how local dev and e2e fixtures already assign roles), this gap becomes immediately live.

Fix: in EscrowService.refund/release, accept the authenticated caller's id/role from the controller and assert it matches escrow.sponsorId (for SPONSOR) or an appropriate maintainer-of-repository check (for MAINTAINER), not just "has a SPONSOR/MAINTAINER role somewhere."

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions