sherlock-audit / 2023-05-ironbank-judging

2 stars 2 forks source link

shealtielanz - Malicious Borrower can `repay` debt in a market with actual paying. #365

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

shealtielanz

high

Malicious Borrower can repay debt in a market with actual paying.

Line with Bug

Summary

the _getborrowerbalance function which is used in the repay function could round its return value to zero, which could be manipulated by a malicious borrower, where the borrower borrows a calculated amount of funds that would be rounded to zero by the _getBorrowerBalance function and repay with a zero amount of tokens.

Vulnerability Detail

The _getBorrowBalance is is susceptible to potential rounding errors that may result in returning a value of 0, even if the actual borrow balance should be non-zero.

    function _getBorrowBalance(DataTypes.Market storage m, address user) internal view returns (uint256) {
        DataTypes.UserBorrow memory b = m.userBorrows[user];
        if (b.borrowBalance == 0) {
            return 0;
        }
   // @audit there is no check to ensure the rounded value has been rounded to zero
        return (b.borrowBalance * m.borrowIndex) / b.borrowIndex;
    }

As you can see it is used to get the borrow balance of the to address:

    function _repay(DataTypes.Market storage m, address from, address to, address market, uint256 amount)
        internal
        returns (uint256)
    {
        uint256 borrowBalance = _getBorrowBalance(m, to);
        if (amount == type(uint256).max) {
            amount = borrowBalance;
        }
   // --more code--
         if (m.userSupplies[to] == 0 && newUserBorrowBalance == 0) {
            _exitMarket(market, to);
        }
          IERC20(market).safeTransferFrom(from, address(this), amount);

        emit Repay(market, from, to, amount, newUserBorrowBalance, newTotalBorrow);

as you can see, if the borrow balance is rounded to zero the amount paid to the IronBank contract would be zero, while also making the user's borrow balance set to zero and even exit the market.

Impact

Malicious Borrowers can borrow calculated amounts that would certainly be rounded to zero when the _getBorrowBalance and repay their loans without actually sending anything(by sending zero amount of tokens) to the contract.

Code Snippet

Manual Review

Recommendation

The _getBorrowBalance may be re-written like this to prevent rounding to zero issues.

   function _getBorrowBalance(DataTypes.Market storage m, address user) internal view returns (uint256) {
        DataTypes.UserBorrow memory b = m.userBorrows[user];

        if (b.borrowBalance == 0) {
            return 0;
        }
       // borrowBalanceWithInterests = borrowBalance * marketBorrowIndex / userBorrowIndex
       uint balance =  (b.borrowBalance * m.borrowIndex) / b.borrowIndex;
       //@audit  check if it is rounded to zero.
       require(balance != 0, "zero is bad for business");

       return balance;
    }

OR The values used to get the borrowBalanceWithInterests should be raised by a certain factor to mitigate the loss of precision and rounding to zero errors.