search

sherlock-audit / 2023-02-openq-judging

4 stars 0 forks source link

GimelSec - `TieredFixedBountyV1.closeCompetition()` doesn't check if the bounty has enough payoutSchedule balances of payoutTokenAddress #501

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

GimelSec

medium

TieredFixedBountyV1.closeCompetition() doesn't check if the bounty has enough payoutSchedule balances of payoutTokenAddress

Summary

TieredFixedBountyV1.closeCompetition() doesn't check if the bounty has enough payoutSchedule balances of payoutTokenAddress. The lower-ranked claimant could block the higher-ranked claimant from claiming the bounty.

Vulnerability Detail

ClaimManagerV1._claimTieredFixedBounty() is a claim method for TieredFixedBountyV1, it calls _bounty.closeCompetition() to set status to CLOSE, and calls _bounty.claimTieredFixed() to transfer tokens.

But if the TieredFixedBountyV1 doesn't have enough tokens, and once a claimant calls permissionedClaimTieredBounty() to claim the bounty, other claimants will be unable to claim due to the insufficient funds. This scenario could happen with crowdfunding for TieredFixedBountyV1.

Assume we have a TieredFixedBountyV1 which has a payout schedule:

But currently the TieredFixedBountyV1 only has 1000 USDC funds. If the tier 3 claimant calls permissionedClaimTieredBounty(), the status will become CLOSE, nobody is able to call fundBountyToken. If the tier 1 claimant tries to call permissionedClaimTieredBounty(), the transaction will be reverted due to insufficient funds.

Impact

Only some claimant could claim bounty, the lower-ranked claimant could block the higher-ranked claimant from claiming the bounty.

Code Snippet

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/ClaimManager/Implementations/ClaimManagerV1.sol#L290-L305 https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/TieredFixedBountyV1.sol#L88-L118

Tool used

Manual Review

Recommendation

Check whether the TieredFixedBountyV1 has enough payout tokens (the sum of payoutSchedule) when closingCompetition().

FlacoJones commented 1 year ago

We should remove _bounty.closeCompetition(); from _claimTieredFixedBounty. It should stay open until the admin explicitly closes it

IAm0x52 commented 1 year ago

Invalid. Bounty issuer is responsible for making sure the contract has enough tokens before adding any winners or closing the competition. Additionally the scenario presented by the watson is invalid because they could just send tokens directly to the contract to cover the shortfall.

hrishibhat commented 1 year ago

Agree with Lead watson's comment