lil.eth
high
The _getAccountLiquidity() function, which uses a for-loop to iterate over all markets entered by a user, exhausts more gas as the number of markets increases. This can lead to a situation where a user with a high number of entered markets exceeds the block gas limit during the redemption process, making the redemption impossible.
As currently there is no mechanism implement to ensure this can't happen I believe it must be outlined for the security of this protocol.
In the design of the _getAccountLiquidity() function, it loops through all the markets a user has entered. Each additional market increases the computational complexity and thus, the gas cost of this function. When a user attempts to redeem assets, if the user has entered many markets, the gas cost for executing the redemption transaction escalates and can surpass the block gas limit, causing the transaction to fail.
This vulnerability creates an avenue to lock funds within the protocol. A user who enters an excessive number of markets inflates the gas cost for their redemption transaction beyond the block gas limit, thereby rendering the redemption transaction unexecutable and the assets unredeemable.
function _getAccountLiquidity(address user) internal view returns (uint256, uint256) {
...
address[] memory userEnteredMarkets = allEnteredMarkets[user]; //E Market array
for (uint256 i = 0; i < userEnteredMarkets.length; i++) {
DataTypes.Market storage m = markets[userEnteredMarkets[i]];
uint256 supplyBalance = m.userSupplies[user];
uint256 borrowBalance = _getBorrowBalance(m, user);
uint256 assetPrice = PriceOracleInterface(priceOracle).getPrice(userEnteredMarkets[i]);
require(assetPrice > 0, "invalid price");
uint256 collateralFactor = m.config.collateralFactor;
if (supplyBalance > 0 && collateralFactor > 0) {
uint256 exchangeRate = _getExchangeRate(m);
collateralValue += (supplyBalance * exchangeRate * assetPrice * hh) / 1e36 / FACTOR_SCALE;
}
....
}Manual Review
Implement a restriction on the number of markets a user can enter. The limit should be designed such that the highest possible gas cost for _getAccountLiquidity() does not exceed the block gas limit.
An alternative solution is to cache the value of a user's account liquidity after each transaction that modifies it. Instead of recalculating the liquidity for each redemption, the cached value is used, reducing the computational complexity and gas cost.
Refactoring the _getAccountLiquidity() function to calculate liquidity over multiple transactions is another possible approach. This would require adjustments to the contract's logic to handle intermediate states of redemption but would allow users who have entered a large number of markets to redeem their assets in parts.