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

bin2chen - deposits OUT_OF_GAS attacks #149

Closed
github-actions bot opened this issue Feb 21, 2023 · 0 comments
Closed

bin2chen - deposits OUT_OF_GAS attacks #149

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

bin2chen

medium

deposits OUT_OF_GAS attacks

Summary

deposits lacks the maximum limit. Can be OUT_OF_GAS attacks

Vulnerability Detail

At present, in order to avoid OUT_OF_GAS attacks, the following two mechanisms have been added:

  1. Maximum token Address Quantity Limit: openQTokenWhitelist.TOKEN_ADDRESS_LIMIT
  2. Maximum NFT quantity Limit :nftDepositLimit =5

However, there is no limit on the maximum number of BountyCore.deposits, If it is too large, OUT_OF_GAS will appear.

DepositManagerV1.refundDeposit() -> bounty.getLockedFunds()

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

        return lockedFunds;
    }

And the cost of adding a deposit is not high, At least only 1gwei token is required.

suggested to add the maximum limit of deposit, and the issuer is not subject to this limit.

Impact

OUT_OF_GAS attacks, can't refund.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

    function receiveFunds(
        address _funder,
        address _tokenAddress,
        uint256 _volume,
        uint256 _expiration
    )
        external
        payable
        virtual
        onlyDepositManager
        nonReentrant
        returns (bytes32, uint256)
    {

+       require(_funder == issuer || deposits.length <=1000,"too more deposits");
        require(_volume != 0, Errors.ZERO_VOLUME_SENT);
        require(_expiration > 0, Errors.EXPIRATION_NOT_GREATER_THAN_ZERO);
        require(status == OpenQDefinitions.OPEN, Errors.CONTRACT_IS_CLOSED);

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