HonorLt - Dependant token transfers can block claims

[github-actions]

HonorLt

high

Dependant token transfers can block claims

Summary

The claim process depends on all the token transfers to succeed. One revert can make the rewards unclaimable.

Vulnerability Detail

When claiming the bounty, it iterates over all the tokens and claims it one by one.

Atomic bounty:

function _claimAtomicBounty(
        ...
    ) internal {
        ...
        for (uint256 i = 0; i < _bounty.getTokenAddresses().length; i++) {
            ...
            uint256 volume = _bounty.claimBalance(
                _closer,
                _bounty.getTokenAddresses()[i]
            );
            ...
        }

        for (uint256 i = 0; i < _bounty.getNftDeposits().length; i++) {
            _bounty.claimNft(_closer, _bounty.nftDeposits(i));
            ...
        }
    }

Tiered percentage bounty:

    function _claimTieredPercentageBounty(
       ...
    ) internal {
        ...
        for (uint256 i = 0; i < _bounty.getTokenAddresses().length; i++) {
            uint256 volume = _bounty.claimTiered(
                _closer,
                _tier,
                _bounty.getTokenAddresses()[i]
            );
            ...
        }

        for (uint256 i = 0; i < _bounty.getNftDeposits().length; i++) {
              ...
                _bounty.claimNft(_closer, _depositId);
              ...
            }
        }
           ...
    }

Tiered fixed bounty:

 function _claimTieredFixedBounty(
        ...
    ) internal {
        ...
        for (uint256 i = 0; i < _bounty.getNftDeposits().length; i++) {
            bytes32 _depositId = _bounty.nftDeposits(i);
            if (_bounty.tier(_depositId) == _tier) {
                _bounty.claimNft(_closer, _depositId);
                ...
            }
        }
     ...
    }

When token address limit is not reached, anyone can fund any token:

    function fundBountyToken(
        ...
    ) external payable onlyProxy {
        ...
        if (!isWhitelisted(_tokenAddress)) {
            require(
                !tokenAddressLimitReached(_bountyAddress),
                Errors.TOO_MANY_TOKEN_ADDRESSES
            );
        }
       ...

A malicious actors can create their own token that reverts when the sender is not they, then fund the bounty to make the legitimate claims revert. there is no way to opt-out from tokens you want to skip when claiming. Even legitimate tokens may sometimes revert on certain conditions.

Impact

If any of the deposits, either ERC20 token or NFT reverts on transfer, the whole claim process will revert making it impossible to claim the bounty.

Code Snippet

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/ClaimManager/Implementations/ClaimManagerV1.sol#L130-L134

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/ClaimManager/Implementations/ClaimManagerV1.sol#L130-L134

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/ClaimManager/Implementations/ClaimManagerV1.sol#L230-L235

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/ClaimManager/Implementations/ClaimManagerV1.sol#L251-L254

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/ClaimManager/Implementations/ClaimManagerV1.sol#L320-L323

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

Tool used

Manual Review

Recommendation

Claim tokens should have opt-out option to exclude potentially malicious or not interesting rewards.

Duplicate of #62