Moonwell - ast3ros's results

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/Governance/TemporalGovernor.sol#L322 https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/Governance/TemporalGovernor.sol#L344-L409

Vulnerability details

Impact

A proposal for another contract can be executed in the TemporalGovernor contract if the guardian turns malicious. It can happen because if the guardian uses fastTrackProposalExecution, the _executeProposal function does not check the intendedRecipient of the proposal. The intendedRecipient is the address of the contract that the proposal is for.

Proof of Concept

The guardian account can call fastTrackProposalExecution to execute a proposal without queuing. The intended recipient is checked in queueProposal but not in executeProposal.

https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/Governance/TemporalGovernor.sol#L322 https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/Governance/TemporalGovernor.sol#L344-L409

Therefore, the guardian has the ability to execute a proposal not intended for the Temporal Governor. If the guardian is malicious, it can use this loophole to execute a proposal and set the protocol to a wrong state.

Tools Used

Manual Review

Recommended Mitigation Steps

Add the intended recipient check in _executeProposal function.

        function _executeProposal(bytes memory VAA, bool overrideDelay) private {
            ...
            require(
                trustedSenders[vm.emitterChainId].contains(vm.emitterAddress), /// allow multiple per chainid
                "TemporalGovernor: Invalid Emitter Address"
            );
+           require(intendedRecipient == address(this), "TemporalGovernor: Incorrect destination");
            ...
        }

Assessed type

Context

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/IRModels/JumpRateModel.sol#L74 https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/IRModels/WhitePaperInterestRateModel.sol#L57

Vulnerability details

Impact

The expected utilization rate is between 0 and 1e18. However, the utilization rate can be more than 1e18 when the reserve is more than cash.

Proof of Concept

The utilization rate is calculated by the following formula:

    borrows / (cash + borrows - reserves)

https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/IRModels/JumpRateModel.sol#L74 https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/IRModels/WhitePaperInterestRateModel.sol#L57

However, when the protocol runs for a very long time, the reserves can accumulate and can be more than cash. Therefore, the utilization rate can be more than 1e18.

For reference, please check this issue: https://github.com/code-423n4/2023-05-venus-findings/issues/122

Tools Used

Manual Review

Recommended Mitigation Steps

Add a case where the utilization rate returns 1 if reserves > cash.

Assessed type

Other

[L-1] Missing min and max checking when setting closeFactorMantissa

There is no min and max checking when setting closeFactorMantissa. The admin can mistakenly set it to wrong number that break the operation of the protocol. For example if it is set to 0, no one can liquidate. If it set to more than 1, the liquidation and accounting process will be wrong.

https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/Comptroller.sol#L689-L698

[L-2] Key parameters are not set in constructor in Comptroller contract

Some key parameters, such as closeFactorMantissa and liquidationIncentiveMantissa, are not set in the constructor. This can cause the protocol to operate incorrectly if the admin forgets to call the function to set them later.

It is advisable to set default values for these parameters in the constructor, so that they are initialized when the contract is deployed. This can prevent potential errors and ensure the proper functioning of the protocol.

https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/Comptroller.sol#L70-L72 https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/ComptrollerStorage.sol#L40 https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/ComptrollerStorage.sol#L45

[L-3] protocolSeizeShareMantissa are not set in initialize in MToken contract

protocolSeizeShareMantissa is not set in the initialize function. This can cause the protocol to operate incorrectly if the admin forgets to call the function to set it later. It is advisable to set default values for these parameters in the initialize function, so that they are initialized when the contract is deployed. This can prevent potential errors and ensure the proper functioning of the protocol.

https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/MErc20.sol#L23-L36 https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/MTokenInterfaces.sol#L81

[N-1] Unupdated naming

The current code uses the variable name blockDelta to store the difference between the current and the previous block timestamps. This can be misleading, as it implies that the variable represents the number of blocks, not the time elapsed.

A better name for this variable would be blockTimeStampDelta, as it clearly indicates that it is the delta of the block timestamps. This can avoid confusion and make the code more readable and understandable.

https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/MToken.sol#L405-L407

[N-2] Consider using descriptive constants instead of value

1e18 value should be replaced with constant to improve readability.

https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/IRModels/WhitePaperInterestRateModel.sol#L38-L39 https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/IRModels/WhitePaperInterestRateModel.sol#L57 https://github.com/code-423n4/2023-07-moonwell/blob/657fb2bcdb25ef44a4826b08390677c3fd41b92d/src/core/IRModels/WhitePaperInterestRateModel.sol#L83-L84