unforgiven - Attacker can perform DOS on any bounty contract by over calling fundBountyToken() with different tokens

[github-actions]

unforgiven

medium

Attacker can perform DOS on any bounty contract by over calling fundBountyToken() with different tokens

Summary

Attacker can create multiple number of deposits with small amount and short durations and Bounty would reach the token address limit and attacker would prevent real issuer from depositing and funding the Bounty. some Deposits would be locked for deposit time for nothing and issued Bounty would be useless.

Vulnerability Detail

Function fundBountyToken() is for depositing funds into the bounty and it is callable by anyone. when deposits accrue for a bounty, Bounty contract add it to deposits and add token address to tokenAddresses in the BountyCore.receiveFunds() function. attacker can call fundBountyToken() for a bounty a couple of time with different white listed token addresses and make that bounty to reach the limit and then issuer can't fund the bounty. it's a DOS and griefing attack as some deposit funds would stuck in the contract for deposit time and Bounty contract won't be usefull.
This is fundBountyToken() code:

    function fundBountyToken(
        address _bountyAddress,
        address _tokenAddress,
        uint256 _volume,
        uint256 _expiration,
        string memory funderUuid
    ) external payable onlyProxy {
        IBounty bounty = IBounty(payable(_bountyAddress));

        if (!isWhitelisted(_tokenAddress)) {
            require(
                !tokenAddressLimitReached(_bountyAddress),
                Errors.TOO_MANY_TOKEN_ADDRESSES
            );
        }

        require(bountyIsOpen(_bountyAddress), Errors.CONTRACT_ALREADY_CLOSED);

        (bytes32 depositId, uint256 volumeReceived) = bounty.receiveFunds{
            value: msg.value
        }(msg.sender, _tokenAddress, _volume, _expiration);

        bytes memory funderUuidBytes = abi.encode(funderUuid);
.........
..........
    }

As you can see code checks tokenAddressLimitReached(_bountyAddress) to be false, and that function returns true if the total number of unique tokens deposited on then bounty is greater than the OpenQWhitelist TOKEN_ADDRESS_LIMIT.
and then code calls bounty.receiveFunds() which is:

    function receiveFunds(
        address _funder,
        address _tokenAddress,
        uint256 _volume,
        uint256 _expiration
    )
        external
        payable
        virtual
        onlyDepositManager
        nonReentrant
        returns (bytes32, uint256)
    {
        require(_volume != 0, Errors.ZERO_VOLUME_SENT);
        require(_expiration > 0, Errors.EXPIRATION_NOT_GREATER_THAN_ZERO);
        require(status == OpenQDefinitions.OPEN, Errors.CONTRACT_IS_CLOSED);

        bytes32 depositId = _generateDepositId();

        uint256 volumeReceived;
        if (_tokenAddress == address(0)) {
            volumeReceived = msg.value;
        } else {
            volumeReceived = _receiveERC20(_tokenAddress, _funder, _volume);
        }

        funder[depositId] = _funder;
        tokenAddress[depositId] = _tokenAddress;
        volume[depositId] = volumeReceived;
        depositTime[depositId] = block.timestamp;
        expiration[depositId] = _expiration;
        isNFT[depositId] = false;

        deposits.push(depositId);
        tokenAddresses.add(_tokenAddress);

        return (depositId, volumeReceived);
    }

which adds the deposit tokens to tokenAddresses set. so attacker can perform this steps:

1. white listed tokens are 100 which are registered in OpenQWhitelist.
2. the value of TOKEN_ADDRESS_LIMIT is 10.
3. issuer creates Bounty1 and he wants to deposit 100K USDT and 100K USDC.
4. issuer deposits 100K USD for 3 month which is the duration of the bounty.
5. attacker deposits small amounts of 9 different tokens to Bounty1.
6. now issuer can't deposit 100K USDC because Bounty1 has 10 token in tokenAddresses and code won't allow deposits.

Impact

attacker can cause griefing and DOS and temporary fund lock.

Code Snippet

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

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

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/Bounty/Implementations/BountyCore.sol#L55

Tool used

Manual Review

Recommendation

create two limit for deposit tokens. one for issuer and one for others. in this way issuer can always deposit tokens even if others deposits small amounts before him.

Duplicate of #530