DeFi Reentrancy Remediation

Overview

Related Detector: DeFi Reentrancy

DeFi reentrancy occurs when external calls (CPI, lamport transfers) execute before internal state is updated, allowing re-entrant calls to operate on stale data. The fix is to follow the checks-effects-interactions pattern: validate inputs first, update all state second, perform external calls last.

Recommended Fix

Before (Vulnerable)

pub fn withdraw(ctx: Context<Withdraw>, amount: u64) -> Result<()> {
    let vault = &ctx.accounts.vault;

    // Check
    require!(vault.user_balance >= amount, ErrorCode::InsufficientFunds);

    // Interaction BEFORE effect -- VULNERABLE
    let cpi_ctx = CpiContext::new(/* ... */);
    token::transfer(cpi_ctx, amount)?;

    // Effect after interaction -- stale during re-entry
    let vault = &mut ctx.accounts.vault;
    vault.user_balance -= amount;
    Ok(())
}

After (Fixed)

pub fn withdraw(ctx: Context<Withdraw>, amount: u64) -> Result<()> {
    let vault = &mut ctx.accounts.vault;

    // 1. CHECK: validate
    require!(vault.user_balance >= amount, ErrorCode::InsufficientFunds);

    // 2. EFFECT: update state FIRST
    vault.user_balance = vault.user_balance
        .checked_sub(amount)
        .ok_or(ErrorCode::ArithmeticError)?;

    // 3. INTERACTION: external call LAST
    let cpi_ctx = CpiContext::new(/* ... */);
    token::transfer(cpi_ctx, amount)?;
    Ok(())
}

Alternative Mitigations

1. Reentrancy guard

Add a mutex flag that prevents re-entry:

#[account]
pub struct VaultState {
    pub balance: u64,
    pub locked: bool,  // Reentrancy guard
}

pub fn withdraw(ctx: Context<Withdraw>, amount: u64) -> Result<()> {
    let vault = &mut ctx.accounts.vault;

    // GUARD: prevent re-entry
    require!(!vault.locked, ErrorCode::ReentrancyDetected);
    vault.locked = true;

    require!(vault.balance >= amount, ErrorCode::InsufficientFunds);
    vault.balance -= amount;

    // Transfer (even with re-entry, locked flag prevents second withdrawal)
    let cpi_ctx = CpiContext::new(/* ... */);
    token::transfer(cpi_ctx, amount)?;

    // Release guard
    vault.locked = false;
    Ok(())
}

2. PDA-based vault authority

Using a PDA as the vault authority limits which programs can initiate transfers. Combined with checks-effects-interactions, this provides defense in depth:

pub fn withdraw(ctx: Context<Withdraw>, amount: u64) -> Result<()> {
    let vault = &mut ctx.accounts.vault;
    vault.balance -= amount;  // State update first

    // PDA-signed transfer -- only this program can execute
    let seeds = &[b"vault", ctx.accounts.user.key.as_ref(), &[vault.bump]];
    let signer = &[&seeds[..]];
    let cpi_ctx = CpiContext::new_with_signer(/* ... */, signer);
    token::transfer(cpi_ctx, amount)?;
    Ok(())
}

3. Separate withdraw and claim instructions

Split the operation into two instructions that must be executed sequentially:

// Instruction 1: Record withdrawal intent (state update only)
pub fn request_withdrawal(ctx: Context<RequestWithdraw>, amount: u64) -> Result<()> {
    let vault = &mut ctx.accounts.vault;
    vault.balance -= amount;
    vault.pending_withdrawal = amount;
    Ok(())
}

// Instruction 2: Execute transfer (external call only)
pub fn execute_withdrawal(ctx: Context<ExecuteWithdraw>) -> Result<()> {
    let vault = &ctx.accounts.vault;
    let amount = vault.pending_withdrawal;
    require!(amount > 0, ErrorCode::NoPendingWithdrawal);

    let cpi_ctx = CpiContext::new(/* ... */);
    token::transfer(cpi_ctx, amount)?;

    let vault = &mut ctx.accounts.vault;
    vault.pending_withdrawal = 0;
    Ok(())
}

Common Mistakes

Mistake 1: Updating only part of the state before the external call

vault.user_balance -= amount;     // Updated
// WRONG: total_locked not updated yet
let cpi_ctx = CpiContext::new(/* ... */);
token::transfer(cpi_ctx, amount)?;
vault.total_locked -= amount;     // Stale during re-entry

Update all related state variables before any external call.

Mistake 2: Assuming Solana prevents all reentrancy

Solana prevents cross-transaction reentrancy, but CPI within a single transaction allows re-entry. A program can call another program, which calls back the original program.

Mistake 3: Reentrancy guard without release on error

vault.locked = true;
let result = token::transfer(cpi_ctx, amount);
if result.is_ok() {
    vault.locked = false;  // Not released on error!
}

Use a guard pattern that always releases, or rely on Solana’s transaction atomicity (state rolls back on error).

References

• Solana CPI Documentation
• Sealevel Attacks
• CWE-841: Improper Enforcement of Behavioral Workflow