Root + Impact
Description
-
Describe the normal behavior in one or more sentences
The `refund` function does not validate that `contribution.amount > 0` before processing a refund. This means users who haven't contributed (or have already been refunded) can call the refund function, wasting compute units and potentially causing state inconsistencies.
-
Explain the specific issue or problem in one or more sentences
The normal behavior should verify that the contributor has a non-zero contribution amount before processing a refund. The current implementation reads the amount but doesn't validate it.
```rust:66-88:programs/rustfund/src/lib.rs
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());
}
**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)?;
ctx.accounts.contribution.amount = 0;
Ok(())
}
```
If `amount` is 0, the function will still execute the lamport transfers (transferring 0), reset the contribution amount, and consume compute units unnecessarily.
Risk
Likelihood:
-
* This occurs when users who haven't contributed call refund
* This occurs when users who were already refunded call refund again
* Spam/DoS vector for wasting compute units
Impact:
-
* Unnecessary compute unit consumption
* Potential for spam attacks
* State inconsistencies if function is called with 0 amount
* Wasted transaction fees for users
Proof of Concept
1. User creates contribution account but never contributes (amount = 0)
2. User calls `refund()` - function executes but transfers 0 SOL
3. Function resets `contribution.amount` to 0 (already 0)
4. Compute units wasted, no actual refund occurs
Recommended Mitigation
```diff
pub fn refund(ctx: Context<FundRefund>) -> Result<()> {
let amount = ctx.accounts.contribution.amount;
+
+ // Validate contribution amount
+ if amount == 0 {
+ return Err(ErrorCode::NoContribution.into());
+ }
+
if ctx.accounts.fund.deadline != 0 && ctx.accounts.fund.deadline > Clock::get().unwrap().unix_timestamp.try_into().unwrap() {
return Err(ErrorCode::DeadlineNotReached.into());
}
+
+ // Check if goal was not met
+ if ctx.accounts.fund.amount_raised >= ctx.accounts.fund.goal {
+ return Err(ErrorCode::GoalMet.into());
+ }
**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)?;
// Reset contribution amount after refund
ctx.accounts.contribution.amount = 0;
+ ctx.accounts.fund.amount_raised = ctx.accounts.fund.amount_raised
+ .checked_sub(amount)
+ .ok_or(ErrorCode::CalculationOverflow)?;
Ok(())
}
```
