Summary
Contributors should be able to get a refund when the deadline has passed but the goal is not met. However, 'contribution.amount' is never incremented in 'contribute()' despite successful SOL transfers. This leads to the 'refund()' function always refunding the contributors an amount of 0.
Vulnerability Details
The 'Contribution' struct tracks the contributor, the fund he has contributed to, and the amount he has contributed.
However, the amount is never incremented by the amount sent to the contribute() function.
refund() relies on the contribution.amount to determine the refund amount. This amount will always be 0 since it is never incremented anywhere in the code.
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());
}
...
Rest of function
...
*/
}
POC
The refund test provided by the devs did not correctly test this functionality.
In tests/rustfund.ts, replace the refund test with:
it("Refunds contribution", async () => {
const fundBalanceBefore = await provider.connection.getBalance(fundPDA);
const contributorBalanceBefore = await provider.connection.getBalance(provider.wallet.publicKey);
console.log("fundBalanceBefore", fundBalanceBefore);
console.log("contributorBalanceBefore", contributorBalanceBefore);
await new Promise(resolve => setTimeout(resolve, 15000));
await program.methods
.refund()
.accounts({
fund: fundPDA,
contribution: contributionPDA,
contributor: creator.publicKey,
systemProgram: anchor.web3.SystemProgram.programId,
})
.rpc();
const fundBalanceAfter = await provider.connection.getBalance(fundPDA);
const contributorBalanceAfter = await provider.connection.getBalance(provider.wallet.publicKey);
const contributionAccount = await program.account.contribution.fetch(contributionPDA);
console.log("fundBalanceAfter", fundBalanceAfter);
console.log("contributionAccount", contributionAccount);
console.log("contributorBalanceAfter", contributorBalanceAfter);
const fundBalanceDiff = fundBalanceBefore - fundBalanceAfter;
const contributorBalanceDiff = contributorBalanceAfter - contributorBalanceBefore;
console.log("fundBalanceDiff", fundBalanceDiff);
console.log("contributorBalanceDiff", contributorBalanceDiff);
const expectedRefundAmount = 500000000;
expect(contributionAccount.amount.toNumber()).to.equal(0, "Contribution amount should be reset to 0");
expect(fundBalanceDiff).to.equal(expectedRefundAmount, "Fund balance should decrease by the contributed amount");
const maxFeeTolerance = 10000;
expect(contributorBalanceDiff).to.be.closeTo(expectedRefundAmount, maxFeeTolerance, "Contributor balance should increase by the refunded amount minus fees");
});
Which fails:
4 passing (17s)
1 failing
1) rustfund
Refunds contribution:
Fund balance should decrease by the contributed amount
+ expected - actual
-0
+500000000
Impact
The contributor is not refunded the amount he is entitled to.
Tools Used
Manual review, anchor tests.
Recommendations
Increment the amount contributed in the contribution struct.
pub fn contribute(ctx: Context<FundContribute>, amount: u64) -> Result<()> {
let fund = &mut ctx.accounts.fund;
let contribution = &mut ctx.accounts.contribution;
if fund.deadline != 0 && fund.deadline < Clock::get().unwrap().unix_timestamp.try_into().unwrap() {
return Err(ErrorCode::DeadlineReached.into());
}
// Initialize or update contribution record
if contribution.contributor == Pubkey::default() {
contribution.contributor = ctx.accounts.contributor.key();
contribution.fund = fund.key();
contribution.amount = 0;
}
// Transfer SOL from contributor to fund account
let cpi_context = CpiContext::new(
ctx.accounts.system_program.to_account_info(),
system_program::Transfer {
from: ctx.accounts.contributor.to_account_info(),
to: fund.to_account_info(),
},
);
system_program::transfer(cpi_context, amount)?;
+ contribution.amount += amount;
fund.amount_raised += amount;
Ok(())
}
