search

hats-finance / Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2

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

claiming twice of rewards #17

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0xe200a80c3a1ddfa549183c5b58227b065d1968e37e58afc5d0f84c84e45585bc Severity: high

Description: Description\

Attack Scenario\

The distributeRewards() function is called by the updater. The function will update the associated reward metadata. It is expected that the merkle root will be updated expecting to correctly identify which claimers have already claimed tokens.

The distributeRewards() However, there is potential for mis-use if users frontrun calls to TheDistributeRewards() and claim their reward after the new merkle root has been calculated and updated by the admin role. This may allow the claimer to double claim their rewards or lead to a loss in rewards if the reward metadata completely replaces the data of the previous claimers . when using a merkle proof system, the new proof is calculated at a certain time.

If User Bob didn't claim when the proof was generated, they technically have time between when the proof is generated and the proof is published to claim for the first time.

Because the new Merkle Proof (ProofX) was built to allow User BOB to claim, they will be able to claim again. the possible Reclaiming of Rewards will result in a loss of funds .

Attachmentshttps://github.com/hats-finance/Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2/blob/e9d6b1e594d5bb3694bfe68f73399156ebb5d3a4/src/Metrom.sol#L234-L247

  1. Revised Code File (Optional) Consider implementing a delay where users cannot claim rewards before a call to Distributerewards() is made. This should ensure the updater role can construct a merkle tree based on the most up-to-date and correct data.
luzzif commented 3 months ago

The reward.claimed[_claimOwner] update in the _processRewardClaim should avoid the possibility of any double claim, and there's also a test regarding that in the test suite. The backend works in a way that claims are generated with an ever-increasing amount of tokens assigned as time passes in the campaign, and with a mechanism like that coupled with the already claimed rewards accounting double claims should be avoided. There is a rare possibility though that the updater can frontrun the user while updating the Merkle root on-chain and make the user rewards claim transaction fail, that's true. Can you provide a PoC for the double claim attack scenario?