code-423n4 / 2022-10-inverse-findings

0 stars 0 forks source link

Misleading code in BorrowController including no validation of the amounts to be borrowed. #558

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L389-L401 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/BorrowController.sol#L46-L49

Vulnerability details

Impact

BorrowController contract checks if any contract is in the borrow list. However, due to the use of msg.sender == tx.origin statement, it returns true for the EOA addresses.

It's also observed that any amount of borrowing is acceptable by the BorrowerController since require(borrowController.borrowAllowed(msg.sender, borrower, amount), "Denied by borrow controller"); is not validated at the BorrowController.

Proof of Concept

    function borrowAllowed(address msgSender, address, uint) public view returns (bool) {
        if(msgSender == tx.origin) return true;
        return contractAllowlist[msgSender];
    }

Permalink

Market contract calling BorrowController to validate the amount and the address;

    function borrowInternal(address borrower, address to, uint amount) internal {
        require(!borrowPaused, "Borrowing is paused");
        if(borrowController != IBorrowController(address(0))) {
            require(borrowController.borrowAllowed(msg.sender, borrower, amount), "Denied by borrow controller"); //@audit-info .<----- validation
        }
        uint credit = getCreditLimitInternal(borrower);
        debts[borrower] += amount;
        require(credit >= debts[borrower], "Exceeded credit limit");
        totalDebt += amount;
        dbr.onBorrow(borrower, amount);
        dola.transfer(to, amount);
        emit Borrow(borrower, amount);
    }

Permalink

Tools Used

Manual Review

Recommended Mitigation Steps

If the code intents to return a boolean for the contractAllowlist with the contract address, it should first be checked whether the caller is not coming from an EOA via;

require(msgSender != tx.origin)

Consider validating amounts.

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Insufficient quality