Open code423n4 opened 2 years ago
Given the code available the warden has shown a possible scenario where certain depositors cannot receive reward tokens.
Because this is contingent on a improper configuration and because this relates to loss of Yield I believe Medium Severity to be more appropriate
Lines of code
https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L154-L158
Vulnerability details
https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L154-L158
In the current implementation, the contract only checks if balanceOf
rewardsToken
is greater than or equal to the future rewards.However, under normal circumstances, since users can not withdraw all their rewards in time, the balance in the contract contains rewards that belong to the users but have not been withdrawn yet. This means the current checks can not be sufficient enough to make sure the contract has enough amount of rewardsToken.
As a result, if the
rewardsDistribution
mistakenlynotifyRewardAmount
with a larger amount, the contract may end up in a wrong state that makes some users unable to claim their rewards.PoC
Given:
1,000
stakingToken;rewardsDistribution
sends100
rewardsToken to the contract;rewardsDistribution
callsnotifyRewardAmount()
withamount
=100
;earned()
and it returns100
rewardsToken, but Alice choose not togetReward()
for now;rewardsDistribution
callsnotifyRewardAmount()
withamount
=100
without send any fund to contract, the tx will succees;earned()
200
rewardsToken, when Alice tries to callgetReward()
, the transaction will fail due to insufficient balance of rewardsToken.Expected Results:
The tx in step 5 should revert.
Recommendation
Consider changing the function
notifyRewardAmount
toaddRward
and usetransferFrom
to transfer rewardsToken into the contract: