ctf_sec - Bounty Claim can revert in error insufficient balance

[github-actions]

ctf_sec

medium

Bounty Claim can revert in error insufficient balance

Summary

Bounty Claim can revert in error insufficient balance

Vulnerability Detail

In the current implementation, the bounty issue can call the function below setPayout to change payout token and payout volume.

/// @notice Sets payout token address and volume on bounty with id _bountyId
    /// @param _bountyId The id to update
    /// @param _payoutToken The token address to be used for the payout
    /// @param _payoutVolume The volume of token to be used for the payout
    function setPayout(
        string calldata _bountyId,
        address _payoutToken,
        uint256 _payoutVolume
    ) external onlyProxy {
        IBounty bounty = getBounty(_bountyId);

        require(msg.sender == bounty.issuer(), Errors.CALLER_NOT_ISSUER);

        bounty.setPayout(_payoutToken, _payoutVolume);

        emit PayoutSet(
            address(bounty),
            _payoutToken,
            _payoutVolume,
            bounty.bountyType(),
            new bytes(0),
            VERSION_1
        );
    }

However, the ongoing bounty claim and ongoing bounty refund can be impacted.

When the claim function is called on OngoingBoutyV1.sol

function claimOngoingPayout(
   address _payoutAddress,
   bytes calldata _closerData
) external onlyClaimManager nonReentrant returns (address, uint256) {
    (, string memory claimant, , string memory claimantAsset) = abi.decode(
        _closerData,
        (address, string, address, string)
    );

    bytes32 _claimId = generateClaimId(claimant, claimantAsset);

    claimId[_claimId] = true;
    claimIds.push(_claimId);

    _transferToken(payoutTokenAddress, payoutVolume, _payoutAddress);
    return (payoutTokenAddress, payoutVolume);
}

note the line:

_transferToken(payoutTokenAddress, payoutVolume, _payoutAddress);

For example, in the beginning, the bounty issuer set payout token to USDC and payout volume to 100 USDC.

A bounty funder fund the onGoingBounty contract with 300 USDC.

Two developers claim the bounty from ongoing bounty via ClaimManager.sol and claim total 200 USDC.

Then the bounty issuer changes the payout volume to 60 USDC.

Another developer claim 60 USDC from the contract, at this time, there are 40 USDC left in the contract, but if another developer wants to claim 60 USDC, the transaction reverts because there is an insufficient balance left in the bounty contract.

The same issue happens when claiming TieredFixedBounty and TieredPercentageBounty

/// @notice Transfers the fixed amount of balance associated with the tier
    /// @param _payoutAddress The destination address for the fund
    /// @param _tier The ordinal of the claimant (e.g. 1st place, 2nd place)
    function claimTieredFixed(address _payoutAddress, uint256 _tier)
        external
        onlyClaimManager
        nonReentrant
        returns (uint256)
    {
        require(
            bountyType == OpenQDefinitions.TIERED_FIXED,
            Errors.NOT_A_TIERED_FIXED_BOUNTY
        );
        require(!tierClaimed[_tier], Errors.TIER_ALREADY_CLAIMED);

        uint256 claimedBalance = payoutSchedule[_tier];

        _transferToken(payoutTokenAddress, claimedBalance, _payoutAddress);
        return claimedBalance;
    }

In TieredFixedBounty, if the contract balance belows the “claimedBalance”, transaction revert in insufficient balance. And adjusting payoutSchedule of the tier can impact the claim function to make the transaction revert in insufficient balance.

function claimTiered(
        address _payoutAddress,
        uint256 _tier,
        address _tokenAddress
    ) external onlyClaimManager nonReentrant returns (uint256) {
        require(
            bountyType == OpenQDefinitions.TIERED_PERCENTAGE,
            Errors.NOT, _A_TIERED_BOUNTY
        );
        require(!tierClaimed[_tier], Errors.TIER_ALREADY_CLAIMED);

        uint256 claimedBalance = (payoutSchedule[_tier] *
            fundingTotals[_tokenAddress]) / 100;

        _transferToken(_tokenAddress, claimedBalance, _payoutAddress);
        return claimedBalance;
    }

In TieredPercentageBountV1.sol. if the contract balance belows the “claimedBalance”, transaction revert in insufficient balance. And adjusting payoutSchedule of the tier can impact the claim function to make the transaction revert in insufficient balance.

Impact

Bounty Claim can revert in error insufficient balance because adjustment of the payout volume and payout schedule impact the ongoing bounty claim.

Code Snippet

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/OngoingBountyV1.sol#L91-L113

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/TieredFixedBountyV1.sol#L87-L108

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

Tool used

Manual Review

Recommendation

We recommend the protocol map the claim id to the eligible claim amount.

For example, mapping claim id 1 to payout volume 100 USDC.

If there are 60 USDC in the bounty contract, let the developer claim the 60 USDC and decrease and update the mapping: now the eligible amount for developer using claim id 100 USDC (payout volume) - 60 USDC (claimed amount) = 40 USDC, then next time the developer can use the claim id to claim 40 USDC (or less than 40 USDC).

Duplicate of #272

[FlacoJones]

This would simply show that the ongoing bounty is insolvent. User beware.

[FlacoJones]

I do like the recommended fix - its a bit complex, and id probably rather opt for just preventing claims on insolvent bounties. The reputation hit of the funder is a consideration here too

[FlacoJones]

Will simply fix for now by removing Ongoing

[FlacoJones]

OpenQDev/OpenQ-Contracts#112