hats-finance / Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2

Smart contracts for the Metrom project.
GNU General Public License v3.0
0 stars 0 forks source link

Rewards should only be recoverable after the campain finishes #21

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

Github username: @@giorgiodalla Twitter username: 0xAuditism Submission hash (on-chain): 0x7342e6241dd4449bf5af0021cb50ac7038451efa2f05be4d0ec0b6ba25d590b0 Severity: medium

Description: Description\ The campain owner is able to recover unclaimed or unclaimable rewards. However the owner should only be able to claim those after the campain finishes. Otherwise users might be wrongfully incentivised on rewards that have been fully recovered by the campain owner.

Attack Scenario\

Campain owner wants launches an incentive to boost trades on his dex. Campain owner doesn't decides that he wants to withdraw the incentives. He does so while the campain is still on. Potential claimers are unable to do so because rewards have been recovered.

Attachments

  1. Proof of Concept (PoC) File In the recoverRewards function we can see that rewards may be reclaimed at any time. In _processRewardClaim the same applies rewards may be withdrawn at any time. Which means the campain owner is always able to claim rewards.

  2. Revised Code File (Optional)

Add a time restriction, only enabling the owner to withdraw funds when the campain is over.

    function recoverRewards(ClaimRewardBundle[] calldata _bundles) external override {
        for (uint256 _i; _i < _bundles.length; _i++) {
            ClaimRewardBundle calldata _bundle = _bundles[_i];

            Campaign storage campaign = _getExistingCampaign(_bundle.campaignId);
            if (msg.sender != campaign.owner) revert Forbidden();
    +      if(campain.to > block.timestamp) revert campainStillOngoing();
            uint256 _claimedAmount = _processRewardClaim(campaign, _bundle, address(0));
            emit RecoverReward(_bundle.campaignId, _bundle.token, _claimedAmount, _bundle.receiver); 
        }
    }
luzzif commented 4 months ago

Kind of duplicate of https://github.com/hats-finance/Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2/issues/20, the comment I gave there applies to this too.