Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0x73696d616f - Having no deposits in StakedEXA will lead to stuck rewards when harvesting #48

Open
sherlock-admin4 opened this issue Jul 25, 2024 · 2 comments
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A Medium severity issue. Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin4
Copy link

sherlock-admin4 commented Jul 25, 2024

0x73696d616f

Medium

Having no deposits in StakedEXA will lead to stuck rewards when harvesting

Summary

The provider in StakedEXA is free to distributed rewards as it pleases and there is an automated way to distribute via StakedEXA::harvest(). The harvest setup may be started but no actual deposits have been made to StakedEXA or users may withdraw from StakedEXA while automated ongoing rewards are happening, which will lead to stuck rewards inside StakedEXA.

Root Cause

In StakedEXA::globalIndex() it returns a stale index if the totalSupply() is null (no deposits), but it should instead calculate the amount (rate x deltaTime) and send it to savings. This way rewards will never be stuck.

Internal pre-conditions

  1. No deposits are present in StakedEXA.

External pre-conditions

None.

Attack Path

  1. Deposits were not yet made and rewards are harvested or users withdraw all deposits while rewards are rolling out.

Impact

Stuck rewards as the index is not increased but rewardData.updatedAt increases.

PoC

In StakedEXA::updateIndex(), the index is updated to globalIndex(reward); and rewardData.updatedAt to the current block.timestamp or rewardData.finishAt.

function updateIndex(IERC20 reward) internal {
  RewardData storage rewardData = rewards[reward];
  rewardData.index = globalIndex(reward);
  rewardData.updatedAt = uint40(lastTimeRewardApplicable(rewardData.finishAt));
}

In StakedEXA::globalIndex(), the index is not increased if the total supply is null:

function globalIndex(IERC20 reward) public view returns (uint256) {
  RewardData storage rewardData = rewards[reward];
  if (totalSupply() == 0) return rewardData.index;

  return
    rewardData.index +
    (rewardData.rate * (lastTimeRewardApplicable(rewardData.finishAt) - rewardData.updatedAt)).divWadDown(
      totalSupply()
    );
}

Thus, as can be seen, the index will not be updated by rewardData.updatedAt is increased, losing these rewards forever.

Mitigation

If the total supply is null the amount not distributed can be calculated by doing rate x deltaTime and send to savings.

@github-actions github-actions bot added the Medium A Medium severity issue. label Jul 28, 2024
@sherlock-admin3 sherlock-admin3 added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Jul 30, 2024
@z3s z3s added the Has Duplicates A valid issue with 1+ other issues describing the same vulnerability label Aug 9, 2024
@sherlock-admin2 sherlock-admin2 changed the title Large Fuchsia Dove - Having no deposits in StakedEXA will lead to stuck rewards when harvesting 0x73696d616f - Having no deposits in StakedEXA will lead to stuck rewards when harvesting Aug 9, 2024
@sherlock-admin2 sherlock-admin2 added the Reward A payout will be made for this issue label Aug 9, 2024
@sherlock-admin2
Copy link
Contributor

The protocol team fixed this issue in the following PRs/commits:
exactly/protocol#752

@sherlock-admin2
Copy link
Contributor

The Lead Senior Watson signed off on the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A Medium severity issue. Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

4 participants