sherlock-audit / 2023-05-ironbank-judging

2 stars 2 forks source link

oxchryston - Malicious `credit` `account` can ` drain` the `contract` of it's funds #364

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

oxchryston

high

Malicious credit account can drain the contract of it's funds

Summary

Credit account can borrow above creditlimit using the borrow function, thereby withdrawing more funds than required or even draining the contract.

Vulnerability Detail

In the function borrow, the check for if caller is creditaccount and if borrow amount is greater than creditlimit comes after the safetransfer of funds. A malicious creditaccount can potentially drain the contract of it's funds.

  function borrow(address from, address to, address market, uint256 amount)
        external
        nonReentrant
        isAuthorized(from)
    {
//More code...
// Update storage.
        unchecked {
            m.totalCash -= amount;
        }
        m.totalBorrow = newTotalBorrow;
        m.userBorrows[from].borrowBalance = newUserBorrowBalance;
        m.userBorrows[from].borrowIndex = m.borrowIndex;

        // Enter the market.
        if (amount > 0) {
            _enterMarket(market, from);
        }

        IERC20(market).safeTransfer(to, amount);

        if (isCreditAccount(from)) {
            require(from == to, "credit account can only borrow to itself");
            require(creditLimits[from][market] >= newUserBorrowBalance, "insufficient credit limit");
        } else {
            _checkAccountLiquidity(from);
        }

        emit Borrow(market, from, to, amount, newUserBorrowBalance, newTotalBorrow);
    }

Impact

Contract can be drained by malicious creditaccount.

Code Snippet

https://github.com/sherlock-audit/2023-05-ironbank/blob/9ebf1702b2163b55479624794ab7999392367d2a/ib-v2/src/protocol/pool/IronBank.sol#L351

Tool used

Manual Review

Recommendation

The check for if from is creditaccount should be written before safetransfer of funds to caller or use a modifier to regulate the creditaccount