hats-finance / Paladin-0x1610bfde27e57b068af7f38aec3d2a7b1d146989

Smart contract for the Vote-Flywheel part of Paladin Tokenomics
Other
1 stars 1 forks source link

Zero Amount Claim Prevents Future Claims #53

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

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

Github username: -- Twitter username: 97Sabit Submission hash (on-chain): 0x8c38ac77a6bad4ddaa0135eda34392ae10cb675159ca2043c8aefdf3c5f827c9 Severity: high

Description: Description\ The claim() function allows a user to enter an amount of 0 when claiming quest rewards. This results in the user being barred from claiming any more rewards for that quest and period, even though they did not actually claim any rewards.

Also, the safeERC20 library's safeTransfer function allows transfer of zero amount.

The same issue arises when zero amount is the input in claims[i].amount for claimQuest and multiClaim functions.

There should be a check against having zero amount as input.

  1. Proof of Concept (PoC) File

https://github.com/PaladinFinance/Vote-Flywheel/blob/cf3c82f102a76f58acf003980c480eb9028f0e94/contracts/MultiMerkleDistributorV2.sol#L148C3-L152C1

Kogaroshi commented 9 months ago

If an user inputs 0 when claiming, and the proof verification goes through, it means they are meant to have 0 rewards in the Merkle Tree, which in this case is not an issue. If they input 0 but have an amount to claim, then the 0 passed in the node creation to verify the Merkle data, then the claim will fail (since the validation is not correct), so that will not prevent them from claiming when passing the correct data. And since each Quest period is separated in the Distributor, there is no impact for the other claims.

If you have a PoC proving that the situation you describe is possible, can you provide it so I can consider this issue as valid.

ololade97 commented 9 months ago

Ok, I will work on a PoC.