Skip to content
This repository has been archived by the owner on Jun 2, 2024. It is now read-only.
This repository has been archived by the owner on Jun 2, 2024. It is now read-only.

lemonmon - Rewards may be lost when SdtStakingPositionService.processSdtRewards() is processing multiple rewards that contain the same reward token #194

Closed
@sherlock-admin2

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:

  1. The bribes rewards, fetched on line 90 in SdtBuffer.sol
  2. The rewards from the StakeDAO gauge. The number of rewards is fetched on line 96 in SdtBuffer.sol
  3. Both of these rewards are then processed and written into the return value which is the tokenAmounts
  4. First the gauge rewards are processed and written into tokenAmounts (line 106-147 SdtBuffer.sol)
  5. Second the bribes rewards are processed and written into tokenAmounts (line 150-168 SdtBuffer.sol)
  6. Finally the tokenAmounts are returned (line 170 SdtBuffer.sol), which contains the IERC20 token and amount 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:

  1. 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.
  2. 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.
  3. Then these rewards which contain two USDC rewards are processed in SdtStakingPositionService.processSdtRewards()
  4. 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).
  5. 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.
  6. 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

https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Staking/StakeDAO/SdtStakingPositionService.sol#L550-L573

https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Rewards/StakeDAO/SdtBuffer.sol#L74-L170

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

Metadata

Assignees

No one assigned

    Labels

    DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelHighA valid High severity issueRewardA payout will be made for this issue

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions