You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
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
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."
I would like to take on and implement this issue: 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.
Technical Implementation Roadmap:
Formal Soroban Invariant Design: Audit storage layout (Instance, Persistent, Temporary) with strict TTL management and explicit require_auth guards.
Core Rust WASM Implementation: Implement deterministic state transition handlers eliminating arithmetic overflow risks using checked/saturating math.
Hermetic Testing Suite: Build comprehensive test cases in tests/ asserting authorized vs unauthorized invocations, mock auth environments, and edge value bounds.
Gas & Footprint Optimization: Profile CPU/memory instruction budgets under Soroban host environment to ensure optimal runtime efficiency.
Looking forward to delivering this robust implementation. Please assign whenever convenient!
Activity
Ranjeet2063 commentedon Sep 20, 2026
Hi team,
I would like to take on and implement this issue:
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.Technical Implementation Roadmap:
Instance,Persistent,Temporary) with strict TTL management and explicitrequire_authguards.tests/asserting authorized vs unauthorized invocations, mock auth environments, and edge value bounds.Looking forward to delivering this robust implementation. Please assign whenever convenient!
shobhamerabacha-star commentedon Oct 3, 2026
Hi,
I have experience with similar implementations and would like to take this on.
Proposed steps:
I can deliver a clean, tested PR within 24 hours. Please assign!
fix(escrow): assert caller authorization on refund and release (Merge…
fix(escrow): assert caller authorization on refund and release (Merge…
fix(escrow): assert caller authorization on refund and release (Merge…
fix(escrow): assert caller authorization on refund and release (Merge…