RustFund
First Flight #36
Beginner FriendlyRust
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

CEI Not Followed in Refund Function

0xblackadam

Summary

A critical vulnerability exists in the refund function of the crowdfunding contract due to violation of the Checks-Effects-Interactions (CEI) pattern, potentially exposing the contract to reentrancy attacks and creating significant security risks.

Vulnerability Details

The problematic code segment reveals fundamental CEI pattern violations:

pub fn refund(ctx: Context<FundRefund>) -> Result<()> {
let amount = ctx.accounts.contribution.amount;
if ctx.accounts.fund.deadline != 0 && ctx.accounts.fund.deadline > Clock::get().unwrap().unix_timestamp.try_into().unwrap() {
return Err(ErrorCode::DeadlineNotReached.into());
}
​
// Direct external call before state changes
**ctx.accounts.fund.to_account_info().try_borrow_mut_lamports()? =
ctx.accounts.fund.to_account_info().lamports()
.checked_sub(amount)
.ok_or(ProgramError::InsufficientFunds)?;
**ctx.accounts.contributor.to_account_info().try_borrow_mut_lamports()? =
ctx.accounts.contributor.to_account_info().lamports()
.checked_add(amount)
.ok_or(ErrorCode::CalculationOverflow)?;
​
// State update after external interactions
ctx.accounts.contribution.amount = 0;
​
Ok(())
}

Key technical issues include:

  • Performs external transfers before state modifications

  • Violates CEI (Checks-Effects-Interactions) pattern

  • Exposes contract to potential reentrancy attacks

  • Inconsistent state management

  • Lacks proper safeguards against recursive calls

Impact

The vulnerability creates severe consequences:

  • Potential unauthorized fund drainage

  • Risk of recursive call exploitation

  • Compromised contract state integrity

  • Possible financial losses

  • Security vulnerabilities in fund management

Recommendations

Immediate and comprehensive recommendations include:

  1. Implement Proper CEI Pattern

pub fn refund(ctx: Context<FundRefund>) -> Result<()> {
// Checks: Validate conditions first
if ctx.accounts.fund.deadline != 0 &&
ctx.accounts.fund.deadline > Clock::get().unwrap().unix_timestamp.try_into().unwrap() {
return Err(ErrorCode::DeadlineNotReached.into());
}
​
// Effects: Modify internal state before interactions
let amount = ctx.accounts.contribution.amount;
ctx.accounts.contribution.amount = 0;
​
​
// Interactions: External transfer as the last step
**ctx.accounts.contributor.to_account_info().try_borrow_mut_lamports()? =
ctx.accounts.contributor.to_account_info().lamports()
.checked_add(amount)
.ok_or(ErrorCode::CalculationOverflow)?;
​
Ok(())
}

    • Perform all state modifications before external calls

Tools Used

  • Manual code review

  • Static code analysis

Updates

Appeal created

bube Lead Judge 2 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[Invalid] Reentrancy in refund

The reentrancy risk on Solana is highly eliminated. The `try_borrow_mut_lamports` ensures that only one reference to an account exists at a time. Also, once the fund’s lamports are borrowed mutably, no other transaction can modify them until the borrow is released. This means the function will reset the `amount` before the next call.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.