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

rvierdiiev - DepositManagerV1.refundDeposit will revert when big number of deposits will be created in bounty #66

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

Comments

@github-actions
Copy link

github-actions bot commented Feb 21, 2023

rvierdiiev

medium

DepositManagerV1.refundDeposit will revert when big number of deposits will be created in bounty

Summary

DepositManagerV1.refundDeposit will revert when big number of deposits will be created in bounty. This is because BountyCore.getLockedFunds will revert with out of gas error. As result it will be not possible to refund deposit.

Vulnerability Detail

When depositor creates new deposit, then it is added to the deposit array.
It's possible that the bounty contract will contain a lot of different deposits, so the number will increase with time.

In order to refund deposit, user calls DepositManagerV1.refundDeposit. This function will then call bounty.getLockedFunds in order to receive locked tokens amount.

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

    function getLockedFunds(address _depositId)
        public
        view
        virtual
        returns (uint256)
    {
        uint256 lockedFunds;
        bytes32[] memory depList = this.getDeposits();
        for (uint256 i = 0; i < depList.length; i++) {
            if (
                block.timestamp <
                depositTime[depList[i]] + expiration[depList[i]] &&
                tokenAddress[depList[i]] == _depositId
            ) {
                lockedFunds += volume[depList[i]];
            }
        }


        return lockedFunds;
    }

As you can see, this function iterates over all deposits array and in case if it's too big, then it will just revert with out of gas error.

Also pls, note that anyone can create new deposit with 0 amount(using token address 0 and msg.value 0), so it allows to create a lot of deposits in order to prevent depositor from refunding if it's needed.

Impact

It will be not possible to refund deposit.

Code Snippet

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

Tool used

Manual Review

Recommendation

Add some deposit limit amount to the bounty, or think about another way to get deposits in specific token.

Duplicate of #77

@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High 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 High A valid High severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant