sherlock-audit / 2024-05-elfi-protocol-judging

11 stars 7 forks source link

pwning_dev - Improper Handling of Array Length #250

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 4 months ago

pwning_dev

Medium

Improper Handling of Array Length

pwning_dev

Improper Handling of Array Length

Summary

Vulnerability Detail

The getPortfolioNetValue, getTotalUsedValue and getCrossUsedValueAndBorrowingValue functions iterate over an array of tokens without checking the length of the array explicitly. This can lead to potential out-of-bounds errors or inefficiencies.

address[] memory tokens = accountProps.getTokens();
for (uint256 i; i < tokens.length; i++) {
    // Processing logic
}

Impact

  1. Attack Scenario: An attacker could potentially manipulate accountProps.getTokens() to return an empty array (tokens.length = 0). This could lead to an attempt to access tokens[0], resulting in a runtime error or unexpected behavior.
  2. Attack Scenario: An attacker could manipulate accountProps.getTokens() to return an excessively large array. This could lead to a denial-of-service (DoS) attack by exhausting gas limits, preventing legitimate transactions from being processed.

Code Snippet

https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/main/elfi-perp-contracts/contracts/process/AccountProcess.sol#L97C1-L99C1 https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/main/elfi-perp-contracts/contracts/process/AccountProcess.sol#L59C1-L61C1

Tool used

Manual Review

Recommendation

address[] memory tokens = accountProps.getTokens();
uint256 tokensLength = tokens.length;
for (uint256 i = 0; i < tokensLength; i++) {
    if (!AppTradeTokenConfig.getTradeTokenConfig(tokens[i]).isSupportCollateral) {
        continue;
    }
    Account.TokenBalance memory tokenBalance = accountProps.tokenBalances[tokens[i]];
    totalNetValue = totalNetValue.add(_getTokenNetValue(tokens[i], tokenBalance, oracles));
}
nevillehuang commented 4 months ago

Invalid, tokens are whitelisted by config Role, so invalid per sherlock rules:

  1. Out of Gas: Issues that result in Out of Gas errors either by the malicious user filling up the arrays or there is a practical call flow that results in OOG can be considered a valid medium or in cases of blocking all user funds forever maybe a valid high. Exception: In case the array length is controlled by the trusted admin/owner or the issue describes an impractical usage of parameters to reach OOG state then these submissions would be considered as low.