carrot - Function closeCompetition() in TieredBountyPercentage contract can be bricked, stopping claims

[github-actions]

carrot

high

Function closeCompetition() in TieredBountyPercentage contract can be bricked, stopping claims

Summary

The function closeCompetition() in contract TieredBountyPercentage is called when a claim is made. This function however makes unsafe external calls which, if it reverts, can stop the funds from being claimed.

Vulnerability Detail

The function closecompetition is defined as shown
https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/TieredPercentageBountyV1.sol#L123-L136
This function is crucial for the claiming process, since if this function reverts, the claims do not go through.

This function calls getTokenBalance, which ends up calling the contract at _tokenAddress to check the balance
https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/BountyCore.sol#L275-L286
https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/BountyCore.sol#L291-L299

A user can deposit the contract with a malicious token contract, and make the balanceOf() call revert when the bounty contract tries to call it. This will stop the claims process and break the bounty.

Impact

Broken bounty contract where winners are unable to claim their prize tokens/Nfts

Code Snippet

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/TieredPercentageBountyV1.sol#L123-L136

Tool used

Manual Review

Recommendation

Make external calls such as balanceOf() inside a try-catch block, to prevent unintended reverts.

Duplicate of #62

[FlacoJones]

Will fix by implementing an explicit token whitelist

[FlacoJones]

OpenQDev/OpenQ-Contracts#112 and OpenQDev/OpenQ-Contracts#113

[IAm0x52]

Dupe of #62. Same root cause of adding a malicious ERC20