Contracts V2 Updates - Moonwell

In the router contract, The redeem function aims to redeem mTokens equivalent to mTokenRedeemAmount. The call mToken.redeem(mTokenRedeemAmount); is responsible for the redemption action.

In the event of an error, the mToken.redeem() function from Compound's mToken contract does not revert, but instead returns a non-zero error code as an uint. This behavior deviates from the standard Solidity function behavior that typically reverts in case of an error.

The redeem() function in the MTOKEN contract does not check the return value of mToken.redeem(mTokenRedeemAmount);. If this redemption operation fails (returns a non-zero error code), the contract still proceeds with the remaining operations, leading to a silent failure. As a result, the contract behaves as if tokens were redeemed when they were not, creating a discrepancy between the actual and perceived balance of mtokens and eth.

• Code Location

    function redeem(uint256 mTokenRedeemAmount, address recipient) external {
        IERC20(address(mToken)).safeTransferFrom(
            msg.sender,
            address(this),
            mTokenRedeemAmount
        );

        mToken.redeem(mTokenRedeemAmount);

        weth.withdraw(weth.balanceOf(address(this)));

        (bool success, ) = payable(recipient).call{
            value: address(this).balance
        }("");
        require(success, "WETHRouter: ETH transfer failed");
    }

Step 1 : An external actor (say, an address 'A') calls the redeem() function with a certain mTokenRedeemAmount and recipient.

Step 2 : The function starts by transferring mTokenRedeemAmount of mTokens from 'A' to the contract itself. This is done via the IERC20(address(mToken)).safeTransferFrom(msg.sender, address(this), mTokenRedeemAmount); statement.

Step 3 : Next, the function attempts to redeem the mTokens that have just been transferred to the contract, using mToken.redeem(mTokenRedeemAmount);. But, for some reason, this redemption fails. In normal circumstances, this failure should cause the transaction to revert. However, due to the atypical behavior of the mToken.redeem() method (it does not revert on failure but returns a non-zero uint instead), the execution continues to the next line.

Step 4 : Now, the contract attempts to convert its entire WETH balance to ETH via weth.withdraw(weth.balanceOf(address(this)));. Since the redemption in step 3 failed, this step should not result in any additional ETH being added to the contract. However, let's assume that the contract already had some ETH balance before the transaction began.

Step 5 : The contract then tries to transfer its entire ETH balance to the recipient specified in step 1. Despite the failed redemption, the function ends up transferring the contract's existing ETH balance to the recipient.

It is recommended to add the return value validation.

Remediation Plan

SOLVED: The Moonwell Finance team solved the issue by adding the return value validation.

Commit ID: c39f98bdc9dd4e448ba585923034af1d47f74dfa

In the mintWithPermit function, the implementation invokes the underlying token's permit() function and proceeds with the assumption that the operation was successful, without verifying the outcome. However, certain tokens may not adhere to the IERC20Permit standard. For example, the DAI Stablecoin utilizes a permit() function that deviates from the reference implementation. This lack of verification may lead to inconsistencies and unexpected behavior when interacting with non-conforming tokens.

• Code Location

    function mintWithPermit(
        uint mintAmount,
        uint deadline,
        uint8 v, bytes32 r, bytes32 s
    ) override external returns (uint) {

        // Go submit our pre-approval signature data to the underlying token
        IERC20Permit(underlying).permit(
            msg.sender, address(this),
            mintAmount, deadline,
            v, r, s
        );
        (uint err,) = mintInternal(mintAmount);
        return err;
    }

• DAI Code

pragma solidity =0.5.12;

contract Dai is LibNote {

    // --- Approve by signature ---
    function permit(address holder, address spender, uint256 nonce, uint256 expiry,
                    bool allowed, uint8 v, bytes32 r, bytes32 s) external
    {
        bytes32 digest =
            keccak256(abi.encodePacked(
                "\x19\x01",
                DOMAIN_SEPARATOR,
                keccak256(abi.encode(PERMIT_TYPEHASH,
                                     holder,
                                     spender,
                                     nonce,
                                     expiry,
                                     allowed))
        ));

        require(holder != address(0), "Dai/invalid-address-0");
        require(holder == ecrecover(digest, v, r, s), "Dai/invalid-permit");
        require(expiry == 0 || now <= expiry, "Dai/permit-expired");
        require(nonce == nonces[holder]++, "Dai/invalid-nonce");
        uint wad = allowed ? uint(-1) : 0;
        allowance[holder][spender] = wad;
        emit Approval(holder, spender, wad);
    }
}

It is recommended to use the safePermit function.

Remediation Plan

SOLVED: The Moonwell Finance team solved the issue by using the safePermit function.

In the _addEmissionConfig function, which is responsible for creating new market emission configurations, there is no validation check for the _endTime parameter. This oversight may lead to the creation of markets with incorrect or unreasonable end times, resulting in wrong market index calculations and potentially impacting the overall functioning of the system.

• Code Location

    function _addEmissionConfig(
        MToken _mToken,
        address _owner,
        address _emissionToken,
        uint _supplyEmissionPerSec,
        uint _borrowEmissionsPerSec,
        uint _endTime
    ) external {
        requireComptrollersAdmin();

...
        MarketConfig memory config = MarketConfig({
            // Set the owner of the reward distributor config
            owner: _owner,
            // Set the emission token address
            emissionToken: _emissionToken,

            // Set the time that the emission campaign should end at
            endTime: _endTime,

            // Initialize the global supply
            supplyGlobalTimestamp: safe32(block.timestamp, "block timestamp exceeds 32 bits"),
            supplyGlobalIndex: initialIndexConstant,

            // Initialize the global borrow index + timestamp
            borrowGlobalTimestamp: safe32(block.timestamp, "block timestamp exceeds 32 bits"),
            borrowGlobalIndex: initialIndexConstant,

            // Set supply and reward borrow speeds
            supplyEmissionsPerSec: _supplyEmissionPerSec,
            borrowEmissionsPerSec: _borrowEmissionsPerSec
        });
...
    }

    function createDistributorWithRoundValuesAndConfig(uint tokensToMint, uint supplyEmissionsPerSecond, uint borrowEmissionsPerSecond) internal returns (MultiRewardDistributor distributor) {
        faucetToken.allocateTo(address(this), tokensToMint);
        faucetToken.approve(address(mToken), tokensToMint);

        MultiRewardDistributor distributorHarness = new MultiRewardDistributor(
            address(comptroller), address(this)
        );

        // 1 year of rewards
        uint endTime = block.timestamp - (60 * 60 * 24 * 365);

        // Add config + send emission tokens
        emissionToken.allocateTo(address(distributorHarness), 100e18);
        distributorHarness._addEmissionConfig(
            mToken,
            address(this),
            address(emissionToken),
            supplyEmissionsPerSecond,
            borrowEmissionsPerSecond,
            endTime
        );

        return distributorHarness;
    }

It is recommended to add a validation check for the _endTime parameter.

Remediation Plan

SOLVED: The Moonwell Finance team solved the issue by adding the _endTime validation.

The executeProposal() function in the current smart contract is responsible for parsing and verifying VAAs (Validators Aggregated Attestations) and then executing transactions based on these VAAs. The function does not verify the Chain ID or the receiver address (recipient of the transaction).

The absence of chain ID and receiver address verification could lead to significant security issues. Since the chain ID and recipient address are not checked, an attacker can craft a VAA to target an address on another chain, causing a cross-chain replay attack.

• Code Location

   function _executeProposal(bytes memory VAA, bool overrideDelay) private {
        // This call accepts single VAAs and headless VAAs
        (
            IWormhole.VM memory vm,
            bool valid,
            string memory reason
        ) = wormholeBridge.parseAndVerifyVM(VAA);

        require(valid, reason); /// ensure VAA parsing verification succeeded

        if (!overrideDelay) {
            require(
                queuedTransactions[vm.hash].queueTime != 0,
                "TemporalGovernor: tx not queued"
            );
            require(
                queuedTransactions[vm.hash].queueTime + proposalDelay <=
                    block.timestamp,
                "TemporalGovernor: timelock not finished"
            );
        } else if (queuedTransactions[vm.hash].queueTime == 0) {
            /// if queue time is 0 due to fast track execution, set it to current block timestamp
            queuedTransactions[vm.hash].queueTime = block.timestamp.toUint248();
        }



        require(
            !queuedTransactions[vm.hash].executed,
            "TemporalGovernor: tx already executed"
        );

        queuedTransactions[vm.hash].executed = true;

        address[] memory targets; /// contracts to call
        uint256[] memory values; /// native token amount to send
        bytes[] memory calldatas; /// calldata to send
        (, targets, values, calldatas) = abi.decode(
            vm.payload,
            (address, address[], uint256[], bytes[])
        );

        /// Interaction (s)

        _sanityCheckPayload(targets, values, calldatas);

        for (uint256 i = 0; i < targets.length; i++) {
            address target = targets[i];
            uint256 value = values[i];
            bytes memory data = calldatas[i];

            // Go make our call, and if it is not successful revert with the error bubbling up
            (bool success, bytes memory returnData) = target.call{value: value}(
                data
            );

            /// revert on failure with error message if any
            require(success, string(returnData));

            emit ExecutedTransaction(target, value, data);
        }
    }

Step 1 : An attacker crafts a wormhole message that appears to be valid but is intended for a different chain (different chain ID) or is directed to an unintended recipient address.

Step 2 : The attacker submits this crafted payload to the _executeProposal() function in the smart contract.

Step 3 : Since there are no checks in place for the chain ID or recipient address, the function treats the VAA as valid and begins to execute the transaction(s) specified in the VAA payload.

It is recommended to add the necessary validations.

Remediation Plan

SOLVED: The Moonwell Finance team solved the issue by adding the necessary validations.

Commit ID: c39f98bdc9dd4e448ba585923034af1d47f74dfa

The _updateBorrowSpeed function in the smart contract is responsible for updating the borrow emission speed for the specified MToken and emissionToken. However, it has been identified that the wrong event is being emitted at the end of the function. The current implementation emits the NewSupplyRewardSpeed event instead of the expected NewBorrowRewardSpeed event.

This discrepancy can lead to confusion and incorrect data being captured by event listeners, potentially impacting the system's overall efficiency, accuracy, and traceability.

• Code Location

    /// @notice Update the borrow emissions for a given mtoken, emission token pair.
    function _updateBorrowSpeed(MToken _mToken, address _emissionToken, uint _newBorrowSpeed) public {
        MarketEmissionConfig storage emissionConfig = fetchConfigByEmissionToken(_mToken, _emissionToken);

        // Safety check this is the owner or the admin
        requireEmissionConfigOwnerOrAdmin(emissionConfig);

        uint currentBorrowSpeed = emissionConfig.config.borrowEmissionsPerSec;

        require(_newBorrowSpeed != currentBorrowSpeed, "Can't set new borrow emissions to be equal to current!");
        require(_newBorrowSpeed < emissionCap, "Cannot set a borrow reward speed higher than the emission cap!");

        // Make sure we update our indices before setting the new speed
        updateMarketBorrowIndexInternal(_mToken);

        // Update borrow speed
        emissionConfig.config.borrowEmissionsPerSec = _newBorrowSpeed;

        emit NewSupplyRewardSpeed(_mToken, _emissionToken, currentBorrowSpeed, _newBorrowSpeed);
    }

It is recommended to emit the NewBorrowRewardSpeed event instead of NewSupplyRewardSpeed.

Remediation Plan

SOLVED: The Moonwell Finance team solved the issue by changing the event.

The emissionCap variable in the given smart contract does not have an upper bound in the _setEmissionCap function. By default, the emission cap is set to 100 * 10^18 tokens per second to avoid unbounded computation/multiplication overflows. However, the function _setEmissionCap allows changing the emissionCap value without any restriction on the upper limit, which can potentially lead to overflows and other unexpected issues in the contract's execution.

• Code Location

    function _setEmissionCap(uint _newEmissionCap) external {
        requireComptrollersAdmin();

        uint oldEmissionCap = emissionCap;

        emissionCap = _newEmissionCap;

        emit NewEmissionCap(oldEmissionCap, _newEmissionCap);
    }

It is recommended to have an upper bound in the _setEmissionCap function.

Remediation Plan

RISK ACCEPTED: The Moonwell Finance team accepted the risk of this issue.

The redeem() function in the current design of the WETHRouter smart contract is designed to handle the redemption of mToken and subsequent withdrawal of WETH. However, this function does not restrict the receipt of tokens to only WETH/mToken. As a result, any native token sent directly to the WETHRouter contract will be sent to the first redeemer.

In this setup, an unintentional or malicious transfer of arbitrary tokens to the WETHRouter contract could lead to an unexpected balance increase. When the redeem() function is called, it attempts to withdraw all ETH equivalent in the contract and sends it to the recipient. If an arbitrary amount of tokens or native ETH is sent to the contract, it would inflate the balance available for withdrawal, making it retrievable by the first redeemer.

• Code Location

    function redeem(uint256 mTokenRedeemAmount, address recipient) external {
        IERC20(address(mToken)).safeTransferFrom(
            msg.sender,
            address(this),
            mTokenRedeemAmount
        );

        mToken.redeem(100 ether);

        weth.withdraw(weth.balanceOf(address(this)));

        (bool success, ) = payable(recipient).call{
            value: address(this).balance
        }("");
        require(success, "WETHRouter: ETH transfer failed");
    }

    receive() external payable {}

It is recommended to add an address validation.

Remediation Plan

SOLVED: The Moonwell Finance team solved the issue by adding an address validation.

The contracts are upgradable, inheriting from the Initializable contract. However, the current implementations are missing the _disableInitializers() function call in the constructors. Thus, an attacker can initialize the implementation. Usually, the initialized implementation has no direct impact on the proxy itself; however, it can be exploited in a phishing attack. In rare cases, the implementation might be mutable and may have an impact on the proxy.

It is recommended to implement the _disableInitializers() function call in the constructors.

Remediation Plan

SOLVED: The contracts now implement the _disableInitializers() function call in the constructors.

Commit ID: c39f98bdc9dd4e448ba585923034af1d47f74dfa

The WethUnwrapper contract contains a hard-coded mToken address (0x628ff693426583D9a7FB391E54366292F509D457). This design pattern can create limitations, particularly in scenarios where cross-chain functionality or upgradability is desired.

Hard-coding an address within a contract can lead to a lack of flexibility, making it challenging to adapt to changes or to interact with different instances of a token on various chains. In a cross-chain environment, where tokens might be represented on multiple blockchains, having a fixed address can hinder the ability to seamlessly interact across different networks.

• Code Location

/src/WethUnwrapper.sol#L8

contract WethUnwrapper {
    /// @notice the mToken address
    address public constant mToken = 0x628ff693426583D9a7FB391E54366292F509D457;

    /// @notice reference to the WETH contract
    address public immutable weth;

    /// @notice construct a new WethUnwrapper
    /// @param _weth the WETH contract address
    constructor(address _weth) {
        weth = _weth;
    }

It is recommended to define the variable as immutable.

Remediation Plan

SOLVED: The Moonwell Finance team solved the issue by defining the variable as immutable.

Commit ID: 8d1848c5344c12ffb0978721efcf7d44bf250867

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it’s not necessarily best to index the maximum allowed per event (three fields).

• Code Location

File: MultiRewardDistributorCommon.sol

61: event GlobalSupplyIndexUpdated(MToken mToken, address emissionToken, uint newSupplyIndex, uint32 newSupplyGlobalTimestamp);
62: event GlobalBorrowIndexUpdated(MToken mToken, address emissionToken, uint newIndex, uint32 newTimestamp);
65: event DisbursedSupplierRewards(MToken mToken, address supplier, address emissionToken, uint totalAccrued);
66: event DisbursedBorrowerRewards(MToken mToken, address borrower, address emissionToken, uint totalAccrued);
69: event NewConfigCreated(MToken mToken, address owner, address emissionToken, uint supplySpeed, uint borrowSpeed, uint endTime);
70: event NewPauseGuardian(address oldPauseGuardian, address newPauseGuardian);
71: event NewEmissionCap(uint oldEmissionCap, uint newEmissionCap);
72: event NewEmissionConfigOwner(MToken mToken, address emissionToken, address currentOwner, address newOwner);
73: event NewRewardEndTime(MToken mToken, address emissionToken, uint currentEndTime, uint newEndTime);
74: event NewSupplyRewardSpeed(MToken mToken, address emissionToken, uint oldRewardSpeed, uint newRewardSpeed);
75: event NewBorrowRewardSpeed(MToken mToken, address emissionToken, uint oldRewardSpeed, uint newRewardSpeed);
76: event FundsRescued(address token, uint amount);
83: event InsufficientTokensToEmit(address payable user, address rewardToken, uint amount);

It is recommended to add the indexed keyword to the events.

Remediation Plan

SOLVED: The Moonwell Finance team solved the issue by adding the indexed keyword on the events.

The project contains many instances of floating pragma. Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, either an outdated compiler version that might introduce bugs that affect the contract system negatively or a pragma version too recent which has not been extensively tested.

• Code Location

The ChainlinkCompositeOracle is affected. (^0.8.0)

It is recommended to lock the pragma.

Remediation Plan

SOLVED: The Moonwell Finance team solved the issue by locking the pragma.

Commit ID: 17fce574c46259cb22b8b6215b8b982169eb40e7

Custom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they’re hit by avoiding having to allocate and store the revert string. Not defining the strings also saves deployment gas.

It is recommended to use custom errors instead of revert strings to save gas.

Remediation Plan

ACKNOWLEDGED: The Moonwell Finance team acknowledged this finding.

i++ involves checked arithmetic, which is not required. This is because the value of i is always strictly less than length <= 2**256 - 1. Therefore, the theoretical maximum value of i to enter the for-loop body is 2**256 - 2. This means that the i++ in the for loop can never overflow. Regardless, the compiler performs the overflow checks.

• Code Location

File: core/Governance/TemporalGovernor.sol

60:         for (uint256 i = 0; i < _trustedSenders.length; i++) {

103:             for (uint256 i = 0; i < trustedSendersList.length; i++) {

132:             for (uint256 i = 0; i < _trustedSenders.length; i++) {

159:             for (uint256 i = 0; i < _trustedSenders.length; i++) {

380:         for (uint256 i = 0; i < targets.length; i++) {

File: core/MultiRewardDistributor/MultiRewardDistributor.sol

209:         for (uint256 index = 0; index < configs.length; index++) {

240:         for (uint256 index = 0; index < markets.length; index++) {

281:         for (uint256 index = 0; index < configs.length; index++) {

419:         for (uint256 index = 0; index < configs.length; index++) {

983:         for (uint256 index = 0; index < configs.length; index++) {

1009:         for (uint256 index = 0; index < configs.length; index++) {

1053:         for (uint256 index = 0; index < configs.length; index++) {

1112:         for (uint256 index = 0; index < configs.length; index++) {

1163:         for (uint256 index = 0; index < configs.length; index++) {

It is recommended to remove the unnecessary checks.

Remediation Plan

ACKNOWLEDGED: The Moonwell Finance team acknowledged this finding.

The current ownership transfer process for TemporalGovernor contract inheriting from Ownable or OwnableUpgradeable involves the current owner calling the transferOwnership() function:

• Ownable.sol

function transferOwnership(address newOwner) public virtual onlyOwner {
    require(newOwner != address(0), "Ownable: new owner is the zero address");
    _setOwner(newOwner);
}

If the nominated EOA account is not a valid account, it is entirely possible that the owner may accidentally transfer ownership to an uncontrolled account, losing the access to all functions with the onlyOwner modifier.

It is recommended to use the double-step transferOwnership pattern.

Remediation Plan

ACKNOWLEDGED: The Moonwell Finance team acknowledged this finding.

In a for loop, the length of an array can be put in a temporary variable to save some gas. This has been done already in several other locations in the code.

In the above case, the solidity compiler will always read the length of the array during each iteration. That is,

• if it is a storage array, this is an extra sload operation (100 additional extra gas (EIP-2929) for each iteration except for the first),

• if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first),

• if it is a calldata array, this is an extra calldataload operation (3 additional gas for each iteration except for the first)

• Code Location

File: core/Governance/TemporalGovernor.sol

60:         for (uint256 i = 0; i < _trustedSenders.length; i++) {

103:             for (uint256 i = 0; i < trustedSendersList.length; i++) {

132:             for (uint256 i = 0; i < _trustedSenders.length; i++) {

159:             for (uint256 i = 0; i < _trustedSenders.length; i++) {

380:         for (uint256 i = 0; i < targets.length; i++) {

File: core/MultiRewardDistributor/MultiRewardDistributor.sol

209:         for (uint256 index = 0; index < configs.length; index++) {

240:         for (uint256 index = 0; index < markets.length; index++) {

281:         for (uint256 index = 0; index < configs.length; index++) {

419:         for (uint256 index = 0; index < configs.length; index++) {

983:         for (uint256 index = 0; index < configs.length; index++) {

1009:         for (uint256 index = 0; index < configs.length; index++) {

1053:         for (uint256 index = 0; index < configs.length; index++) {

1112:         for (uint256 index = 0; index < configs.length; index++) {

1163:         for (uint256 index = 0; index < configs.length; index++) {

It is recommended to put the length of some arrays in a temporary variable to save some gas.

Remediation Plan

ACKNOWLEDGED: The Moonwell Finance team acknowledged this finding.

The TemporalGovernor contract uses the OpenZeppelin SafeCast library for type conversion operations. However, it is important to note that there is no possibility of an overflow occurring in the variable through the utilization of block.timestamp.

• Code Location

contract TemporalGovernor is ITemporalGovernor, Ownable, Pausable {
    using SafeCast for *;
}

It is recommended to remove the redundant safecast.

Remediation Plan

SOLVED: The Moonwell Finance team solved the issue by removing safecast.

Commit ID: 17fce574c46259cb22b8b6215b8b982169eb40e7

Shortening revert strings to fit in 32 bytes will decrease deploy time gas and will decrease runtime gas when the revert condition has been met.

• Code Location

    function setTrustedSenders(
        TrustedSender[] calldata _trustedSenders
    ) external {
        require(
            msg.sender == address(this),
            "TemporalGovernor: Only this contract can update trusted senders"
        );
}

It is recommended shortening revert strings to fit in 32 bytes to decrease deploy time gas and decrease runtime gas when the revert condition has been met.

Remediation Plan

ACKNOWLEDGED: The Moonwell Finance team acknowledged this finding.

Initialization to 0 or false is not necessary, as these are the default values in Solidity.

• Code Location

File: core/Governance/TemporalGovernor.sol

60:         for (uint256 i = 0; i < _trustedSenders.length; i++) {

103:             for (uint256 i = 0; i < trustedSendersList.length; i++) {

132:             for (uint256 i = 0; i < _trustedSenders.length; i++) {

159:             for (uint256 i = 0; i < _trustedSenders.length; i++) {

380:         for (uint256 i = 0; i < targets.length; i++) {

File: core/MultiRewardDistributor/MultiRewardDistributor.sol

209:         for (uint256 index = 0; index < configs.length; index++) {

240:         for (uint256 index = 0; index < markets.length; index++) {

281:         for (uint256 index = 0; index < configs.length; index++) {

419:         for (uint256 index = 0; index < configs.length; index++) {

983:         for (uint256 index = 0; index < configs.length; index++) {

1009:         for (uint256 index = 0; index < configs.length; index++) {

1053:         for (uint256 index = 0; index < configs.length; index++) {

1112:         for (uint256 index = 0; index < configs.length; index++) {

1163:         for (uint256 index = 0; index < configs.length; index++) {

It is recommended to not initialize to 0 or false because they are default values in Solidity.

Remediation Plan

ACKNOWLEDGED: The Moonwell Finance team acknowledged this finding.

The return value of an external call is not stored in a local or state variable.

• Code Location

    function _supportMarket(MToken mToken) external returns (uint) {
        if (msg.sender != admin) {
            return fail(Error.UNAUTHORIZED, FailureInfo.SUPPORT_MARKET_OWNER_CHECK);
        }

        if (markets[address(mToken)].isListed) {
            return fail(Error.MARKET_ALREADY_LISTED, FailureInfo.SUPPORT_MARKET_EXISTS);
        }

        mToken.isMToken(); // Sanity check to make sure its really a MToken

        Market storage newMarket = markets[address(mToken)];
        newMarket.isListed = true;
        newMarket.collateralFactorMantissa = 0;

        _addMarketInternal(address(mToken));

        emit MarketListed(mToken);

        return uint(Error.NO_ERROR);
    }

It is recommended to use a require statement due to the return value of an external call is not stored in a local or state variable.

Remediation Plan

SOLVED: The Moonwell Finance team solved the issue by using require statement.

In the current smart contract implementation, several require() and revert() statements lack descriptive reason strings. These reason strings serve as informative error messages that help developers and users understand the cause of a failed transaction or function call. Omitting these strings can result confused when diagnosing issues or debugging the smart contract, as the cause of the failure may not be immediately apparent.

• Code Location

    function _updateEndTime(MToken _mToken, address _emissionToken, uint _newEndTime) public {
        MarketEmissionConfig storage emissionConfig = fetchConfigByEmissionToken(_mToken, _emissionToken);

        // Safety check this is the owner or the admin
        requireEmissionConfigOwnerOrAdmin(emissionConfig);

        uint currentEndTime = emissionConfig.config.endTime;

        // Must be older than our existing end time AND the current block
        require(_newEndTime > currentEndTime);
        require(_newEndTime > block.timestamp);

        // Update both global indices before setting the new end time. If rewards are off this just updates the
        // global block timestamp to the current second
        updateMarketBorrowIndexInternal(_mToken);
        updateMarketSupplyIndexInternal(_mToken);

        emissionConfig.config.endTime = _newEndTime;
        emit NewRewardEndTime(_mToken, _emissionToken, currentEndTime, _newEndTime);
    }

It is recommended to add descriptive strings in require()/revert() stataments.

Remediation Plan

SOLVED: The Moonwell Finance team solved the issue by adding descriptive strings.