31078 - [SC - High] withdraw doesnt claim all rewards before burnin...
Submitted on May 12th 2024 at 11:07:38 UTC by @Holterhus for Boost | Alchemix
Report ID: #31078
Report type: Smart Contract
Report severity: High
Target: https://github.com/alchemix-finance/alchemix-v2-dao/blob/main/src/VotingEscrow.sol
Impacts:
Permanent freezing of unclaimed yield
Description
Brief/Intro
The claim()
function in the RewardsDistributor
does not always claim all of the pending rewards for a given tokenId
. This is because the _claimable()
function has a finite for
loop that does 50 iterations. However, the withdraw()
function in veALCX
only calls claim()
once before permanently burning the token. This can lead to a permanent freezing of unclaimed yield if the 50 iterations are not sufficient.
Vulnerability Details
The _claimable()
function in the RewardsDistributor
has the following main loop:
Notice that this loop only does a maximum of 50 iterations. This is fine on its own, because calling claim()
multiple times will progress the rewards claiming in multiples of 50 iterations.
However, consider the following code in the withdraw()
function of the veALCX
contract:
Since this only calls claim()
once, this will only do 50 iterations within the _claimable()
calculation, which can be insufficient for claiming all of the user's unclaimed ALCX
rewards.
Impact Details
Yield is permanently frozen whenever a user calls withdraw()
on a tokenId
that needs more than 50 iterations to claim remaining rewards. This can happen if the tokenId
has been deposited and left alone for a long time (perhaps with maxLockEnabled == true
). It can also happen if the user has checkpointed many times before (notice that weekCursor += WEEK
only happens in the else
case of the main loop, so claim()
may not even progress a single week).
References
See the PoC below.
Proof of Concept
I have created the following test file and added it to the tests/
directory: