Rust Fund
AI First Flight #9
Beginner FriendlyRust
EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Manual Lamport Manipulation Instead of Safe Transfers

muzammilmohammed678

Root + Impact

Description

  • Describe the normal behavior in one or more sentences
    The `refund` and `withdraw` functions use direct lamport manipulation via `try_borrow_mut_lamports()` instead of using Anchor's safe transfer mechanisms or the system program's transfer instruction. While this approach works, it's more error-prone and doesn't follow Anchor best practices.

  • Explain the specific issue or problem in one or more sentences
    The normal behavior should use Anchor's built-in transfer mechanisms or CPI to the system program for safer, more auditable fund transfers. The current implementation manually manipulates lamports, which is more complex and harder to verify.

```rust:73-81:programs/rustfund/src/lib.rs
**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)?;
```
​
```rust:93-101:programs/rustfund/src/lib.rs
**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.creator.to_account_info().try_borrow_mut_lamports()? =
ctx.accounts.creator.to_account_info().lamports()
.checked_add(amount)
.ok_or(ErrorCode::CalculationOverflow)?;
```
​
Manual lamport manipulation requires careful handling of rent exemption and can lead to issues if not done correctly.

Risk

Likelihood:

  • * Issues occur when rent exemption calculations are incorrect

    * Problems arise if account balances are near rent exemption threshold

    * Risk increases with complex account state management

Impact:

  • * Potential for rent exemption violations

    * Harder to audit and verify correctness

    * More prone to off-by-one errors

    * Doesn't follow Anchor best practices

Proof of Concept

1. Fund account has exactly rent exemption amount + raised funds
2. Withdrawal attempts to subtract `amount_raised`
3. If calculation doesn't account for rent, account could become rent-exempt
4. Account could be closed, losing state
​

Recommended Mitigation

```diff
pub fn refund(ctx: Context<FundRefund>) -> Result<()> {
let amount = ctx.accounts.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());
}
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)?;
+ // Use system program transfer for safer fund movement
+ let cpi_context = CpiContext::new(
+ ctx.accounts.system_program.to_account_info(),
+ system_program::Transfer {
+ from: ctx.accounts.fund.to_account_info(),
+ to: ctx.accounts.contributor.to_account_info(),
+ },
+ );
+ system_program::transfer(cpi_context, amount)?;
​
ctx.accounts.contribution.amount = 0;
+ ctx.accounts.fund.amount_raised = ctx.accounts.fund.amount_raised
+ .checked_sub(amount)
+ .ok_or(ErrorCode::CalculationOverflow)?;
Ok(())
}
```
​
```diff
pub fn withdraw(ctx: Context<FundWithdraw>) -> Result<()> {
+ if ctx.accounts.fund.amount_raised < ctx.accounts.fund.goal {
+ return Err(ErrorCode::GoalNotMet.into());
+ }
+
let amount = ctx.accounts.fund.amount_raised;
+
+ if amount == 0 {
+ return Err(ErrorCode::AlreadyWithdrawn.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.creator.to_account_info().try_borrow_mut_lamports()? =
- ctx.accounts.creator.to_account_info().lamports()
- .checked_add(amount)
- .ok_or(ErrorCode::CalculationOverflow)?;
+ // Use system program transfer for safer fund movement
+ let cpi_context = CpiContext::new(
+ ctx.accounts.system_program.to_account_info(),
+ system_program::Transfer {
+ from: ctx.accounts.fund.to_account_info(),
+ to: ctx.accounts.creator.to_account_info(),
+ },
+ );
+ system_program::transfer(cpi_context, amount)?;
+ ctx.accounts.fund.amount_raised = 0;
Ok(())
}
```
​
**Note:** However, for `withdraw` and `refund`, the fund account is a PDA, so we need to use `invoke_signed` or ensure the fund account can sign. The current manual approach might be intentional for PDAs. A better approach would be:
​
```rust
// For refund - fund is a PDA, need to sign
let fund_seeds = &[
ctx.accounts.fund.name.as_bytes(),
ctx.accounts.fund.creator.as_ref(),
&[ctx.bumps.get("fund").unwrap()],
];
let fund_signer = &[&fund_seeds[..]];
​
let cpi_context = CpiContext::new_with_signer(
ctx.accounts.system_program.to_account_info(),
system_program::Transfer {
from: ctx.accounts.fund.to_account_info(),
to: ctx.accounts.contributor.to_account_info(),
},
fund_signer,
);
system_program::transfer(cpi_context, amount)?;
```
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 16 days ago
Submission Judgement Published
Validated
Assigned finding tags:

[L-03] Unsafe Direct Lamport Manipulation in refund(), withdraw() Functions

## Description The `refund` function in the provided code directly manipulates the lamports of accounts using `try_borrow_mut_lamports()`. This approach bypasses the Solana runtime's safety checks, leading to potential security vulnerabilities and program instability. ## Vulnerability Details In the `refund` function, lamports are transferred between accounts by directly adjusting their balances: &#x20; ```Rust **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)?; ``` This method of direct lamport manipulation can lead to several issues: 1. **Bypassing Rent Exemption Checks:** Accounts in Solana must maintain a minimum balance to be rent-exempt. Directly reducing an account's lamports without verifying rent exemption can result in the account being marked for deletion by the Solana runtime. 2. **Ownership Constraints:** Only the owning program of an account can modify its data and lamport balance. Direct manipulation without proper checks can violate these constraints, leading to program errors. 3. **Lack of Atomicity:** Direct lamport transfers lack the atomic transaction guarantees provided by the system program's transfer instruction, potentially leading to inconsistent states in case of program interruptions. ## Impact Exploiting this vulnerability can result in unauthorized fund transfers, violation of Solana's account ownership rules, and potential loss of funds due to accounts becoming non-rent-exempt. ## Recommendations Replace the direct lamport manipulation with Solana's system program transfer instruction to ensure safe and compliant fund transfers in refund() & withdraw() functions: &#x20; ```Rust let cpi_context = CpiContext::new( ctx.accounts.system_program.to_account_info(), system_program::Transfer { from: ctx.accounts.fund.to_account_info(), to: ctx.accounts.contributor.to_account_info(), }, ); system_program::transfer(cpi_context, amount)?; ``` This approach leverages Solana's native mechanisms for transferring lamports, ensuring adherence to the platform's safety and security protocols.

Support

FAQs

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

Give us feedback!