code-423n4 / 2024-06-size-findings

0 stars 0 forks source link

Incorrect checked address in `CapsLibrary.validateVariablePoolHasEnoughLiquidity` causes critical functions to be in a DoS state #385

Closed howlbot-integration[bot] closed 2 months ago

howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/CapsLibrary.sol#L67-L72

Vulnerability details

Impact

    function validateVariablePoolHasEnoughLiquidity(State storage state, uint256 amount) public view {
        uint256 liquidity = state.data.underlyingBorrowToken.balanceOf(address(state.data.variablePool));
        if (liquidity < amount) {
            revert Errors.NOT_ENOUGH_BORROW_ATOKEN_LIQUIDITY(liquidity, amount);
        }
    }

The validateVariablePoolHasEnoughLiquidity() function in the CapsLibrary contract is used by buyCreditMarket(), sellCreditMarket(), and liquidateWithReplacement() to verify that there is sufficient liquidity in AAVE for cash withdrawal.

However, the function incorrectly reads the USDC balance from the variablePool as liquidity. The actual liquidity is stored in the aUSDC token. This can be confirmed by checking the USDC balance of the following addresses on Ethereum at block height 20204585:

Consequently, after the mainnet deployment of the Size codebase, validateVariablePoolHasEnoughLiquidity() will use 0 as liquidity and thus revert. This would cause buyCreditMarket() and sellCreditMarket(), the two main functions, to enter a denial-of-service (DoS) state.

Simply put, the Size protocol will be completely non-functional due to this issue.

Proof of Concept

// SPDX-License-Identifier: Unlicense
pragma solidity ^0.8.0;

import {BaseTest} from "@test/BaseTest.sol";
import {USDC} from "@test/mocks/USDC.sol";
import {WETH} from "@test/mocks/WETH.sol";

contract PoC is BaseTest {
    function setUp() public override {
        vm.createSelectFork("eth");
        vm.rollFork(20174926);

        weth = WETH(payable(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2));
        usdc = USDC(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48);

        setupProduction({
            _owner: address(this),
            _feeRecipient: address(this),
            _weth: 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2,
            _usdc: 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48,
            _variablePool: 0x87870Bca3F3fD6335C3F4ce8392D69350B4fA4E2,
            _wethAggregator: 0x5f4eC3Df9cbd43714FE2740f5E3616155c5b8419,
            _usdcAggregator: 0x8fFfFfd4AfB6115b954Bd326cbe7B4BA576818f6,
            _sequencerUptimeFeed: address(0)
        });

        _labels();
    }

    function test_it() public {
        _deposit(alice, weth, 100e18);
        _deposit(alice, usdc, 100e6);
        _deposit(bob, weth, 100e18);
        _deposit(bob, usdc, 100e6);
        _sellCreditLimit(alice, 0.03e18, 365 days);

        uint256 amountIn = 50e6;
        uint256 tenor = 365 days;

        // will throw  NOT_ENOUGH_BORROW_ATOKEN_LIQUIDITY error
        _buyCreditMarket(bob, alice, amountIn, tenor, true);
    }
}

This PoC use a fork test to show that the incorrect liquidity reading in validateVariablePoolHasEnoughLiquidity() will cause a DoS on the buyCreditMarket() function.

To run the this PoC, first update the forge-std library to at least version 1.8.0 (The USDC deal issue is fixed in this version). Then set the RPC for Ethereum mainnet in foundry.toml. Finally, run forge test --mc PoC -vvvv. The result is as follows:

[FAIL. Reason: NOT_ENOUGH_BORROW_ATOKEN_LIQUIDITY(0, 50000000 [5e7])] test_it() (gas: 2237661)

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

Read the liquidity from aToken instead of variablePool in validateVariablePoolHasEnoughLiquidity():

- uint256 liquidity = state.data.underlyingBorrowToken.balanceOf(address(state.data.variablePool));
+ address aToken = state.data.variablePool.getReserveData(address(state.data.underlyingBorrowToken)).aTokenAddress;
+ uint256 liquidity = state.data.underlyingBorrowToken.balanceOf(address(aToken));

Assessed type

DoS

c4-judge commented 2 months ago

hansfriese marked the issue as satisfactory

c4-judge commented 2 months ago

hansfriese changed the severity to 2 (Med Risk)