Invalid CR validation in Size::compensate, blocking users from compensating their loans

[howlbot-integration]

Lines of code

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

Vulnerability details

Impact

Users can use their credit to compensate for some of their debt, this is done by calling Size::compensate. Compensate sometimes contains collateral transfers, however, it only happens if fragmentation happens on a credit position. But the protocol is checking if the user is liquidatable on every compensate TX, even if it doesn't include collateral transfers.

If a liquidatable borrower (has >1 loan), tries to compensate a loan with an exact credit position he owns, but he's unable to compensate 1 loan because he's still liquidatable with the remaining loan, this is because it's checking the user's CR, knowing that no fragmentation happened.

So a user won't be able to compensate his loan because of an invalid CR check, where it shouldn't be triggered as no fragmentation happened.

Proof of Concept

The below test how the borrower is blocked from compensating his loan. To confirm that this works as expected, temporarily remove state.validateUserIsNotUnderwater(msg.sender); from Size::compensate, and verify that the below test doesn't revert.

function test_borrower_cant_compensate() public {
    _deposit(bob, weth, 100e18);
    _deposit(bob, usdc, 5_000e6);
    _deposit(candy, weth, 100e18);
    _deposit(candy, usdc, 500e6);
    _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 creates a limit order
    vm.prank(candy);
    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: 50e6,
            tenor: 365 days,
            deadline: block.timestamp,
            minAPR: 0,
            exactAmountIn: true
        })
    );
    uint256 creditPositionId_1 = type(uint256).max / 2;
    uint256 debtPositionId_1 = 0;

    // Candy buys credit from Alice (lends to Alice)
    vm.prank(candy);
    size.buyCreditMarket(
        BuyCreditMarketParams({
            borrower: alice,
            creditPositionId: type(uint256).max,
            amount: 300e6,
            tenor: 365 days,
            deadline: block.timestamp,
            minAPR: 0,
            exactAmountIn: true
        })
    );
    uint256 creditPositionId_2 = creditPositionId_1 + 1;
    uint256 debtPositionId_2 = debtPositionId_1 + 1;

    // Alice buys credit from Candy (lends to Candy)
    vm.prank(alice);
    size.buyCreditMarket(
        BuyCreditMarketParams({
            borrower: candy,
            creditPositionId: type(uint256).max,
            amount: 50e6,
            tenor: 365 days,
            deadline: block.timestamp,
            minAPR: 0,
            exactAmountIn: false
        })
    );
    uint256 creditPositionId_3 = creditPositionId_2 + 1;
    uint256 debtPositionId_3 = debtPositionId_2 + 1;

    // Collateral price drops
    _setPrice(900e18);

    // Alice is now liquidatable
    assertLt(size.collateralRatio(alice), size.riskConfig().crLiquidation);

    uint256 amountToCompensate = 50e6;
    // Credit_3 == amountToCompensate, i.e. no fragmentation happens
    assertEq(
        size.getCreditPosition(creditPositionId_3).credit,
        amountToCompensate
    );

    // Alice tries to repay debt_1 with credit_3, reverts
    vm.prank(alice);
    vm.expectRevert(
        abi.encodeWithSelector(
            Errors.USER_IS_UNDERWATER.selector,
            alice,
            1216216216216216216 // CR after the first loan is compensated
        )
    );
    size.compensate(
        CompensateParams({
            creditPositionWithDebtToRepayId: creditPositionId_1,
            creditPositionToCompensateId: creditPositionId_3,
            amount: amountToCompensate
        })
    );
}

Tools Used

Manual review

Recommended Mitigation Steps

Instead of checking if the user is underwater on every compensate action, only check it if some credit fragmentation happens. In other words, don't check for the user's CR if no collateral tokens were moved.

Assessed type

Invalid Validation

[c4-judge]

hansfriese marked the issue as satisfactory