Lines of code

https://github.com/code-423n4/2024-06-size/blob/main/src/Size.sol#L162

Vulnerability details

Impact

Users can withdraw their deposited tokens by calling Size::withdraw, which withdraws both the underlying collateral and borrow tokens, according to the user's input. At the end of the withdrawal, it checks if the user CR is above a certain limit to make sure he's not withdrawing collateral that puts him underwater. However, the withdrawal function checks the CR even when withdrawing the underlying borrow tokens.

This poses an issue, where if a user is liquidatable and has some underlying borrow tokens deposited he won't be able to withdraw those. In theory, he should be able to do so, because these underlying borrow tokens don't contribute to the user's CR.

As a result, users won't be able to withdraw their deposited underlying borrow tokens.

Proof of Concept

function test_borrower_cant_withdraw_borrow_tokens() public {
    _deposit(bob, weth, 100e18);
    _deposit(bob, usdc, 5_000e6);

    _deposit(alice, weth, 0.5e18);
    _deposit(alice, usdc, 100e6);

    int256[] memory aprs = new int256[](1);
    uint256[] memory tenors = new uint256[](1);
    uint256[] memory marketRateMultipliers = new uint256[](1);

    aprs[0] = 0.2e18;
    tenors[0] = 365 days;
    marketRateMultipliers[0] = 0;

    // Alice creates a limit order
    vm.prank(alice);
    size.sellCreditLimit(
        SellCreditLimitParams({
            curveRelativeTime: YieldCurve({
                tenors: tenors,
                marketRateMultipliers: marketRateMultipliers,
                aprs: aprs
            })
        })
    );

    // Bob buys credit from Alice (lends to Alice)
    vm.prank(bob);
    size.buyCreditMarket(
        BuyCreditMarketParams({
            borrower: alice,
            creditPositionId: type(uint256).max,
            amount: 300e6,
            tenor: 365 days,
            deadline: block.timestamp,
            minAPR: 0,
            exactAmountIn: true
        })
    );

    // Collateral price drops
    _setPrice(1_000e18);

    uint256 aliceCR = size.collateralRatio(alice);
    uint256 crOpening = size.riskConfig().crOpening;

    // Alice is unable to withdraw borrow tokens
    vm.prank(alice);
    vm.expectRevert(
        abi.encodeWithSelector(
            Errors.CR_BELOW_OPENING_LIMIT_BORROW_CR.selector,
            alice,
            aliceCR,
            crOpening
        )
    );
    size.withdraw(
        WithdrawParams({amount: 50e6, token: address(usdc), to: alice})
    );
}

Tools Used

Manual review

Recommended Mitigation Steps

Instead of checking for the user's CR on every withdrawal, only do it when withdrawing the underlying collateral token, by having something like the following in Size::withdraw:

if (params.token == address(state.data.underlyingCollateralToken)) {
    state.validateUserIsNotUnderwater(msg.sender);
}

Assessed type

Invalid Validation

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

Invalid CR validation in `Size::withdraw`, blocking liquidatable users from withdrawing their borrow tokens #433