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

MerkleProof can be used multiple times.

cryptoticky

Summary

The withdraw, unqueueTokens, and claimLSDTokens functions utilize a verification algorithm based on the Merkle tree.

However, the MerkleProofUpgradeable.verify(_merkleProof, merkleRoot, node) function is a pure function, meaning it does not make any state changes.

Moreover, after verification, there are no state values to prove that this merkleProof and node have been used already.

Therefore, users can continue to use the same merkleProof with the same parameters.

This can be done continuously until the merkleRoot is updated through the updateDistribution function call. The claimLSDTokens function, by its nature, cannot use the merkleProof multiple times.

However, it is possible to use it multiple times in the unqueueTokens and withdraw functions.

function withdraw(
uint256 _amountToWithdraw,
uint256 _amount,
uint256 _sharesAmount,
bytes32[] calldata _merkleProof,
bool _shouldUnqueue,
bool _shouldQueueWithdrawal
) external {
if (_amountToWithdraw == 0) revert InvalidAmount();
​
uint256 toWithdraw = _amountToWithdraw;
address account = msg.sender;
​
// attempt to unqueue tokens before withdrawing if flag is set
if (_shouldUnqueue == true) {
_requireNotPaused();
​
if (_merkleProof.length != 0) {
bytes32 node = keccak256(
bytes.concat(keccak256(abi.encode(account, _amount, _sharesAmount)))
);
if (!MerkleProofUpgradeable.verify(_merkleProof, merkleRoot, node))
revert InvalidProof();
} else if (accountIndexes[account] < merkleTreeSize) {
revert InvalidProof();
}
​
uint256 queuedTokens = getQueuedTokens(account, _amount);
uint256 canUnqueue = queuedTokens <= totalQueued ? queuedTokens : totalQueued;
uint256 amountToUnqueue = toWithdraw <= canUnqueue ? toWithdraw : canUnqueue;
​
if (amountToUnqueue != 0) {
accountQueuedTokens[account] -= amountToUnqueue;
totalQueued -= amountToUnqueue;
toWithdraw -= amountToUnqueue;
emit UnqueueTokens(account, amountToUnqueue);
}
}
​
// attempt to withdraw if tokens remain after unqueueing
if (toWithdraw != 0) {
IERC20Upgradeable(address(stakingPool)).safeTransferFrom(
account,
address(this),
toWithdraw
);
toWithdraw = _withdraw(account, toWithdraw, _shouldQueueWithdrawal);
}
​
token.safeTransfer(account, _amountToWithdraw - toWithdraw);
}
function unqueueTokens(
uint256 _amountToUnqueue,
uint256 _amount,
uint256 _sharesAmount,
bytes32[] calldata _merkleProof
) external whenNotPaused {
if (_amountToUnqueue == 0) revert InvalidAmount();
if (_amountToUnqueue > totalQueued) revert InsufficientQueuedTokens();
​
address account = msg.sender;
​
// verify merkle proof only if sender is included in tree
if (accountIndexes[account] < merkleTreeSize) {
bytes32 node = keccak256(
bytes.concat(keccak256(abi.encode(account, _amount, _sharesAmount)))
);
if (!MerkleProofUpgradeable.verify(_merkleProof, merkleRoot, node))
revert InvalidProof();
}
​
if (_amountToUnqueue > getQueuedTokens(account, _amount)) revert InsufficientBalance();
​
accountQueuedTokens[account] -= _amountToUnqueue;
totalQueued -= _amountToUnqueue;
token.safeTransfer(account, _amountToUnqueue);
​
emit UnqueueTokens(account, _amountToUnqueue);
}



Impact

Although there is no loss of funds, using the Merkle tree to call the withdraw and unqueueTokens functions faces an issue that contradicts the protocol design intended to limit the size of _amountToUnqueue.

Recommendations

It would be beneficial to introduce a nonce to prevent the reuse of already used merkleProof.

Additionally, adding the byte4 code of the function being called to the node would also be a good idea.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Same merkle proof used for `claimLSDTokens` as well as `unqueueTokens`

It does concern different variables. But using the same merkle inside 3 different functions is not a good practice. Nonces, separators and safety.

Appeal created

cryptoticky Submitter
about 1 year ago
inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Same merkle proof used for `claimLSDTokens` as well as `unqueueTokens`

It does concern different variables. But using the same merkle inside 3 different functions is not a good practice. Nonces, separators and safety.

Support

FAQs

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