Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Unexpected Withdrawal Processing in Deposit Function

soloking

Summary

The WithdrawalPool contract processes withdrawals during the deposit function instead of in the performUpkeep function as intended. This unexpected behavior could lead to gas inefficiencies, potential front-running opportunities, and confusion for users and integrators.

Vulnerability Details

The test results show that after queueing withdrawals and then calling the deposit function, all withdrawals were processed immediately:

it.only('should handle large number of withdrawals in performUpkeep', async () => {
const { accounts, withdrawalPool, token, stakingPool } = await loadFixture(deployFixture)
// Queue a large number of withdrawals
const withdrawalCount = 100;
const withdrawalAmount = toEther(10);
for (let i = 0; i < withdrawalCount; i++) {
await withdrawalPool.queueWithdrawal(accounts[0], withdrawalAmount);
}
// Check total queued withdrawals
const totalQueuedBefore = await withdrawalPool.getTotalQueuedWithdrawals();
console.log(`Total queued withdrawals before: ${fromEther(totalQueuedBefore)}`);
// Deposit enough tokens to cover all withdrawals
const depositAmount = ethers.parseEther((10 * withdrawalCount).toString());
await withdrawalPool.deposit(depositAmount);
// Increase time to allow for upkeep
await time.increase(86400);
// Check upkeep status
const [needsUpkeep, performData] = await withdrawalPool.checkUpkeep('0x');
console.log(`Needs upkeep: ${needsUpkeep}`);
if (needsUpkeep) {
try {
// Attempt to perform upkeep
const tx = await withdrawalPool.performUpkeep('0x');
const receipt = await tx.wait();
console.log(`Upkeep performed. Gas used: ${receipt?.gasUsed.toString()}`);
} catch (error) {
console.error('Error performing upkeep:', error);
throw error;
}
} else {
console.log('Upkeep not needed');
}
// Check remaining withdrawals
const remainingWithdrawals = await withdrawalPool.getTotalQueuedWithdrawals();
console.log(`Remaining withdrawals: ${fromEther(remainingWithdrawals)}`);
// Check token balances
const withdrawalPoolBalance = await token.balanceOf(withdrawalPool.target);
const stakingPoolBalance = await stakingPool.balanceOf(withdrawalPool.target);
console.log(`WithdrawalPool token balance: ${fromEther(withdrawalPoolBalance)}`);
console.log(`WithdrawalPool staking balance: ${fromEther(stakingPoolBalance)}`);
// Assert final state
expect(remainingWithdrawals).to.equal(0n, "Not all withdrawals were processed");
expect(withdrawalPoolBalance).to.equal(depositAmount, "Incorrect token balance in WithdrawalPool");
expect(stakingPoolBalance).to.equal(0n, "Incorrect staking balance in WithdrawalPool");
});

  1. 100 withdrawals of 10 tokens each were queued, totaling 1000 tokens.

  2. After depositing 1000 tokens, the remaining withdrawals became 0.

  3. The checkUpkeep function returned false, indicating no upkeep was needed.

  4. The WithdrawalPool token balance was 1000, matching the total amount of queued withdrawals.

This behavior suggests that the deposit function is processing withdrawals, which is not the expected functionality based on the contract's design.

Impact

  1. Gas Inefficiency: Processing withdrawals in the deposit function could lead to unexpectedly high gas costs for users making deposits.

  2. Front-running: Malicious actors could potentially front-run large deposits to benefit from immediate withdrawal processing.

  3. Inconsistent State: The contract's state might become inconsistent with its intended design, leading to difficulties in maintenance and upgrades.

  4. User Confusion: Users expecting withdrawals to be processed periodically might be confused by immediate processing.

Tools Used

Manual , Hardhat

Recommendations

  • Review and refactor the deposit function to ensure it doesn't process withdrawals.

  • Implement withdrawal processing exclusively in the performUpkeep function.

  • Add events to clearly log when and where withdrawals are being processed.

  • Implement additional checks in the checkUpkeep function to correctly identify when upkeep is needed.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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