Last claimer will not be able to claim rewards due to stETH's 1-2 wei corner issue

[hats-bug-reporter[bot]]

Github username: -- Twitter username: MrPotatoMagic Submission hash (on-chain): 0xb4b7a597d95bee4c9e53260b82835d93d894479632bdbfeefff2b116c97ecaba Severity: medium

Description: Description\ The createCampaigns() allows anyone to create campaigns. During this creation, the reward.unclaimed field for a reward token is set to the reward amount provided in the parameters (assuming there are no fees for simplicity).

Attack Scenario:

The issue arises when using stETH as a reward token.

stETH has this special 1-2 wei corner case where due to its internal rounding in share conversion, the actual amount transferred can be 1-2 wei less than expected. The link from the documentation entails the whole scenario.

The issue in our codebase would be that when a campaign is created, the reward.unclaimed would be set to the reward amount passed as parameter instead of the actual tokens transferred (see here). Due to this, the reward.unclaimed would store a value 1-2 wei greater than the actual stETH transferred to the vault.

This would be a problem when users start claiming their rewards since the last user claiming their reward would not be able to due to this subtraction causing an underflow and thus causing a revert.

For example,

reward.unclaimed = 100 tokens

stETH balance of contract = 100 tokens - (1 or 2 wei)

Let's assume there are 10 users who will receive the amounts equally. Once the first 9 users claim their tokens i.e. 90 tokens, the last user would not be able to claim since they try to withdraw 10 tokens but the contract only has 10 tokens - (1 or 2 wei).

Due to this, the subtraction mentioned above in the link would revert and prevent the last user from claiming their rewards.

The issue has been marked as Medium-severity since there is a short term freezing of user rewards (provided the user is able to reach out to the team regarding the issue else the campaign owner could just recover the tokens, in which case the user loses out on their rewards).

Note: This issue is different from other rebasing token issues submitted in other issues.

Attachments

See here - https://github.com/hats-finance/Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2/blob/e9d6b1e594d5bb3694bfe68f73399156ebb5d3a4/src/Metrom.sol#L215

See here - https://github.com/hats-finance/Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2/blob/e9d6b1e594d5bb3694bfe68f73399156ebb5d3a4/src/Metrom.sol#L265

Mitigation: Instead of assigning the parameter amount directly to the reward.unclaimed field, create two new variables i.e. preBalance and postBalance and assign the difference to the reward.unclaimed variable to account for this stETH corner issue.

[0xmahdirostami]

@luzzif

This issue is a duplicate of issue #1 .

The issue in our codebase would be that when a campaign is created, the reward.unclaimed would be set to the reward amount passed as parameter instead of the actual tokens transferred . Due to this, the reward.unclaimed would store a value 1-2 wei greater than the actual stETH transferred to the vault.

Same as issue #1 .

And the mitigation is the same check balance before and after transferfrom.

[mcgrathcoutinho]

@luzzif I believe this issue should be considered instead of #1 since this issue is more elaborate and describes the impact scenario better as to how we revert and who is affected (i.e. the last claimer) in the example as compared to #1.

Additionally, the root cause of this issue is not a fee on transfer but an error in the stETH token, which causes this scenario.

[0xmahdirostami]

I believe this issue should be considered instead of https://github.com/hats-finance/Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2/issues/1 since this issue is more elaborate and describes the impact scenario better as to how we revert and who is affected (i.e. the last claimer) in the example as compared to https://github.com/hats-finance/Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2/issues/1.

Half of the comments say that these are duplicates.

Additionally, the root cause of this issue is not a fee on transfer but an error in the stETH token, which causes this scenario.

The other half says that these are different.

As I mentioned, this is a duplicate of issue #1 and must be invalidated due to HatsFinance guidelines.

[mcgrathcoutinho]

@0xmahdirostami By my second comment, my intention was to point out that this issue is not an exact replica of what #1 describes.

I'll let the sponsor @luzzif decide which issue is more elaborate in explaining the impact.

Thanks!

[luzzif]

While I agree that this issue is more detailed thatn #1, the root cause is the same. So I'm marking this as a duplicate and invalid. Thanks anyway @mcgrathcoutinho.