Inverse Finance contest - Jeiwan's results

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L91

Vulnerability details

Impact

Oracle's viewPrice function doesn't report a dampened price until getPrice is called and today's price is updated. This will impact the public read-only functions that call it:

• getCollateralValue;
• getCreditLimit (calls getCollateralValue);
• getLiquidatableDebt (calls getCreditLimit);
• getWithdrawalLimit.

These functions are used to get on-chain state and prepare values for write calls (e.g. calculate withdrawal amount before withdrawing or calculate a user's debt that can be liquidated before liquidating it). Thus, wrong values returned by these functions can cause withdrawal of a wrong amount or liquidation of a wrong debt or cause reverts.

Proof of Concept

// src/test/Oracle.t.sol
function test_viewPriceNoDampenedPrice_AUDIT() public {
    uint collateralFactor = market.collateralFactorBps();
    uint day = block.timestamp / 1 days;
    uint feedPrice = ethFeed.latestAnswer();

    //1600e18 price saved as daily low
    oracle.getPrice(address(WETH), collateralFactor);
    assertEq(oracle.dailyLows(address(WETH), day), feedPrice);

    vm.warp(block.timestamp + 1 days);
    uint newPrice = 1200e18;
    ethFeed.changeAnswer(newPrice);
    //1200e18 price saved as daily low
    oracle.getPrice(address(WETH), collateralFactor);
    assertEq(oracle.dailyLows(address(WETH), ++day), newPrice);

    vm.warp(block.timestamp + 1 days);
    newPrice = 3000e18;
    ethFeed.changeAnswer(newPrice);

    //1200e18 should be twoDayLow, 3000e18 is current price. We should receive dampened price here.
    // Notice that viewPrice is called before getPrice.
    uint viewPrice = oracle.viewPrice(address(WETH), collateralFactor);
    uint price = oracle.getPrice(address(WETH), collateralFactor);
    assertEq(oracle.dailyLows(address(WETH), ++day), newPrice);

    assertEq(price, 1200e18 * 10_000 / collateralFactor);

    // View price wasn't dampened.
    assertEq(viewPrice, 3000e18);
}

Tools Used

Manual review

Recommended Mitigation Steps

Consider this change:

--- a/src/Oracle.sol
+++ b/src/Oracle.sol
@@ -89,6 +89,9 @@ contract Oracle {
             uint day = block.timestamp / 1 days;
             // get today's low
             uint todaysLow = dailyLows[token][day];
+            if(todaysLow == 0 || normalizedPrice < todaysLow) {
+                todaysLow = normalizedPrice;
+            }
             // get yesterday's low
             uint yesterdaysLow = dailyLows[token][day - 1];
             // calculate new borrowing power based on collateral factor

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L353-L363

Vulnerability details

Impact

A borrower can quit their position and withdraw full collateral before their DBR deficit is replenished. This leaves the protocol with a bad debt. The borrower can borrow again using a different address and an escrow to avoid replenishing their DBR deficit.

Proof of Concept

// src/test/Market.t.sol
function testQuitWithDBRDeficit_AUDIT() public {
    gibWeth(user, wethTestAmount);
    gibDBR(user, wethTestAmount);

    vm.startPrank(user);
    deposit(wethTestAmount);
    uint borrowAmount = getMaxBorrowAmount(wethTestAmount);
    market.borrow(borrowAmount);

    // Simulating DBR deficit accruing.
    dbr.transfer(address(0), dbr.balanceOf(user));
    vm.warp(block.timestamp + 100);
    uint256 deficit = dbr.deficitOf(user);
    assertEq(deficit, 4312531709791983);

    // User's DBR deficit hasn't been replenished.

    // User repays full debt.
    market.repay(user, market.debts(user));

    // User withdraws full collateral.
    uint256 withdrawAmount = market.getWithdrawalLimit(user);
    market.withdraw(withdrawAmount);
    assertEq(DOLA.balanceOf(user), 0);
    assertEq(WETH.balanceOf(user), wethTestAmount);
    // User has 0 collateral.
    assertEq(market.getCollateralValue(user), 0);
    // User has 0 debt.
    assertEq(market.debts(user), 0);

    // However, user still has the deficit of DBR.
    assertEq(dbr.deficitOf(user), deficit);

    vm.stopPrank();

    // The deficit cannot be replenished because user has 0 collateral.
    vm.expectRevert("Exceeded collateral value");
    market.forceReplenish(user, deficit);
}

Tools Used

Manual review

Recommended Mitigation Steps

Consider this change:

--- a/src/Market.sol
+++ b/src/Market.sol
@@ -458,6 +458,7 @@ contract Market {
     @param amount The amount being withdrawn.
     */
     function withdrawInternal(address from, address to, uint amount) internal {
+        require(dbr.deficitOf(from) == 0, "Unreplenished DBR deficit");
         uint limit = getWithdrawalLimitInternal(from);
         require(limit >= amount, "Insufficient withdrawal limit");
         IEscrow escrow = getEscrow(from);

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L6 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L82 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L116

Vulnerability details

Impact

The Oracle contract calls latestAnswer function of a Chainlink oracle, which was deprecated. In case Chainling stops supporting the oracles with deprecated functions, the Oracle contract will has to be patched and redeployed.

Proof of Concept

Oracle.sol#L4-L7:

interface IChainlinkFeed {
    function decimals() external view returns (uint8);
    function latestAnswer() external view returns (uint); // @audit deprecated function
}

latestAnswer in the API reference documentation of Chainlink

Tools Used

Manual review

Recommended Mitigation Steps

Consider updating the interface and using latestRoundData instead of latestAnswer.