sherlock-audit / 2023-10-aloe-judging

9 stars 6 forks source link

Nyx - Wrong repay amount inside the liquidate function #77

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

Nyx

high

Wrong repay amount inside the liquidate function

Summary

Vulnerability Detail

When using the liquidate() function, repayable0 and repayable1 are calculated without interest. During repayment in the repay() function, the _load function accrues interests and updates the borrow index.

// Accrue interest and update reserves
        (Cache memory cache, ) = _load();

        unchecked {
            // Convert `amount` to `units`
            units = (amount * BORROWS_SCALER) / cache.borrowIndex;
            if (!(units < b)) {
                units = b - 1;

                uint256 maxRepay = (units * cache.borrowIndex).unsafeDivUp(BORROWS_SCALER);
                require(b > 1 && amount <= maxRepay, "Aloe: repay too much");
            } 

            // Track borrows
            borrows[beneficiary] = b - units;
            cache.borrowBase -= units;
        }

The units will be lower than expected due to updated borrowIndex, and the borrows[beneficiary] wont be zero as expected.

Impact

Due to an incorrect repayment amount, the borrower balance cannot be fully paid, which may result in bad debt.

Code Snippet

Liquidator.t.sol

function test_spec_repayDAIAndETHWithUniswapPosition() public {
        //@audit
        uint256 strain = 1;
        // give the account 1 DAI and 0.1 ETH
        deal(address(asset0), address(account), 1.1e18);
        deal(address(asset1), address(account), 0.1e18);

        // borrow 200 DAI and 20 ETH
        bytes memory data = abi.encode(Action.BORROW, 200e18, 20e18);
        account.modify(this, data, (1 << 32));

        // create a small Uniswap position
        data = abi.encode(Action.UNI_DEPOSIT, 0, 0);
        account.modify(this, data, (1 << 32));

        _setInterest(lender0, 10010);
        _setInterest(lender1, 10010);
        assertEq(lender0.borrowBalance(address(account)), 200.2e18);
        assertEq(lender1.borrowBalance(address(account)), 20.02e18);

        vm.warp(10 days);

        // Even if the strain is one, debt is not cleared completely.
        account.liquidate(this, bytes(""), strain, (1 << 32));

        console.log(lender0.borrowBalance(address(account)));
        console.log(lender1.borrowBalance(address(account)));

        vm.warp(1000 days);
        lender0.accrueInterest();
        lender1.accrueInterest();
        vm.expectRevert();
        account.liquidate(this, bytes(""), strain, (1 << 32));
    }

Tool used

Manual Review

Recommendation

the accrueInterest() function should be called inside the liquidate function.

Duplicate of #41

sherlock-admin2 commented 1 year ago

1 comment(s) were left on this issue during the judging contest.

MohammedRizwan commented:

valid