Adjusting a CDP allows a user to reduce their collateral under the threshold enforced by the protocol

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-06-badger/blob/main/ebtc-zap-router/src/EbtcLeverageZapRouter.sol#L403-L467

Vulnerability details

Impact

Users have the ability to reduce the risk of their position and bypass the protocol design restrictions, effectively reducing the rewards intended for liquidators and getting rid of the 0.2 stETH gas stipend in the position.

Proof of concept

The protocol enforces users to deposit at least 2 stETH, as stated in the documentation : "CDPs must have a size of at least 2 stETH of collateral"

A "gas stipend" of 0.2 stETH is required to be transferred "in addition to the collateral" as an incentive for the liquidators "to cover the transaction's gas cost".

This essentially means a user must deposit 2.2 stETH when a CDP is opened. This is enforced in _openCdp() through the _requireAtLeastMinNetStEthBalance() function.

https://github.com/code-423n4/2024-06-badger/blob/main/ebtc-zap-router/src/EbtcLeverageZapRouter.sol#L217

// @audit LIQUIDATOR_REWARD == 2e17
_requireAtLeastMinNetStEthBalance(_stEthDepositAmount - LIQUIDATOR_REWARD);

...

function _requireAtLeastMinNetStEthBalance(uint256 _stEthBalance) internal pure {
    // @audit `MIN_NET_STETH_BALANCE` == 2e18
    require(
        _stEthBalance >= MIN_NET_STETH_BALANCE,
        "ZapRouterBase: Cdp's net stEth balance must not fall below minimum"
    );
}

However, a user has the ability to adjust his CDP to reduce his collateral under this 2.2 stETH because it does not implement sufficient checks.

Here is a modified version of the test_adjustCdp_debtDecrease_stEth test that demonstrates the collateral being under 2.2 stETH after adjustment

function test_adjustCdp_under_threshold() public {
    seedActivePool();

    (address user, bytes32 cdpId) = createLeveragedPosition(MarginType.stETH);

    IEbtcZapRouter.PositionManagerPermit memory pmPermit = createPermit(user);
    uint256 debtChange = 0.99e18;
    uint256 marginBalance = 2.87e18;
    uint256 collValue = _debtToCollateral(debtChange) * 10004 / 10000;
    _before();
    vm.startPrank(user);
    leverageZapRouter.adjustCdp(
        cdpId, 
        _getAdjustCdpParams(debtChange, -int256(debtChange), -int256(collValue), -int256(marginBalance), false), 
        abi.encode(pmPermit),
        _getExactInCollateralToDebtTradeData(collValue)
    );
    vm.stopPrank();
    _after();

    // Test zap fee (no fee if debt decrease)
    assertEq(eBTCToken.balanceOf(testFeeReceiver), 1e18 * defaultZapFee / 10000); 

    _checkZapStatusAfterOperation(user);

    pmPermit = createPermit(user);

    vm.startPrank(user);
    (uint256 debt, uint256 collShares) = cdpManager.getSyncedDebtAndCollShares(cdpId);

    assertLt(
        collShares,
        2.004e18
    );

    vm.stopPrank();
}

Tools used

Fuzzing, manual analysis

Recommended mitigation steps

Add additional checks in the adjust operation to enforce the 2.2 stETH collateral in the CDP.

Assessed type

Context

[wtj2021]

Ebtc enforces a 2 stETH minimum collateral requirement. The gas stipend is not considered to be part of that. The require statement in EbtcLeverageZapRouter.openCdp is meant to ensure that the user will provide the minimum collateral plus the gas stipend and terminate early if necessary. It does not mean that the minimum collateral is 2.2 stETH. In adjustCdp, we are not checking the gas stipend because it is already in the system. So, the user should be able to adjust the collateral down to the 2 stETH minimum. The test shows that the invariant holds because collShares after adjustCdp is above 2 stETH.

The invariant is checked inside the ebtc protocol itself, not the router. The router checks are for early termination purposes. As such, the PoC does not demonstrate a broken invariant.

[alex-ppg]

The Warden attempts to demonstrate a vulnerability in the amounts permitted by the EbtcLeverageZapRouter, however, as the Sponsor has correctly outlined, the submission stems from a misconception about the permitted amounts.

As we can see in the BorrowerOperations::_openCdp function, the amount actually credited to the CDP does not include the liquidation incentive. (1, 2, 3)

As such, the funds are necessary for the creation of the CDP but do not need to be accounted for when adjusting a CDP as the CDP was never credited with them.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid

[Odhiambo526]

@alex-ppg Thank you for Judging,

In the developers' response @wtj2021 , it was mentioned that

the gas stipend (LIQUIDATOR_REWARD) is not considered part of the 2 stETH collateral requirement and that the router's checks are for early termination purposes.

However, if adjustCdp allows users to lower collateral below the 2 stETH threshold without considering the gas stipend, doesn't this contradict the protocol's intent to maintain a minimum collateral level for security purposes? Shouldn't there be a check post-adjustment to ensure collateral consistency with the minimum requirement, including the impact of gas stipends? The _requireAtLeastMinNetStEthBalance function in openCdp function ensures that users deposit at least 2 stETH (net of the LIQUIDATOR_REWARD).

However, when using adjustCdp, users can reduce their collateral below this minimum without a similar check being enforced. Evidence:

1. Minimum Collateral Check in openCdp: The openCdp function requires that a new CDP must have a minimum of 2 stETH in collateral. This is ensured by the _requireAtLeastMinNetStEthBalance function, which checks if the deposited stETH minus the LIQUIDATOR_REWARD (0.2 stETH) is at least 2 stETH.

   
   // Ensure at least 2 stETH net of the liquidator reward is deposited
   _requireAtLeastMinNetStEthBalance(_stEthDepositAmount - LIQUIDATOR_REWARD);

function _requireAtLeastMinNetStEthBalance(uint256 _stEthBalance) internal pure { require( _stEthBalance >= MIN_NET_STETH_BALANCE, // MIN_NET_STETH_BALANCE == 2e18 (2 stETH) "ZapRouterBase: Cdp's net stEth balance must not fall below minimum" ); }

2. Lack of Similar Check in `adjustCdp`:
In the `adjustCdp` function, users can modify their CDP by adjusting the collateral and debt levels. However, the code does not enforce a check to ensure that the remaining collateral after adjustment is at least 2 stETH.
Potential Code Path Leading to the Issue:
A user could reduce the collateral to just above 2 stETH or even exactly 2 stETH, potentially dropping below the required minimum when accounting for the `LIQUIDATOR_REWARD`.
```solidity
// adjustCdp function might allow collateral reduction without minimum check
leverageZapRouter.adjustCdp(
    cdpId, 
    _getAdjustCdpParams(debtChange, -int256(debtChange), -int256(collValue), -int256(marginBalance), false), 
    abi.encode(pmPermit),
    _getExactInCollateralToDebtTradeData(collValue)
);

Consequence: This discrepancy means that while the openCdp function enforces the minimum net collateral requirement, the adjustCdp function does not, potentially allowing users to reduce the collateral below the intended minimum. This could lead to under-collateralization of the CDP, which poses a risk to the protocol's stability, especially in cases where the liquidator reward must be deducted from the collateral during liquidation.

Can you clarify how the protocol ensures that the collateral does not fall below 2 stETH after adjustments? The current logic does not appear to prevent under-collateralization, potentially exposing the protocol to liquidation risks.

[alex-ppg]

Hey @Odhiambo526, thank you for your PJQA contribution. As the code indicates, the gas stipend is ignored from the user's deposit. This means that it is "diluted" to the pool for refund purposes and thus will always be available regardless of the actual collateral level of the user's position. In simple terms, the gas stipend is never credited to the user as a deposit and thus the user cannot affect its existence in the pool by reducing their deposit.

[0xGreed]

I would like to bring up the following observations that may cause an issue regarding the checks in EbtcLeverageZapRouter during adjustCdp() :

In the scope of this audit, the MIN_NET_STETH_BALANCE variable is declared in the ZapRouterBase contract which EbtcLeverageZapRouter inherits from.

This variable is used during openCdp() to make sure the minimum requirements holds.

However in adjustCdp() this requirement is never verified. In fact, it is verified in BorrowerOperations that is involved in the process. The issue is that BorrowerOperations also maintains its own MIN_NET_STETH_BALANCE variable.

This can lead to inconsistencies because the same "minimum value" could be de-synchronized between the 2 parties. If the MIN_NET_STETH_BALANCE changes in EbtcLeverageZapRouter or vice-versa, the requirements in adjustCdp() will likely fail.

Thus, there is an issue in the adjustCdp() function where the minimum size of the position is not verified with the EbtcLeverageZapRouter values while they are during openCdp().