hats-finance / Kintsu-0x7d70f9442af3a9a0a734fa6a1b4857f25518e9d2

Smart contracts for Kintsu
Other
0 stars 0 forks source link

`unclaimed` data can mislead campaign owner when recover reward, late user claim will be affected #53

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: chainNue Submission hash (on-chain): 0x2bbf6955c894c1d807ed8e910fbcba8815257626f95448133987df9df363a0f4 Severity: medium

Description: Description

In claimRewards there is no period of time when this reward should be claimed. Even if the campaign is ended, user can keep his rewards in the contract for late claim.

Currently a campaign doesn't have an information how many accumulated reward is in pending for users to claim. In distributeRewards there is also no on-chain information how many token reward is eligible to be distributed and eligible to be claimed by users. Yes, there is a unclaimed data, but it's not represent a total of potential late user claim amount, because this unclaimed is just initial reward substracted by already claimed reward.

File: Metrom.sol
273:     function claimRewards(ClaimRewardBundle[] calldata _bundles) external override {
274:         for (uint256 _i; _i < _bundles.length; _i++) {
275:             ClaimRewardBundle calldata _bundle = _bundles[_i];
276:             uint256 _claimedAmount = _processRewardClaim(_getExistingCampaign(_bundle.campaignId), _bundle, msg.sender);
277:             emit ClaimReward(_bundle.campaignId, _bundle.token, _claimedAmount, _bundle.receiver);
278:         }
279:     }
280: 
281:     /// @inheritdoc IMetrom
282:     function recoverRewards(ClaimRewardBundle[] calldata _bundles) external override {
283:         for (uint256 _i; _i < _bundles.length; _i++) {
284:             ClaimRewardBundle calldata _bundle = _bundles[_i];
285: 
286:             Campaign storage campaign = _getExistingCampaign(_bundle.campaignId);
287:             if (msg.sender != campaign.owner) revert Forbidden();
288: 
289:             uint256 _claimedAmount = _processRewardClaim(campaign, _bundle, address(0));
290:             emit RecoverReward(_bundle.campaignId, _bundle.token, _claimedAmount, _bundle.receiver);
291:         }
292:     }

There is this recoverRewards function which is function to recover any reward token by the campaign owner due to cases, such as zero liquidity in the pool. Campaign owner can recover their reward token. How many token can be recovered, this might be the unclaimed.

This situation became issue, because unclaimed doesn't include unclaimed reward for user.

Worse case situation here is, campaign owner recover all of the unclaimed reward token, and eligible user who is going to have a late claim will get less or even zero rewards.

Recommendation

Introduce a variable to store the amount eligible to be claim, for example in distributeRewards add campaign.pendingreward, and decrease this pending reward when users claim. Finally, when recoverRewards, max amount to be recover is the diff between unclaimed and this pending reward.

chainNue commented 1 month ago

please ignore, wrong contest