ctf_sec - Unbounded gas consumption When calling BountyCore#getLockedFunds

[github-actions]

ctf_sec

medium

Unbounded gas consumption When calling BountyCore#getLockedFunds

Summary

Unbounded gas consumption When calling BountyCore#getDeposits

Vulnerability Detail

In the current implementation, when request refund via DepositManager.sol, available fund need to be calculated.

function refundDeposit(address _bountyAddress, bytes32 _depositId)
	external
	onlyProxy
{
	IBounty bounty = IBounty(payable(_bountyAddress));

	require(
		bounty.funder(_depositId) == msg.sender,
		Errors.CALLER_NOT_FUNDER
	);

	require(
		block.timestamp >=
			bounty.depositTime(_depositId) + bounty.expiration(_depositId),
		Errors.PREMATURE_REFUND_REQUEST
	);

	address depToken = bounty.tokenAddress(_depositId);

	uint256 availableFunds = bounty.getTokenBalance(depToken) -
		bounty.getLockedFunds(depToken);

note the function call:

uint256 availableFunds = bounty.getTokenBalance(depToken) -
	bounty.getLockedFunds(depToken);

Which calls bounty.getLockedFunds:

/// @notice Returns the amount of locked tokens (of a specific token) on a bounty address, only available for claims but not for refunds
/// @param _depositId The depositId that determines which token is being looked at
/// @return uint256
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;
}

this for loop needs to iterate over all result returned from getDeposits function.

/// @notice Returns an array of all deposits (ERC20, protocol token, and NFT) for this bounty
/// @return deposits The array of deposits including ERC20, protocol token, and NFT
function getDeposits() external view virtual returns (bytes32[] memory) {
	return deposits;
}

The issue is the size of the deposit array can increase and grow in no upper limit. The size of the deposits array increase when receiveFund or receiveNFT is called:

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

Basically when calling the function receiveFunds:

function receiveFunds(
	address _funder,
	address _tokenAddress,
	uint256 _volume,
	uint256 _expiration
)

which is called by DepositManager#fundBountyToken

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

bytes memory funderUuidBytes = abi.encode(funderUuid);

An adversary can easiily spam and increase the size of the deposits array by keep depositing only 1 wei of fund and repeat the 1 wei deposit fundBounty transaction for maybe 100 times or 1000 times, which grows the size of the deposits.

Impact

BountyCore#getDeposits can run out of gas.

Code Snippet

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

Tool used

Manual Review

Recommendation

We recommend the protocol set minimum fund deposit threshold when fund the bounty contract.

Duplicate of #77