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

PriorityPool.sol :: claimLSDTokens() if is called with a second claim that is less than the first, the transaction will always revert, making it impossible for the user to claim their tokens.

ivanfitro

Summary

The claimLSDTokens() allows users to claim their withdrawable liquid staking tokens. However, if the second claim is smaller than the first, the transaction will always revert due to an underflow error, preventing the user from claiming their LSD tokens.

Vulnerability Details

claimLSDTokens() is implemented as follows:

function claimLSDTokens(
uint256 _amount,
uint256 _sharesAmount,
bytes32[] calldata _merkleProof
) external {
address account = msg.sender;
​
bytes32 node = keccak256(
bytes.concat(keccak256(abi.encode(account, _amount, _sharesAmount)))
);
if (!MerkleProofUpgradeable.verify(_merkleProof, merkleRoot, node)) revert InvalidProof();
​
uint256 amountToClaim = _amount - accountClaimed[account];
uint256 sharesAmountToClaim = _sharesAmount - accountSharesClaimed[account];
uint256 amountToClaimWithYield = stakingPool.getStakeByShares(sharesAmountToClaim);
​
if (amountToClaimWithYield == 0) revert NothingToClaim();
​
accountClaimed[account] = _amount;
accountSharesClaimed[account] = _sharesAmount;
IERC20Upgradeable(address(stakingPool)).safeTransfer(account, amountToClaimWithYield);
​
emit ClaimLSDTokens(account, amountToClaim, amountToClaimWithYield);
}

As you can see, the function first verifies the proof and then calculates amountToClaim as _amount - accountClaimed[account]. Afterward, it registers the claimed amount by updating the mapping: accountClaimed[account] = _amount.

The issue arises if the user later attempts to claim a smaller amount of LST tokens than they initially claimed. This will cause an underflow because accountClaimed[account] will be greater than _amount, leading to the transaction reverting due to accountClaimed[account] > _amount.

The subtraction is performed to prevent users from replaying the proof within the same Merkle root. However, if the Merkle root is updated using updateDistribution(), the issue persists because the mappings are not reset.

For example, if in the first Merkle root, the proof allows the user to claim 300 tokens, but in the second Merkle root the proof allows only 200 tokens, the transaction will revert due to an underflow. This occurs because the accountClaimed[account] value from the previous Merkle root (300) will be greater than the new _amount (200), causing the subtraction to fail.

POC

To highlight the issue, you can copy the following POC into the priority-pool.test.ts.

it('If the second claim is smaller than the first, it reverts due to underflow.', async () => {
const { signers, accounts, adrs, pp, token, stakingPool, strategy } = await loadFixture(
deployFixture
)
​
await pp.deposit(toEther(2000), true, ['0x'])
await pp.connect(signers[1]).deposit(toEther(500), true, ['0x'])
await strategy.setMaxDeposits(toEther(1500))
await pp.depositQueuedTokens(toEther(100), toEther(1000), ['0x'])
​
//proofs 1
let data = [
[ethers.ZeroAddress, toEther(0), toEther(0)],
[accounts[1], toEther(300), toEther(300)],
]
let tree = StandardMerkleTree.of(data, ['address', 'uint256', 'uint256'])
​
await pp.pauseForUpdate()
await pp.updateDistribution(
tree.root,
ethers.encodeBytes32String('ipfs'),
toEther(500),
toEther(500)
)
​
await token.transfer(adrs.strategy, toEther(1500))
await stakingPool.updateStrategyRewards([0], '0x')
​
//first claim
await pp.connect(signers[1]).claimLSDTokens(toEther(300), toEther(300), tree.getProof(1))
​
//proofs 2
let data2 = [
[ethers.ZeroAddress, toEther(0), toEther(0)],
[accounts[1], toEther(200), toEther(200)],
]
let tree2 = StandardMerkleTree.of(data2, ['address', 'uint256', 'uint256'])
​
//update merkle root
await pp.pauseForUpdate()
await pp.updateDistribution(
tree2.root,
ethers.encodeBytes32String('ipfs'),
0,
0
)
​
//second claim -> reverts underflow -> amountToClaim = _amount - accountClaimed[account] -> 200 - 300
await expect(pp.connect(signers[1]).claimLSDTokens(toEther(200), toEther(200), tree2.getProof(1))).to.be.revertedWithPanic(PANIC_CODES.ARITHMETIC_OVERFLOW)
​
assert.equal(fromEther(await pp.getQueuedTokens(accounts[1], data2[1][1])), 300)
})

Impact

Users are unable to claim their LST tokens due to DOS.

Tools Used

Manual review.

Recommendations

One potential solution to prevent replaying the same proof within the Merkle tree, while avoiding the underflow issue, is to use a nonce system instead of subtracting the accountClaimed value from the amount to claim.

Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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