code-423n4 / 2023-07-amphora-findings

3 stars 2 forks source link

Gas Limit Vulnerability in vaultSummaries Function Due to Large Number of enabledTokens #403

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-amphora/blob/daae020331404647c661ab534d20093c875483e1/core/solidity/contracts/core/VaultController.sol#L988-L1012

Vulnerability details

Impact

The vaultSummaries function in the provided Solidity code has a potential issue that may lead to reverts or out-of-gas errors when there are a large number of enabledTokens. The function is designed to return the status of a range of vaults, including their token balances. However, if the number of enabledTokens is excessively high, the gas cost for fetching and storing the token balances in the loop might exceed the maximum gas limit, causing the function to fail. This can result in failed transactions and loss of user funds if not handled properly.

Proof of Concept

function vaultSummaries(uint96 _start, uint96 _stop) public view returns (VaultSummary[] memory _vaultSummaries) {
  if (_stop > vaultsMinted) _stop = vaultsMinted;
  _vaultSummaries = new VaultSummary[](_stop - _start + 1);
  for (uint96 _i = _start; _i <= _stop;) {
    IVault _vault = _getVault(_i);
    uint256[] memory _tokenBalances = new uint256[](enabledTokens.length);

    for (uint256 _j; _j < enabledTokens.length;) {
      _tokenBalances[_j] = _vault.balances(enabledTokens[_j]);

      unchecked {
        ++_j;
      }
    }
    _vaultSummaries[_i - _start] =
      VaultSummary(_i, _peekVaultBorrowingPower(_vault), this.vaultLiability(_i), enabledTokens, _tokenBalances);

    unchecked {
      ++_i;
    }
  }
}

In the above function, if there are a large number of enabledTokens, the inner loop that populates the _tokenBalances array for each vault might result in excessive gas consumption. When the total gas cost of the function exceeds the maximum gas limit allowed for a single transaction, the function execution will be reverted.

Tools Used

Manual review

Recommended Mitigation Steps

Optimize the vaultSummaries function to process a smaller subset of enabledTokens at a time, reducing the gas cost of the operation.

Assessed type

Other

c4-pre-sort commented 1 year ago

minhquanym marked the issue as duplicate of #98

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Invalid

Nabeel-javaid commented 1 year ago

Hey, Appreciate your judging but issues like these has already been marked as medium severity before. will really appreciate if you have another look at this