TrungOre - Malicious funders can call refundDeposit() after a bounty is closed to make bounty doesn't have enough fund to pay for the winners.

[github-actions]

TrungOre

high

Malicious funders can call refundDeposit() after a bounty is closed to make bounty doesn't have enough fund to pay for the winners.

Summary

Since there is no requirement about when the funder can call refund, they can call it right after the bounty close to make it not enough tokens to pay the competitor.

Vulnerability Detail

Function DepositManagerV1.fundBountyToken() just can be called when the bounty is open, so when a bounty is closed there won't be any deposit be accepted.

function fundBountyToken(
        address _bountyAddress,
        address _tokenAddress,
        uint256 _volume,
        uint256 _expiration,
        string memory funderUuid
    ) external payable onlyProxy {
        /// ... 
        
        /// [#explain] check if a bounty is open or not ? 
        require(bountyIsOpen(_bountyAddress), Errors.CONTRACT_ALREADY_CLOSED);
        
        /// [#explain] process the deposit 
        (bytes32 depositId, uint256 volumeReceived) = bounty.receiveFunds{
            value: msg.value
        }(msg.sender, _tokenAddress, _volume, _expiration);

        /// ... 
    }

Different from the function above, DepositManagerV1.refundDeposit() can be called anytime as long as the deposit is expired. This will create a flaw for malicious funders to trick the bounty to ruin the organization's reputation.

We will consider a scenario with a TieredPercentageBounty.

0. A bounty need 1000 USDC to distribute rewards for competitors. First winner get 30% pool, second one get 70%.
1. Alice deposits 300 USDC into the bounty
2. A Malicious funder deposits 700 USDC into bounty with expiration = 1.
3. After at least 1 second, bounty's issuer see that there are enough fund to distribute the rewards to the winners, he calls claim reward for first winner with prize = 300 USDC
4. Before the time the issuer call claim reward for the second winner, Malicious user (can front-run) called DepositManagerV1.refundDeposit() to claim his deposit back.
5. Now when the issuer tries to call claim reward for the second winner, the tx will fail since there isn't enough fund to execute the claim (The balance of USDC in the contract now is 0 USDC < 1000 * 70% = 70 USDC)

Note that this issue doesn't happen just with TieredPercentageBounty, it also applies for the Atomic and FixedTierBounty by front-running the tx call ClaimManagerV1.claimBounty() to withdraw their deposit to reduce the rewards pool of the bounty.

There is a mitigation for this issue is the issuer can send directly the amount of rewards missing to the bounty (don't need to use DepositManagerV1.fundBountyToken()), but in-case the reward is ETH (native token), it can't be success since the contract implements a auto revert fallback function. So the problem is still remained.

Impact

Bounty won't have enough fund to pay for the competitor.

Code Snippet

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/DepositManager/Implementations/DepositManagerV1.sol#L152

Tool used

Manual review

Recommendation

Consider to divide each bounty into 3 periods:

1. Funding period: Let funders deposit their tokens into bounty, funders can call refund during this period.
2. Distribute period: Distribute rewards to the competitors. No1 can call refund in this phase.
3. Refund period: Let funders claim the remaining tokens left in bounty

Duplicate of #266