Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions contracts/split/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ pub enum ContractError {
MemoMismatch = 33,
/// Issue #439: Creator is in cooldown after cancelling an invoice.
CreatorCooldownActive = 31,
/// Reentrant call detected: a fund-moving function was invoked recursively
/// within the same transaction. Cleared automatically at transaction boundary
/// because the lock lives in temporary storage.
ReentrantCall = 33,
/// RBAC: Caller does not hold the required role for this entry point.
RoleNotHeld = 33,
/// Issue #482: Intermediate multiplication or division overflowed i128 bounds.
Expand Down
82 changes: 82 additions & 0 deletions contracts/split/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,54 @@ fn refunded_key(invoice_id: u64) -> (Symbol, u64) {
(symbol_short!("refunded"), invoice_id)
}

// ---------------------------------------------------------------------------
// Reentrancy guard (issue #451-reentrancy)
// ---------------------------------------------------------------------------

/// Temporary-storage key for the per-transaction reentrancy lock.
///
/// Using *temporary* storage means the flag is automatically invalidated at the
/// end of the transaction (its TTL is never extended), so a stale lock can never
/// block a subsequent independent call.
fn reentrancy_lock_key() -> Symbol {
symbol_short!("re_lock")
}

/// Executes `body` inside a reentrancy guard backed by temporary storage.
///
/// # How it works
/// 1. Check whether the lock key is present in temporary storage. If it is,
/// a recursive call is in progress — return `ReentrantCall` immediately.
/// 2. Set the lock (TTL = 1 ledger; only needs to survive this transaction).
/// 3. Run `body`.
/// 4. Remove the lock so that another *independent* call in the same ledger can
/// still proceed (Soroban executes each top-level invocation as its own
/// transaction, but this is belt-and-suspenders).
///
/// The lock lives in `env.storage().temporary()` so it is **never persisted
/// across transactions** even if the `remove` step is somehow skipped.
fn with_reentrancy_guard<F>(env: &Env, body: F) -> Result<(), ContractError>
where
F: FnOnce() -> Result<(), ContractError>,
{
let key = reentrancy_lock_key();
if env
.storage()
.temporary()
.has(&key)
{
return Err(ContractError::ReentrantCall);
}
// Set the lock with the minimum TTL. The value is irrelevant; presence is
// all we test.
env.storage().temporary().set(&key, &true);
let result = body();
// Always clear the lock so subsequent independent calls within the same
// ledger (different top-level transactions) are not blocked.
env.storage().temporary().remove(&key);
result
}

fn maybe_record_created(env: &Env, creator: &Address, total: i128) {
if let Some(dashboard) = env
.storage()
Expand Down Expand Up @@ -6401,6 +6449,20 @@ impl SplitContract {
/// For tranche invoices, only distributes tranches whose timestamp ≤ now.
/// Blocks with "prerequisite not released" until the prerequisite invoice is Released.
/// If an approver is set, requires the invoice to be approved first (issue #25).
pub fn release_invoice(
env: Env,
caller: Address,
invoice_id: u64,
preimage: Option<Bytes>,
) {
// --- Reentrancy guard (issue #451-reentrancy) ---
// Uses temporary storage so the lock is never persisted across transactions.
let re_key = reentrancy_lock_key();
if env.storage().temporary().has(&re_key) {
panic!("{}", ContractError::ReentrantCall as u32);
}
env.storage().temporary().set(&re_key, &true);
// ------------------------------------------------
pub fn release_invoice(env: Env, caller: Address, invoice_id: u64, preimage: Option<Bytes>) {
require_fn_not_paused(&env, &symbol_short!("release"));
require_not_frozen(&env);
Expand Down Expand Up @@ -6544,6 +6606,8 @@ impl SplitContract {
}

Self::_release(&env, invoice_id, &mut invoice, &caller);
// Clear reentrancy lock on normal exit.
env.storage().temporary().remove(&reentrancy_lock_key());
}

/// Backwards-compatible release entry point.
Expand Down Expand Up @@ -8984,6 +9048,13 @@ impl SplitContract {

/// Refund all payers after the invoice has been marked expired.
pub fn refund(env: Env, invoice_id: u64) {
// --- Reentrancy guard (issue #451-reentrancy) ---
let re_key = reentrancy_lock_key();
if env.storage().temporary().has(&re_key) {
panic!("{}", ContractError::ReentrantCall as u32);
}
env.storage().temporary().set(&re_key, &true);
// ------------------------------------------------
require_fn_not_paused(&env, &symbol_short!("refund"));
let mut invoice = load_invoice(&env, invoice_id);

Expand Down Expand Up @@ -9108,6 +9179,8 @@ impl SplitContract {
.checked_add(1)
.expect("creator_refunded overflow"),
);
// Clear reentrancy lock on normal exit.
env.storage().temporary().remove(&reentrancy_lock_key());
}

/// Backwards-compatible alias for the expiry-driven refund path.
Expand Down Expand Up @@ -9533,6 +9606,13 @@ impl SplitContract {
/// Cancel an invoice. Refunds any payments already made.
/// Issue #89: If stake exists, distributes it equally among unique payers.
pub fn cancel_invoice(env: Env, caller: Address, invoice_id: u64) {
// --- Reentrancy guard (issue #451-reentrancy) ---
let re_key = reentrancy_lock_key();
if env.storage().temporary().has(&re_key) {
panic!("{}", ContractError::ReentrantCall as u32);
}
env.storage().temporary().set(&re_key, &true);
// ------------------------------------------------
require_not_paused(&env);
caller.require_auth();

Expand Down Expand Up @@ -9712,6 +9792,8 @@ impl SplitContract {
.set(&creator_cooldown_key(&invoice.creator), &until_ledger);
events::creator_cooldown_set(&env, &invoice.creator, until_ledger, cooldown_ledgers);
}
// Clear reentrancy lock on normal exit.
env.storage().temporary().remove(&reentrancy_lock_key());
}

/// Transfer invoice ownership to a new creator.
Expand Down
Loading