Skip to content

fix: apply CEI pattern and reentrancy guard to release_payment#292

Merged
Leothosine merged 1 commit into
tosirano:mainfrom
wumibals:fix/payment-cei-reentrancy-guard
Jun 24, 2026
Merged

fix: apply CEI pattern and reentrancy guard to release_payment#292
Leothosine merged 1 commit into
tosirano:mainfrom
wumibals:fix/payment-cei-reentrancy-guard

Conversation

@wumibals

Copy link
Copy Markdown
Contributor

Summary

Fixes a Checks-Effects-Interactions (CEI) violation in contracts/payment/src/lib.rs where release_payment() performed two token transfers before updating payment.status to Released. A malicious token contract that re-entered release_payment() during transfer() could exploit this to drain the escrow twice.

Root cause

token_client.transfer(→ restaurant)   // INTERACTION (external call)
token_client.transfer(→ treasury)     // INTERACTION (external call)
payment.status = Released             // EFFECT — too late
storage.set(payment)                  // EFFECT — too late

Fix

contracts/payment/src/lib.rs

  1. Added Releasing(u64) variant to DataKey — a per-order reentrancy guard stored in instance storage
  2. Restructured release_payment to strict CEI order:
    • CHECKS — auth, status, caller validation (unchanged)
    • CHECKS — reentrancy guard: panics with "reentrant call" if Releasing(order_id) is already set
    • Set Releasing(order_id) = true
    • EFFECTSpayment.status = Released, settled_at, storage.set() — all before any external call
    • INTERACTIONS — token transfers to restaurant and treasury
    • Clear Releasing(order_id) guard
  3. All three existing tests (test_escrow_and_release, test_refund, test_double_escrow_panics) continue to pass
  4. Added test_release_status_is_released_after_successful_call — verifies CEI ordering
  5. Added test_double_release_panics_protecting_against_reentrancy — a second release_payment on the same order sees status=Released and panics, mirroring exactly what a reentrant call would encounter

Closes #281

@Leothosine Leothosine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work

@Leothosine Leothosine merged commit 30247e1 into tosirano:main Jun 24, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CO-03] Fix Checks-Effects-Interactions violation in payment contract release_payment()

2 participants