Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

bin2chen - fundBountyToken() cannot fund token that has been added before #145

Closed
github-actions bot opened this issue Feb 21, 2023 · 0 comments
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@github-actions
Copy link

github-actions bot commented Feb 21, 2023

bin2chen

medium

fundBountyToken() cannot fund token that has been added before

Summary

token address was previously funded, and now adding funds again will not grow the length of tokenAddresses, so it should not be limited by tokenAddressLimit

Vulnerability Detail

in order to avoid OUT_OF_GAS attacks.
So fundBountyToken() will check that cannot excee the maximum number of token addresses
The code is as follows:

    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),    //@audit <---------check token address amount
                Errors.TOO_MANY_TOKEN_ADDRESSES
            );
        }

    function tokenAddressLimitReached(address _bountyAddress)
        public
        view
        returns (bool)
    {
        IBounty bounty = IBounty(payable(_bountyAddress));

        return
            bounty.getTokenAddressesCount() >=      //@audit <---------the number of current token addresses cannot exceed the limit
            openQTokenWhitelist.TOKEN_ADDRESS_LIMIT();
    }

There is a problem: When the maximum number of token addresses has been reached, no more funds can be provided, even for tokens that already exist in tokenAddresses.
If the token address has already been funded before, the array length will not grow again.
It should be possible to add

For example:
Suppose the maximum limit = 3
Once deposited token A = 100 token B = 100 token C = 100
When the user wants to add 50 tokens to token A
At this point the execution of fundBountyToken() will prompt

Errors.TOO_MANY_TOKEN_ADDRESSES

So it is recommended that if the token address has been provided before, it is still possible to add new funds

Impact

can't fund bounty token

Code Snippet

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

Tool used

Manual Review

Recommendation

    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(
+              bounty.containsToken(_tokenAddress) ||          
                !tokenAddressLimitReached(_bountyAddress),
                Errors.TOO_MANY_TOKEN_ADDRESSES
            );
        }

Duplicate of #530

@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue labels Feb 21, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Mar 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant