To claim reward tokens from the MultiRewardStaking contract deployed, a user must call claimRewards(address user, IERC20[] memory _rewardsTokens).
The _rewardsTokens array is populated with getAllRewardsTokens() which returns all the reward Tokens the MultiStakingContract has.
claimRewards calls will likely revert in many cases, locking funds (of rewardTokens) in the staking contract.
Proof of Concept
The VaultController contract is in charge of deploying Staking contracts.
A MultiRewardStaking contract is deployed with deployStaking(IERC20 asset), based on the latest staking template activeTemplateId[STAKING].
Considering a MultiStakingContract with N reward tokens (such as N > 1) IERC20[] public rewardTokens = [iRewardToken1, iRewardToken2, ..., iRewardTokenN],
However, if the balance of one of the N reward tokens is empty (i.e it exists iRewardToken in rewardTokens such as ERC20(iRewardToken).balanceOf(address(staking)) == 0 is true ) and the other tokens are claimable, claimRewards(user, rewardTokens) will revert without the claimable tokens being claimed.
One could say that the user must construct a sub-array of rewardTokens, IERC20[] memory claimableRewardTokens which is only made of the current claimable reward tokens interfaces:
Every iRewardToken in [iRewardToken1 ; iRewardTokenN] where accruedRewards[user][iRewardToken] > 0 after the call of accrueRewards(msg.sender, user)
But there is no logic implemented to get those specific tokens and it would need to compute modifier accrueRewards(msg.sender, user) to know which interfaces to add before calling claimRewards(user, claimableRewardTokens):
The variables that can set the MultiRewardStaking contract in this locked funds state are rewardsPerSecond & amount, (rewardTokenAmount).
Taking In J such as I != J and I,J in [1;N]
rewardsEndTimestampI = block.timestamp + (amount1 / uint256(rewardsPerSecond1))
rewardsEndTimestampJ = block.timestamp + (amount2 / uint256(rewardsPerSecond2))
Through fundStakingRewards():
Anyone can fund any vault.
It executes fundReward from the MultiRewardStaking contract.
Someone can fund rewards in a way that the rewardsEndTimestamps of the rewardTokens don't match, making some reward tokens not claimable until the rewardsEndTimestamps are adjusted.
fundStakingRewards in VaultController.sol GitHub Link, fundRewardin MultiRewardStaking.sol GitHub Link
As a basic PoC, I've set a staking contract with two reward tokens:
[rewardToken1, rewardToken2]
Same reward speed
Different reward amounts.
The first claimRewards call claim 10% of the rewardToken1 and fully claims the rewardToken2.
The second claimRewards call tries to claim another 10% of the rewardToken1 funds but reverts due to rewardToken2 empty balance.
Tools Used
Manual Review
Recommended Mitigation Steps
The claimRewards function from MultiRewardStaking.sol could emit an event instead of reverting, so the claimable rewards can be claimed, regardless of the user input.
Lines of code
https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L153-L188
Vulnerability details
Impact
To claim reward tokens from the MultiRewardStaking contract deployed, a user must call
claimRewards(address user, IERC20[] memory _rewardsTokens)
. The _rewardsTokens array is populated withgetAllRewardsTokens()
which returns all the reward Tokens the MultiStakingContract has.claimRewards
calls will likely revert in many cases, locking funds (of rewardTokens) in the staking contract.Proof of Concept
The VaultController contract is in charge of deploying Staking contracts. A MultiRewardStaking contract is deployed with
deployStaking(IERC20 asset)
, based on the latest staking templateactiveTemplateId[STAKING]
.Considering a MultiStakingContract with
N
reward tokens (such as N > 1
)IERC20[] public rewardTokens = [iRewardToken1, iRewardToken2, ..., iRewardTokenN]
,The user must give an
IERC20[] memory rewardTokens
to callclaimRewards(user, rewardTokens)
. It can be done withgetAllRewardsTokens()
claimRewards function in MultiRewardStaking.sol GitHub LinkHowever, if the balance of one of the
N
reward tokens is empty (i.eit exists iRewardToken in rewardTokens such as ERC20(iRewardToken).balanceOf(address(staking)) == 0 is true
) and the other tokens are claimable,claimRewards(user, rewardTokens)
will revert without the claimable tokens being claimed.One could say that the user must construct a sub-array of
rewardTokens
,IERC20[] memory claimableRewardTokens
which is only made of the current claimable reward tokens interfaces:Every iRewardToken in [iRewardToken1 ; iRewardTokenN] where accruedRewards[user][iRewardToken] > 0 after the call of accrueRewards(msg.sender, user)
But there is no logic implemented to get those specific tokens and it would need to computemodifier accrueRewards(msg.sender, user)
to know which interfaces to add before callingclaimRewards(user, claimableRewardTokens)
:accrueRewards(msg.sender, user)
is a modifier accrueRewards modifier GitHub Link_accrueRewards(IERC20 _rewardToken, uint256 accrued)
is internal. _accrueRewards internal function GitHub Link Creating such a sub-array is impossible.The variables that can set the MultiRewardStaking contract in this locked funds state are rewardsPerSecond & amount, (rewardTokenAmount).
_calcRewardsEnd function GitHub Link
If
rewardsEndTimestampI != rewardsEndTimestampJ
, one of the reward token claim attempts will revert as seen above.There are multiple ways to set up the reward tokens accordingly:
changeStakingRewardsSpeeds()
: Limited to the vaultCreator. It executeschangeRewardsSpeed
from the MultiRewardStaking contract. It can change the rewardSpeed in a way that therewardsEndTimestamps
of therewardTokens
don't match. (changing every rewardTokens speed or only some). changeStakingRewardsSpeeds in VaultController.sol GitHub Link, changeRewardSpeed in MultiRewardStaking.sol GitHub LinkfundStakingRewards()
: Anyone can fund any vault. It executesfundReward
from the MultiRewardStaking contract. Someone can fund rewards in a way that therewardsEndTimestamps
of therewardTokens
don't match, making some reward tokens not claimable until therewardsEndTimestamps
are adjusted. fundStakingRewards in VaultController.sol GitHub Link, fundRewardin MultiRewardStaking.sol GitHub LinkAs a basic PoC, I've set a staking contract with two reward tokens:
claimRewards
call claim 10% of the rewardToken1 and fully claims the rewardToken2.claimRewards
call tries to claim another 10% of the rewardToken1 funds but reverts due to rewardToken2 empty balance.Tools Used
Manual Review
Recommended Mitigation Steps
The
claimRewards
function from MultiRewardStaking.sol could emit an event instead of reverting, so the claimable rewards can be claimed, regardless of the user input.