Summary
The smart contract contains a Medium severity vulnerability related to unsafe handling of the Solana Clock sysvar. The contract uses the ? operator on Clock::get() in multiple functions, which can cause transaction failures instead of graceful error handling
Vulnerability Details
In the Rustfund contract, the Clock sysvar is accessed in several critical functions (contribute, refund, withdraw) using the ? operator for error propagation. If the Clock sysvar is unavailable for any reason, this will cause the entire transaction to fail rather than handling the error gracefully.
Affected code from programs/rustfund/src/lib.rs:
if fund.deadline != 0 && fund.deadline < Clock::get()?.unix_timestamp.try_into().unwrap() {
return Err(ErrorCode::DeadlineReached.into());
}
if ctx.accounts.fund.deadline != 0 && ctx.accounts.fund.deadline > Clock::get()?.unix_timestamp.try_into().unwrap() {
return Err(ErrorCode::DeadlineNotReached.into());
}
Impact
This vulnerability has medium severity because:
It can lead to unexpected transaction failures if the Clock sysvar is unavailable
It affects core functionality of the contract, including contributions and refunds
It represents poor error handling practices that could cause denial of service
POC
it("Uses unwrap on Clock sysvar which could lead to panic", async () => {
const clockFundName = "Clock Test Fund";
const [clockFundPDA] = await PublicKey.findProgramAddress(
[Buffer.from(clockFundName), creator.publicKey.toBuffer()],
program.programId
);
await program.methods
.fundCreate(clockFundName, description, goal)
.accounts({
fund: clockFundPDA,
creator: creator.publicKey,
systemProgram: anchor.web3.SystemProgram.programId,
})
.rpc();
const now = Math.floor(Date.now() / 1000);
const shortDeadline = new anchor.BN(now + 1);
await program.methods
.setDeadline(shortDeadline)
.accounts({
fund: clockFundPDA,
creator: creator.publicKey,
})
.rpc();
await new Promise((resolve) => setTimeout(resolve, 2000));
const clockContribution = new anchor.BN(10000000);
const [clockContribPDA] = await PublicKey.findProgramAddress(
[clockFundPDA.toBuffer(), provider.wallet.publicKey.toBuffer()],
program.programId
);
try {
await program.methods
.contribute(clockContribution)
.accounts({
fund: clockFundPDA,
contributor: provider.wallet.publicKey,
contribution: clockContribPDA,
systemProgram: anchor.web3.SystemProgram.programId,
})
.rpc();
} catch (e) {
console.log("Contribution failed due to Clock check as expected");
}
reportBug(
"MEDIUM",
"Unsafe Clock Usage",
"The program uses ? operator on Clock::get() without proper error handling, which could cause transaction failures",
`Deadline set to: ${shortDeadline.toString()} (${new Date(
shortDeadline.toNumber() * 1000
).toISOString()})\n` +
`Current time: ${now + 2} (${new Date(
(now + 2) * 1000
).toISOString()})\n\n` +
`In lib.rs:49-50, the contribute() function uses:\n` +
`\`\`\`rust\n` +
`let clock = Clock::get()?;\n` +
`require!(clock.unix_timestamp <= fund.deadline, RustfundError::DeadlinePassed);\n` +
`\`\`\`\n\n` +
`If Clock::get() fails, the ? operator will propagate the error directly, aborting the transaction.\n` +
`Same pattern appears in refund() and withdraw() functions.\n\n` +
`Recommendation: Use proper error handling for Clock::get() to ensure graceful failure.`
);
});
Output:
========================================
🐛 BUG REPORT [MEDIUM]: Unsafe Clock Usage
----------------------------------------
Description: The program uses ? operator on Clock::get() without proper error handling, which could cause transaction failures
Evidence: Deadline set to: 1716227141 (2024-05-20T18:52:21.000Z)
Current time: 1716227143 (2024-05-20T18:52:23.000Z)
In lib.rs:49-50, the contribute() function uses:
```rust
let clock = Clock::get()?;
require!(clock.unix_timestamp <= fund.deadline, RustfundError::DeadlinePassed);
If Clock::get() fails, the ? operator will propagate the error directly, aborting the transaction.
Same pattern appears in refund() and withdraw() functions.
Recommendation: Use proper error handling for Clock::get() to ensure graceful failure.
## Tools Used
- Manual code review
## Recommendations
Replace the ? operator with proper error handling:
```rust
let clock = Clock::get()?;
let clock = Clock::get().map_err(|_| ErrorCode::ClockUnavailable)?;
And add a new error code:
#[error_code]
pub enum ErrorCode {
#[msg("Clock sysvar is unavailable")]
ClockUnavailable,
}
