lemonmon - Rewards may be lost when SdtStakingPositionService.processSdtRewards()
is processing multiple rewards that contain the same reward token #194
Description
lemonmon
medium
Rewards may be lost when SdtStakingPositionService.processSdtRewards()
is processing multiple rewards that contain the same reward token
Summary
When rewards are processed inside SdtStakingPositionService.processSdtRewards()
, reward amounts may be lost, because the reward amount may be overwritten if there are multiple rewards processed that contain the same reward token.
Vulnerability Detail
Whenever rewards are processed with SdtStakingPositionService.processSdtRewards()
, the TokenAmount Array _rewardAssets
is fetched by calling SdtBuffer.pullRewards()
(line 559 SdtStakingPositionService.sol).
SdtBuffer.pullRewards()
will return rewards from two different sources:
- The bribes rewards, fetched on line 90 in SdtBuffer.sol
- The rewards from the StakeDAO gauge. The number of rewards is fetched on line 96 in SdtBuffer.sol
- Both of these rewards are then processed and written into the return value which is the
tokenAmounts
- First the gauge rewards are processed and written into
tokenAmounts
(line 106-147 SdtBuffer.sol) - Second the bribes rewards are processed and written into
tokenAmounts
(line 150-168 SdtBuffer.sol) - Finally the
tokenAmounts
are returned (line 170 SdtBuffer.sol), which contains the IERC20token
andamount
data for each reward.
Back in SdtStakingPositionService.processSdtRewards()
the _rewardAssets
TokenAmount Array is then processed inside a for loop (line 561-577 SdtStakingPositionService.sol).
The issue is on line 570 where the _token
(_rewardAssets[i].token
) and _rewardAssets[i].amount
are processed and assigned to _sdtRewardsByCycle[_cvgStakingCycle][erc20Id]
. The problem here is that if the _rewardAssets
array has multiple TokenAmount
entries with the same token which share the same erc20Id
, then rewards will be overwritten and lost.
Example:
SdtBuffer.pullRewards()
fetches rewards from bribes rewards and from StakeDAO gauge rewards that contain an identical reward token, for example both of them are containing a USDC reward.- Then the return value
tokenAmounts
will contain two entries with the USDC token: the first USDC entry from StakeDAO gauge rewards and the second USDC entry from bribes rewards. - Then these rewards which contain two USDC rewards are processed in
SdtStakingPositionService.processSdtRewards()
SdtStakingPositionService.processSdtRewards()
is iterating over the_rewardAssets
, processing the first USDC reward entry from_rewardAssets
and writes the_token
(USDC) and amount into_sdtRewardsByCycle[_cvgStakingCycle][erc20Id]
(line 570 SdtStakingPositionService.sol).- Then
SdtStakingPositionService.processSdtRewards()
processes the second USDC reward entry from_rewardAssets
and overwrites the existing reward amount from the first USDC reward inside_sdtRewardsByCycle[_cvgStakingCycle][erc20Id]
with the second USDC reward amount. - Because the reward amount for the first USDC reward was overwritten, the first USDC reward is lost.
Impact
Rewards may be lost if there are identical reward tokens from the StakeDAO gauge rewards and from bribes rewards. For each identical reward token the rewards from the StakeDAO gauge will be lost, since they are written first into the tokenAmounts
return value in SdtBuffer.pullRewards()
, and will later be overwritten by the bribes reward token amount when rewards are processed inside SdtStakingPositionService.processSdtRewards()
.
Code Snippet
Tool used
Manual Review
Recommendation
Consider adding the reward amount instead of overwriting the reward amount when processing SDT rewards:
// SdtStakingPositionService.processSdtRewards()
572 amount: _sdtRewardsByCycle[_cvgStakingCycle][erc20Id].amount + _rewardAssets[i].amount
Duplicate of #182