Report generated by auditbase

Summary

Medium Risk Issues

	Issue	Instances
[M001]	The owner is a single point of failure and a centralization risk	2
[M002]	Unsafe use of transfer()/transferFrom() with IERC20	2
[M003]	Return values of transfer()/transferFrom() not checked	2
[M004]	Missing checks for the L2 Sequencer grace period	2
[M005]	Insufficient oracle validation	1

Total: 9 instances over 5 issues

Low Risk Issues

	Issue	Instances
[L001]	Unsafe ERC20 Operation(s)	6
[L002]	abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()	4
[L003]	Initializers could be front-run	1
[L004]	Loss of precision due to division by large numbers	2
[L005]	Array lengths not checked	1
[L006]	Division by zero not prevented	2
[L007]	require() should be used instead of assert()	4
[L008]	Missing checks for address(0x0) when assigning values to address state variables	5
[L009]	Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions	1
[L010]	Events are missing sender information	29
[L011]	Use Ownable2Step rather than Ownable	1
[L012]	Upgradeable contract not initialized	1
[L013]	Array does not have a pop function	3
[L014]	Approve type(uint256).max not work with some tokens	1
[L015]	Solidity version 0.8.20 may not work on other chains due to PUSH0	3
[L016]	External call recipient may consume all transaction gas	2
[L017]	Setters should have initial value check	36
[L018]	Empty receive()/payable fallback() function does not authorize requests	1
[L019]	Contracts are designed to receive ETH but do not implement function for withdrawal	1
[L020]	Functions calling contracts/addresses with transfer hooks are missing reentrancy guards	2
[L021]	Some ERC20 revert on zero value transfer	10

Total: 119 instances over 21 issues

Non-critical Issues

	Issue	Instances
[NC001]	TODO Left in the code	2
[NC002]	External calls in an unbounded for-loop may result in a DoS	18
[NC003]	Constants should be defined rather than using magic numbers	38
[NC004]	Numeric values having to do with time should use time units for readability	1
[NC005]	Events that mark critical parameter changes should contain both the old and the new value	6
[NC006]	Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)	9
[NC007]	Event is not properly indexed	12
[NC008]	Function ordering does not follow the Solidity style guide	34
[NC009]	Imports could be organized more systematically	1
[NC010]	Constants in comparisons should appear on the left side	50
[NC011]	else-block not required	10
[NC012]	Events may be emitted out of order due to reentrancy	1
[NC013]	If-statement can be converted to a ternary	4
[NC014]	Import declarations should import specific identifiers, rather than the whole file	1
[NC015]	Public functions not called by the contract should be declared external instead	27
[NC016]	Constant redefined elsewhere	1
[NC017]	Duplicated require()/revert() checks should be refactored to a modifier or function	7
[NC018]	Use a more recent version of solidity	2
[NC019]	Variable names that consist of all capital letters should be reserved for constant/immutable variables	2
[NC020]	Consider using delete rather than assigning zero to clear values	1
[NC021]	Contract does not follow the Solidity style guide's suggested layout ordering	11
[NC022]	Lines are too long	108
[NC023]	All @param values in a function's NatSpec comment should match the function's parameters	155
[NC024]	NatSpec @return argument is missing	15
[NC025]	Inconsistent spacing in comments	1
[NC026]	Contract does not follow the Solidity style guide's suggested layout ordering	11
[NC027]	Non-external variable and function names should begin with an underscore	3
[NC028]	Consider disabling renounceOwnership()	1
[NC029]	Non-library/interface files should use fixed compiler versions, not floating ones	3
[NC030]	Empty Function Body - Consider commenting why	2
[NC031]	Remove forge-std import	1
[NC032]	Contracts should have full test coverage	1
[NC033]	Large or complicated code bases should implement invariant tests	1
[NC034]	Long functions should be refactored into multiple, smaller, functions	6
[NC036]	Consider using block.number instead of block.timestamp.	16
[NC037]	Large numeric literals should use underscores for readability	1
[NC038]	Cast to bytes or bytes32 for clearer semantic meaning	4
[NC039]	Critical Changes Should Use Two-step Procedure	3
[NC040]	Variable names don't follow the Solidity style guide	14
[NC041]	Consider using named mappings	1
[NC042]	Consider adding a deny-list	13
[NC043]	Use a more recent version of solidity	1
[NC044]	Mixed usage of int/uint with int256/uint256	88
[NC045]	Consider moving msg.sender checks to a common authorization modifier	26

Total: 717 instances over 45 issues

M001 - The owner is a single point of failure and a centralization risk:

Having a single EOA as the only owner of contracts is a large centralization risk and a single point of failure. A single private key may be taken in a hack, or the sole holder of the key may become unable to retrieve the key when necessary. Consider changing to a multi-signature setup, or having a role-based authorization model.

File: 2023-07-moonwell/src/core/Governance/TemporalGovernor.sol


266         function fastTrackProposalExecution(bytes memory VAA) external onlyOwner {


274         function togglePause() external onlyOwner {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Governance/TemporalGovernor.sol#L274:274

M002 - Unsafe use of transfer()/transferFrom() with IERC20:

Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether (USDT)'s transfer() and transferFrom() functions on L1 do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast to IERC20, their function signatures do not match and therefore the calls made, revert (see this link for a test case). Use OpenZeppelin’s SafeERC20's safeTransfer()/safeTransferFrom() instead

File: 2023-07-moonwell/src/core/Comptroller.sol


965                 token.transfer(admin, token.balanceOf(address(this)));


967                 token.transfer(admin, _amount);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L967:967

M003 - Return values of transfer()/transferFrom() not checked:

Not all IERC20 implementations revert() when there's a failure in transfer()/transferFrom(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually making a payment

File: 2023-07-moonwell/src/core/Comptroller.sol


965                 token.transfer(admin, token.balanceOf(address(this)));


967                 token.transfer(admin, _amount);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L967:967

M004 - Missing checks for the L2 Sequencer grace period:

Chainlink recommends that users using price oracles, check whether the grace period has passed in the event the Arbitrum Sequencer goes down. To help your applications identify when the sequencer is unavailable, you can use a data feed that tracks the last known status of the sequencer at a given point in time. This helps you prevent mass liquidations by providing a grace period to allow customers to react to such an event.

File: 2023-07-moonwell/src/core/Oracles/ChainlinkCompositeOracle.sol


180         function getPriceAndDecimals(
181             address oracleAddress
182         ) public view returns (int256, uint8) {
183             (
184                 uint80 roundId,
185                 int256 price,
186                 ,
187                 ,
188                 uint80 answeredInRound
189             ) = AggregatorV3Interface(oracleAddress).latestRoundData();
190             bool valid = price > 0 && answeredInRound == roundId;
191             require(valid, "CLCOracle: Oracle data is invalid");
192             uint8 oracleDecimals = AggregatorV3Interface(oracleAddress).decimals();
193     
194             return (price, oracleDecimals); /// price always gt 0 at this point
195         }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkCompositeOracle.sol#L180:195

File: 2023-07-moonwell/src/core/Oracles/ChainlinkOracle.sol


97          function getChainlinkPrice(
98              AggregatorV3Interface feed
99          ) internal view returns (uint256) {
100             (, int256 answer, , uint256 updatedAt, ) = AggregatorV3Interface(feed)
101                 .latestRoundData();
102             require(answer > 0, "Chainlink price cannot be lower than 0");
103             require(updatedAt != 0, "Round is in incompleted state");
104     
105             // Chainlink USD-denominated feeds store answers at 8 decimals
106             uint256 decimalDelta = uint256(18).sub(feed.decimals());
107             // Ensure that we don't multiply the result by 0
108             if (decimalDelta > 0) {
109                 return uint256(answer).mul(10 ** decimalDelta);
110             } else {
111                 return uint256(answer);
112             }
113         }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkOracle.sol#L97:113

M005 - Insufficient oracle validation:

There is no freshness check on the timestamp of the prices fetched from the Chainlink oracle, so old prices may be used if OCR was unable to push an update in time. Add a staleness threshold number of seconds configuration parameter, and ensure that the price fetched is from within that time range.

File: 2023-07-moonwell/src/core/Oracles/ChainlinkCompositeOracle.sol


189             ) = AggregatorV3Interface(oracleAddress).latestRoundData();

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkCompositeOracle.sol#L189:189

L001 - Unsafe ERC20 Operation(s):

ERC20 operations can be unsafe due to different implementations and vulnerabilities in the standard. It is therefore recommended to always either use OpenZeppelin's SafeERC20 library or at least to wrap each operation in a require statement. To circumvent ERC20's approve functions race-condition vulnerability use OpenZeppelin's SafeERC20 library's safe{Increase|Decrease}Allowance functions.

Click to show 6 findings

File: 2023-07-moonwell/src/core/Comptroller.sol


token.transfer(admin, token.balanceOf(address(this)));

token.transfer(admin, _amount);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L967:967

File: 2023-07-moonwell/src/core/MErc20.sol


token.transfer(admin, balance);

token.transferFrom(from, address(this), amount);

token.transfer(to, amount);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MErc20.sol#L225:225

File: 2023-07-moonwell/src/core/router/WETHRouter.sol


_weth.approve(address(_mToken), type(uint256).max);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/router/WETHRouter.sol#L26:26

L002 - abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256():

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead. If all arguments are strings and or bytes, bytes.concat() should be used instead.

Click to show 4 findings

File: 2023-07-moonwell/src/core/Oracles/ChainlinkOracle.sol


nativeToken = keccak256(abi.encodePacked(_nativeToken));

if (keccak256(abi.encodePacked(symbol)) == nativeToken) { /// @dev this branch should never get called as native tokens are not supported on this deployment

feeds[keccak256(abi.encodePacked(symbol))] = AggregatorV3Interface(

return feeds[keccak256(abi.encodePacked(symbol))];

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkOracle.sol#L161:161

L003 - Initializers could be front-run:

Initializers could be front-run, allowing an attacker to either set their own values, take ownership of the contract, and in the best case forcing a re-deployment

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


88          function initialize(
89              address _comptroller,
90              address _pauseGuardian
91          ) external initializer {
92              // Sanity check the params
93              require(
94                  _comptroller != address(0),
95                  "Comptroller can't be the 0 address!"
96              );
97              require(
98                  _pauseGuardian != address(0),
99                  "Pause Guardian can't be the 0 address!"
100             );
101     
102             comptroller = Comptroller(payable(_comptroller));
103     
104             require(
105                 comptroller.isComptroller(),
106                 "Can't bind to something that's not a comptroller!"
107             );
108     
109             pauseGuardian = _pauseGuardian;
110             emissionCap = 100e18;
111         }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L88:111

L004 - Loss of precision due to division by large numbers:

Division by large numbers may result in the result being zero, due to solidity not supporting fractions. Consider requiring a minimum amount for the numerator to ensure that it is always larger than the denominator.

File: 2023-07-moonwell/src/core/Oracles/ChainlinkCompositeOracle.sol


95              return ((basePrice * priceMultiplier) / scalingFactor).toUint256();


158                 ((firstPrice * secondPrice * thirdPrice) / scalingFactor)

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkCompositeOracle.sol#L158:158

L005 - Array lengths not checked:

If the length of the arrays are not required to be of the same length, user operations may not be fully executed due to a mismatch in the number of items iterated over, versus the number of items provided in the second array.

File: 2023-07-moonwell/src/core/Comptroller.sol

1028        function claimRewsard(address[] memory holders, MToken[] memory mTokens, bool borrowers, bool suppliers) public {
1029            require(address(rewardDistributor) != address(0), "No reward distributor configured!");
1030    
1031            for (uint i = 0; i < mTokens.length; i++) {
1032    
1033                // Safety check that the supplied mTokens are active/listed
1034                MToken mToken = mTokens[i];
1035                require(markets[address(mToken)].isListed, "market must be listed");
1036    
1037                // Disburse supply side
1038                if (suppliers == true) {
1039                    rewardDistributor.updateMarketSupplyIndex(mToken);
1040                    for (uint holderIndex = 0; holderIndex < holders.length; holderIndex++) {
1041                        rewardDistributor.disburseSupplierRewards(mToken, holders[holderIndex], true);
1042                    }
1043                }
1044    
1045                // Disburse borrow side
1046                if (borrowers == true) {
1047                    rewardDistributor.updateMarketBorrowIndex(mToken);
1048                    for (uint holderIndex = 0; holderIndex < holders.length; holderIndex++) {
1049                        rewardDistributor.disburseBorrowerRewards(mToken, holders[holderIndex], true);
1050                    }
1051                }
1052            }
1053        }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L1028:1053

L006 - Division by zero not prevented:

The divisions below take an input parameter which does not have any zero-value checks, which may lead to the functions reverting when zero is passed.

File: 2023-07-moonwell/src/core/Oracles/ChainlinkCompositeOracle.sol


158                 ((firstPrice * secondPrice * thirdPrice) / scalingFactor)


95              return ((basePrice * priceMultiplier) / scalingFactor).toUint256();

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkCompositeOracle.sol#L95:95

L007 - require() should be used instead of assert():

Prior to solidity version 0.8.0, hitting an assert consumes the remainder of the transaction's available gas rather than returning it, as require()/revert() do. assert() should be avoided even past solidity version 0.8.0 as its documentation states that 'The assert function creates an error of type Panic(uint256). … Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix'.

Click to show 4 findings

File: 2023-07-moonwell/src/core/Comptroller.sol


194             assert(assetIndex < len);


329                 assert(markets[mToken].accountMembership[borrower]);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L329:329

File: 2023-07-moonwell/src/core/Governance/TemporalGovernor.sol


256             assert(!guardianPauseAllowed); /// this should never revert, statement for SMT solving


289             assert(!guardianPauseAllowed); /// this should be an unreachable state

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Governance/TemporalGovernor.sol#L289:289

L008 - Missing checks for address(0x0) when assigning values to address state variables:

This issue arises when an address state variable is assigned a value without a preceding check to ensure it isn't address(0x0). This can lead to unexpected behavior as address(0x0) often represents an uninitialized address.

Click to show 5 findings

File: 2023-07-moonwell/src/core/Oracles/ChainlinkCompositeOracle.sol


40              base = baseAddress;


41              multiplier = multiplierAddress;


42              secondMultiplier = secondMultiplierAddress;

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkCompositeOracle.sol#L42:42

File: 2023-07-moonwell/src/core/Oracles/ChainlinkOracle.sol


175             admin = newAdmin;

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkOracle.sol#L175:175

File: 2023-07-moonwell/src/core/Governance/TemporalGovernor.sol


67              wormholeBridge = IWormhole(wormholeCore);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Governance/TemporalGovernor.sol#L67:67

L009 - Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions:

This issue arises when an upgradeable contract doesn't include a __gap[50] storage variable. This variable is crucial for facilitating future upgrades and preserving storage layout.

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


43      contract MultiRewardDistributor is

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L43:43

L010 - Events are missing sender information:

When an action is triggered based on a user's action, not being able to filter based on who triggered the action makes event processing a lot more cumbersome. Including the msg.sender the events of these types of action will make events much more useful to end users. Include msg.sender in the event output.

Click to show 29 findings

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


452             emit NewConfigCreated(
453                 _mToken,
454                 _owner,
455                 _emissionToken,
456                 _supplyEmissionPerSec,
457                 _borrowEmissionsPerSec,
458                 _endTime
459             );


486             emit FundsRescued(_tokenAddress, _amount);


505             emit NewPauseGuardian(currentPauseGuardian, _newPauseGuardian);


519             emit NewEmissionCap(oldEmissionCap, _newEmissionCap);


689             emit NewSupplyRewardSpeed(
690                 _mToken,
691                 _emissionToken,
692                 currentSupplySpeed,
693                 _newSupplySpeed
694             );


733             emit NewBorrowRewardSpeed(
734                 _mToken,
735                 _emissionToken,
736                 currentBorrowSpeed,
737                 _newBorrowSpeed
738             );


761             emit NewEmissionConfigOwner(
762                 _mToken,
763                 _emissionToken,
764                 currentOwner,
765                 _newOwner
766             );


804             emit NewRewardEndTime(
805                 _mToken,
806                 _emissionToken,
807                 currentEndTime,
808                 _newEndTime
809             );

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L804:809

File: 2023-07-moonwell/src/core/Comptroller.sol


695             emit NewCloseFactor(oldCloseFactorMantissa, closeFactorMantissa);


737             emit NewCollateralFactor(mToken, oldCollateralFactorMantissa, newCollateralFactorMantissa);


761             emit NewLiquidationIncentive(oldLiquidationIncentiveMantissa, newLiquidationIncentiveMantissa);


789             emit MarketListed(mToken);


817                 emit NewBorrowCap(mTokens[i], newBorrowCaps[i]);


835             emit NewBorrowCapGuardian(oldBorrowCapGuardian, newBorrowCapGuardian);


854                 emit NewSupplyCap(mTokens[i], newSupplyCaps[i]);


872             emit NewSupplyCapGuardian(oldSupplyCapGuardian, newSupplyCapGuardian);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L872:872

File: 2023-07-moonwell/src/core/Oracles/ChainlinkOracle.sol


123             emit PricePosted(
124                 asset,
125                 prices[asset],
126                 underlyingPriceMantissa,
127                 underlyingPriceMantissa
128             );


136             emit PricePosted(asset, prices[asset], price, price);


149             emit FeedSet(feed, symbol);


177             emit NewAdmin(oldAdmin, newAdmin);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkOracle.sol#L177:177

File: 2023-07-moonwell/src/core/Governance/TemporalGovernor.sol


151                     emit TrustedSenderUpdated(
152                         _trustedSenders[i].chainId,
153                         _trustedSenders[i].addr,
154                         true /// added to list
155                     );


178                     emit TrustedSenderUpdated(
179                         _trustedSenders[i].chainId,
180                         _trustedSenders[i].addr,
181                         false /// removed from list
182                     );


197             emit GuardianPauseGranted(block.timestamp);


220             emit GuardianRevoked(oldGuardian);


258             emit PermissionlessUnpaused(block.timestamp);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Governance/TemporalGovernor.sol#L258:258

File: 2023-07-moonwell/src/core/MToken.sol


162             emit Approval(src, spender, amount);


1158            emit NewPendingAdmin(oldPendingAdmin, newPendingAdmin);


1184            emit NewAdmin(oldAdmin, admin);


1185            emit NewPendingAdmin(oldPendingAdmin, pendingAdmin);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MToken.sol#L1185:1185

L011 - Use Ownable2Step rather than Ownable:

Ownable2Step and Ownable2StepUpgradeable prevent the contract ownership from mistakenly being transferred to an address that cannot handle it (e.g. due to a typo in the address), by requiring that the recipient of the owner permissions actively accept via a contract call of its own.

File: 2023-07-moonwell/src/core/Governance/TemporalGovernor.sol


27      contract TemporalGovernor is ITemporalGovernor, Ownable, Pausable {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Governance/TemporalGovernor.sol#L27:27

L012 - Upgradeable contract not initialized:

Upgradeable contracts are initialized via an initializer function rather than by a constructor. Leaving such a contract uninitialized may lead to it being taken over by a malicious user.

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


43      contract MultiRewardDistributor is

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L43:43

L013 - Array does not have a pop function:

Arrays without the pop operation in Solidity can lead to inefficient memory management and increase the likelihood of out-of-gas errors.

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


462             MarketEmissionConfig storage newConfig = configs.push();

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L462:462

File: 2023-07-moonwell/src/core/Comptroller.sol


140             accountAssets[borrower].push(mToken);


798             allMarkets.push(MToken(mToken));

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L798:798

L014 - Approve type(uint256).max not work with some tokens:

Source: https://github.com/d-xo/weird-erc20#revert-on-large-approvals–transfers

File: 2023-07-moonwell/src/core/router/WETHRouter.sol


26              _weth.approve(address(_mToken), type(uint256).max);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/router/WETHRouter.sol#L26:26

L015 - Solidity version 0.8.20 may not work on other chains due to PUSH0:

The compiler for Solidity 0.8.20 switches the default target EVM version to Shanghai, which includes the new PUSH0 op code. This op code may not yet be implemented on all L2s, so deployment on these chains will fail. To work around this issue, use an earlier EVM version. While the project itself may or may not compile with 0.8.20, other projects with which it integrates, or which extend this project may, and those projects will have problems deploying these contracts/libraries.

File: 2023-07-moonwell/test/proposals/mips/mip00.sol


2       pragma solidity ^0.8.0;

https://github.com/code-423n4/2023-07-moonwell/tree/main/test/proposals/mips/mip00.sol#L2:2

File: 2023-07-moonwell/src/core/Oracles/ChainlinkCompositeOracle.sol


2       pragma solidity ^0.8.0;

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkCompositeOracle.sol#L2:2

File: 2023-07-moonwell/src/core/Governance/TemporalGovernor.sol


2       pragma solidity ^0.8.0;

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Governance/TemporalGovernor.sol#L2:2

L016 - External call recipient may consume all transaction gas:

There is no limit specified on the amount of gas used, so the recipient can use up all of the transaction's gas, causing it to revert. Use addr.call{gas: <amount>}() or this library instead.

File: 2023-07-moonwell/src/core/Governance/TemporalGovernor.sol


400                 (bool success, bytes memory returnData) = target.call{value: value}(

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Governance/TemporalGovernor.sol#L400:400

File: 2023-07-moonwell/src/core/router/WETHRouter.sol


59              (bool success, ) = payable(recipient).call{
60                  value: address(this).balance
61              }("");

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/router/WETHRouter.sol#L59:61

L017 - Setters should have initial value check:

Setters should have initial value check to prevent assigning wrong value to the variable. Assginment of wrong value can lead to unexpected behavior of the contract.

Click to show 37 findings

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


512         function _setEmissionCap(
513             uint256 _newEmissionCap
514         ) external onlyComptrollersAdmin {
515             uint256 oldEmissionCap = emissionCap;
516     
517             emissionCap = _newEmissionCap;
518     
519             emit NewEmissionCap(oldEmissionCap, _newEmissionCap);
520         }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L512:520

File: 2023-07-moonwell/src/core/Comptroller.sol


665         function _setPriceOracle(PriceOracle newOracle) public returns (uint) {
666             // Check caller is admin
667             if (msg.sender != admin) {
668                 return fail(Error.UNAUTHORIZED, FailureInfo.SET_PRICE_ORACLE_OWNER_CHECK);
669             }
670     
671             // Track the old oracle for the comptroller
672             PriceOracle oldOracle = oracle;
673     
674             // Set comptroller's oracle to newOracle
675             oracle = newOracle;
676     
677             // Emit NewPriceOracle(oldOracle, newOracle)
678             emit NewPriceOracle(oldOracle, newOracle);
679     
680             return uint(Error.NO_ERROR);
681         }


689         function _setCloseFactor(uint newCloseFactorMantissa) external returns (uint) {
690             // Check caller is admin
691         	require(msg.sender == admin, "only admin can set close factor");
692     
693             uint oldCloseFactorMantissa = closeFactorMantissa;
694             closeFactorMantissa = newCloseFactorMantissa;
695             emit NewCloseFactor(oldCloseFactorMantissa, closeFactorMantissa);
696     
697             return uint(Error.NO_ERROR);
698         }


707         function _setCollateralFactor(MToken mToken, uint newCollateralFactorMantissa) external returns (uint) {
708             // Check caller is admin
709             if (msg.sender != admin) {
710                 return fail(Error.UNAUTHORIZED, FailureInfo.SET_COLLATERAL_FACTOR_OWNER_CHECK);
711             }
712     
713             // Verify market is listed
714             Market storage market = markets[address(mToken)];
715             if (!market.isListed) {
716                 return fail(Error.MARKET_NOT_LISTED, FailureInfo.SET_COLLATERAL_FACTOR_NO_EXISTS);
717             }
718     
719             Exp memory newCollateralFactorExp = Exp({mantissa: newCollateralFactorMantissa});
720     
721             // Check collateral factor <= 0.9
722             Exp memory highLimit = Exp({mantissa: collateralFactorMaxMantissa});
723             if (lessThanExp(highLimit, newCollateralFactorExp)) {
724                 return fail(Error.INVALID_COLLATERAL_FACTOR, FailureInfo.SET_COLLATERAL_FACTOR_VALIDATION);
725             }
726     
727             // If collateral factor != 0, fail if price == 0
728             if (newCollateralFactorMantissa != 0 && oracle.getUnderlyingPrice(mToken) == 0) {
729                 return fail(Error.PRICE_ERROR, FailureInfo.SET_COLLATERAL_FACTOR_WITHOUT_PRICE);
730             }
731     
732             // Set market's collateral factor to new collateral factor, remember old value
733             uint oldCollateralFactorMantissa = market.collateralFactorMantissa;
734             market.collateralFactorMantissa = newCollateralFactorMantissa;
735     
736             // Emit event with asset, old collateral factor, and new collateral factor
737             emit NewCollateralFactor(mToken, oldCollateralFactorMantissa, newCollateralFactorMantissa);
738     
739             return uint(Error.NO_ERROR);
740         }


748         function _setLiquidationIncentive(uint newLiquidationIncentiveMantissa) external returns (uint) {
749             // Check caller is admin
750             if (msg.sender != admin) {
751                 return fail(Error.UNAUTHORIZED, FailureInfo.SET_LIQUIDATION_INCENTIVE_OWNER_CHECK);
752             }
753     
754             // Save current value for use in log
755             uint oldLiquidationIncentiveMantissa = liquidationIncentiveMantissa;
756     
757             // Set liquidation incentive to new incentive
758             liquidationIncentiveMantissa = newLiquidationIncentiveMantissa;
759     
760             // Emit event with old incentive, new incentive
761             emit NewLiquidationIncentive(oldLiquidationIncentiveMantissa, newLiquidationIncentiveMantissa);
762     
763             return uint(Error.NO_ERROR);
764         }


807         function _setMarketBorrowCaps(MToken[] calldata mTokens, uint[] calldata newBorrowCaps) external {
808         	require(msg.sender == admin || msg.sender == borrowCapGuardian, "only admin or borrow cap guardian can set borrow caps");
809     
810             uint numMarkets = mTokens.length;
811             uint numBorrowCaps = newBorrowCaps.length;
812     
813             require(numMarkets != 0 && numMarkets == numBorrowCaps, "invalid input");
814     
815             for(uint i = 0; i < numMarkets; i++) {
816                 borrowCaps[address(mTokens[i])] = newBorrowCaps[i];
817                 emit NewBorrowCap(mTokens[i], newBorrowCaps[i]);
818             }
819         }


825         function _setBorrowCapGuardian(address newBorrowCapGuardian) external {
826             require(msg.sender == admin, "only admin can set borrow cap guardian");
827     
828             // Save current value for inclusion in log
829             address oldBorrowCapGuardian = borrowCapGuardian;
830     
831             // Store borrowCapGuardian with value newBorrowCapGuardian
832             borrowCapGuardian = newBorrowCapGuardian;
833     
834             // Emit NewBorrowCapGuardian(OldBorrowCapGuardian, NewBorrowCapGuardian)
835             emit NewBorrowCapGuardian(oldBorrowCapGuardian, newBorrowCapGuardian);
836         }


844         function _setMarketSupplyCaps(MToken[] calldata mTokens, uint[] calldata newSupplyCaps) external {
845             require(msg.sender == admin || msg.sender == supplyCapGuardian, "only admin or supply cap guardian can set supply caps");
846     
847             uint numMarkets = mTokens.length;
848             uint numSupplyCaps = newSupplyCaps.length;
849     
850             require(numMarkets != 0 && numMarkets == numSupplyCaps, "invalid input");
851     
852             for(uint i = 0; i < numMarkets; i++) {
853                 supplyCaps[address(mTokens[i])] = newSupplyCaps[i];
854                 emit NewSupplyCap(mTokens[i], newSupplyCaps[i]);
855             }
856         }


862         function _setSupplyCapGuardian(address newSupplyCapGuardian) external {
863             require(msg.sender == admin, "only admin can set supply cap guardian");
864     
865             // Save current value for inclusion in log
866             address oldSupplyCapGuardian = supplyCapGuardian;
867     
868             // Store supplyCapGuardian with value newSupplyCapGuardian
869             supplyCapGuardian = newSupplyCapGuardian;
870     
871             // Emit NewSupplyCapGuardian(OldSupplyCapGuardian, NewSupplyCapGuardian)
872             emit NewSupplyCapGuardian(oldSupplyCapGuardian, newSupplyCapGuardian);
873         }


880         function _setPauseGuardian(address newPauseGuardian) public returns (uint) {
881             if (msg.sender != admin) {
882                 return fail(Error.UNAUTHORIZED, FailureInfo.SET_PAUSE_GUARDIAN_OWNER_CHECK);
883             }
884     
885             // Save current value for inclusion in log
886             address oldPauseGuardian = pauseGuardian;
887     
888             // Store pauseGuardian with value newPauseGuardian
889             pauseGuardian = newPauseGuardian;
890     
891             // Emit NewPauseGuardian(OldPauseGuardian, NewPauseGuardian)
892             emit NewPauseGuardian(oldPauseGuardian, pauseGuardian);
893     
894             return uint(Error.NO_ERROR);
895         }


901         function _setRewardDistributor(MultiRewardDistributor newRewardDistributor) public {
902             require(msg.sender == admin, "Unauthorized");
903     
904             MultiRewardDistributor oldRewardDistributor = rewardDistributor;
905     
906             rewardDistributor = newRewardDistributor;
907     
908             emit NewRewardDistributor(oldRewardDistributor, newRewardDistributor);
909         }


911         function _setMintPaused(MToken mToken, bool state) public returns (bool) {
912             require(markets[address(mToken)].isListed, "cannot pause a market that is not listed");
913             require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause");
914             require(msg.sender == admin || state == true, "only admin can unpause");
915     
916             mintGuardianPaused[address(mToken)] = state;
917             emit ActionPaused(mToken, "Mint", state);
918             return state;
919         }


921         function _setBorrowPaused(MToken mToken, bool state) public returns (bool) {
922             require(markets[address(mToken)].isListed, "cannot pause a market that is not listed");
923             require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause");
924             require(msg.sender == admin || state == true, "only admin can unpause");
925     
926             borrowGuardianPaused[address(mToken)] = state;
927             emit ActionPaused(mToken, "Borrow", state);
928             return state;
929         }


931         function _setTransferPaused(bool state) public returns (bool) {
932             require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause");
933             require(msg.sender == admin || state == true, "only admin can unpause");
934     
935             transferGuardianPaused = state;
936             emit ActionPaused("Transfer", state);
937             return state;
938         }


940         function _setSeizePaused(bool state) public returns (bool) {
941             require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause");
942             require(msg.sender == admin || state == true, "only admin can unpause");
943     
944             seizeGuardianPaused = state;
945             emit ActionPaused("Seize", state);
946             return state;
947         }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L940:947

File: 2023-07-moonwell/src/core/Oracles/ChainlinkOracle.sol


118         function setUnderlyingPrice(
119             MToken mToken,
120             uint256 underlyingPriceMantissa
121         ) external onlyAdmin {
122             address asset = address(MErc20(address(mToken)).underlying());
123             emit PricePosted(
124                 asset,
125                 prices[asset],
126                 underlyingPriceMantissa,
127                 underlyingPriceMantissa
128             );
129             prices[asset] = underlyingPriceMantissa;
130         }


135         function setDirectPrice(address asset, uint256 price) external onlyAdmin {
136             emit PricePosted(asset, prices[asset], price, price);
137             prices[asset] = price;
138         }


144         function setFeed(string calldata symbol, address feed) external onlyAdmin {
145             require(
146                 feed != address(0) && feed != address(this),
147                 "invalid feed address"
148             );
149             emit FeedSet(feed, symbol);
150             feeds[keccak256(abi.encodePacked(symbol))] = AggregatorV3Interface(
151                 feed
152             );
153         }


173         function setAdmin(address newAdmin) external onlyAdmin {
174             address oldAdmin = admin;
175             admin = newAdmin;
176     
177             emit NewAdmin(oldAdmin, newAdmin);
178         }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkOracle.sol#L173:178

File: 2023-07-moonwell/src/core/Unitroller.sol


39          function _setPendingImplementation(address newPendingImplementation) public returns (uint) {
40      
41              if (msg.sender != admin) {
42                  return fail(Error.UNAUTHORIZED, FailureInfo.SET_PENDING_IMPLEMENTATION_OWNER_CHECK);
43              }
44      
45              address oldPendingImplementation = pendingComptrollerImplementation;
46      
47              pendingComptrollerImplementation = newPendingImplementation;
48      
49              emit NewPendingImplementation(oldPendingImplementation, pendingComptrollerImplementation);
50      
51              return uint(Error.NO_ERROR);
52          }


86          function _setPendingAdmin(address newPendingAdmin) public returns (uint) {
87              // Check caller = admin
88              if (msg.sender != admin) {
89                  return fail(Error.UNAUTHORIZED, FailureInfo.SET_PENDING_ADMIN_OWNER_CHECK);
90              }
91      
92              // Save current value, if any, for inclusion in log
93              address oldPendingAdmin = pendingAdmin;
94      
95              // Store pendingAdmin with value newPendingAdmin
96              pendingAdmin = newPendingAdmin;
97      
98              // Emit NewPendingAdmin(oldPendingAdmin, newPendingAdmin)
99              emit NewPendingAdmin(oldPendingAdmin, newPendingAdmin);
100     
101             return uint(Error.NO_ERROR);
102         }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Unitroller.sol#L86:102

File: 2023-07-moonwell/src/core/MErc20Delegator.sol


61          function _setImplementation(address implementation_, bool allowResign, bytes memory becomeImplementationData) override public {
62              require(msg.sender == admin, "MErc20Delegator::_setImplementation: Caller must be admin");
63      
64              if (allowResign) {
65                  delegateToImplementation(abi.encodeWithSignature("_resignImplementation()"));
66              }
67      
68              address oldImplementation = implementation;
69              implementation = implementation_;
70      
71              delegateToImplementation(abi.encodeWithSignature("_becomeImplementation(bytes)", becomeImplementationData));
72      
73              emit NewImplementation(oldImplementation, implementation);
74          }


372         function _setPendingAdmin(address payable newPendingAdmin) override external returns (uint) {
373             bytes memory data = delegateToImplementation(abi.encodeWithSignature("_setPendingAdmin(address)", newPendingAdmin));
374             return abi.decode(data, (uint));
375         }


382         function _setComptroller(ComptrollerInterface newComptroller) override public returns (uint) {
383             bytes memory data = delegateToImplementation(abi.encodeWithSignature("_setComptroller(address)", newComptroller));
384             return abi.decode(data, (uint));
385         }


392         function _setReserveFactor(uint newReserveFactorMantissa) override external returns (uint) {
393             bytes memory data = delegateToImplementation(abi.encodeWithSignature("_setReserveFactor(uint256)", newReserveFactorMantissa));
394             return abi.decode(data, (uint));
395         }


433         function _setInterestRateModel(InterestRateModel newInterestRateModel) override public returns (uint) {
434             bytes memory data = delegateToImplementation(abi.encodeWithSignature("_setInterestRateModel(address)", newInterestRateModel));
435             return abi.decode(data, (uint));
436         }


443         function _setProtocolSeizeShare(uint newProtocolSeizeShareMantissa) override external returns (uint) {
444             bytes memory data = delegateToImplementation(abi.encodeWithSignature("_setProtocolSeizeShare(uint256)", newProtocolSeizeShareMantissa));
445             return abi.decode(data, (uint));
446         }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MErc20Delegator.sol#L443:446

File: 2023-07-moonwell/src/core/Governance/TemporalGovernor.sol


137         function setTrustedSenders(
138             TrustedSender[] calldata _trustedSenders
139         ) external {
140             require(
141                 msg.sender == address(this),
142                 "TemporalGovernor: Only this contract can update trusted senders"
143             );
144     
145             unchecked {
146                 for (uint256 i = 0; i < _trustedSenders.length; i++) {
147                     trustedSenders[_trustedSenders[i].chainId].add(
148                         addressToBytes(_trustedSenders[i].addr)
149                     );
150     
151                     emit TrustedSenderUpdated(
152                         _trustedSenders[i].chainId,
153                         _trustedSenders[i].addr,
154                         true /// added to list
155                     );
156                 }
157             }
158         }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Governance/TemporalGovernor.sol#L137:158

File: 2023-07-moonwell/src/core/MToken.sol


1145        function _setPendingAdmin(address payable newPendingAdmin) override external returns (uint) {
1146            // Check caller = admin
1147            if (msg.sender != admin) {
1148                return fail(Error.UNAUTHORIZED, FailureInfo.SET_PENDING_ADMIN_OWNER_CHECK);
1149            }
1150    
1151            // Save current value, if any, for inclusion in log
1152            address oldPendingAdmin = pendingAdmin;
1153    
1154            // Store pendingAdmin with value newPendingAdmin
1155            pendingAdmin = newPendingAdmin;
1156    
1157            // Emit NewPendingAdmin(oldPendingAdmin, newPendingAdmin)
1158            emit NewPendingAdmin(oldPendingAdmin, newPendingAdmin);
1159    
1160            return uint(Error.NO_ERROR);
1161        }


1195        function _setComptroller(ComptrollerInterface newComptroller) override public returns (uint) {
1196            // Check caller is admin
1197            if (msg.sender != admin) {
1198                return fail(Error.UNAUTHORIZED, FailureInfo.SET_COMPTROLLER_OWNER_CHECK);
1199            }
1200    
1201            ComptrollerInterface oldComptroller = comptroller;
1202            // Ensure invoke comptroller.isComptroller() returns true
1203            require(newComptroller.isComptroller(), "marker method returned false");
1204    
1205            // Set market's comptroller to newComptroller
1206            comptroller = newComptroller;
1207    
1208            // Emit NewComptroller(oldComptroller, newComptroller)
1209            emit NewComptroller(oldComptroller, newComptroller);
1210    
1211            return uint(Error.NO_ERROR);
1212        }


1219        function _setReserveFactor(uint newReserveFactorMantissa) override external nonReentrant returns (uint) {
1220            uint error = accrueInterest();
1221            if (error != uint(Error.NO_ERROR)) {
1222                // accrueInterest emits logs on errors, but on top of that we want to log the fact that an attempted reserve factor change failed.
1223                return fail(Error(error), FailureInfo.SET_RESERVE_FACTOR_ACCRUE_INTEREST_FAILED);
1224            }
1225            // _setReserveFactorFresh emits reserve-factor-specific logs on errors, so we don't need to.
1226            return _setReserveFactorFresh(newReserveFactorMantissa);
1227        }


1234        function _setReserveFactorFresh(uint newReserveFactorMantissa) internal returns (uint) {
1235            // Check caller is admin
1236            if (msg.sender != admin) {
1237                return fail(Error.UNAUTHORIZED, FailureInfo.SET_RESERVE_FACTOR_ADMIN_CHECK);
1238            }
1239    
1240            // Verify market's block timestamp equals current block timestamp
1241            if (accrualBlockTimestamp != getBlockTimestamp()) {
1242                return fail(Error.MARKET_NOT_FRESH, FailureInfo.SET_RESERVE_FACTOR_FRESH_CHECK);
1243            }
1244    
1245            // Check newReserveFactor ≤ maxReserveFactor
1246            if (newReserveFactorMantissa > reserveFactorMaxMantissa) {
1247                return fail(Error.BAD_INPUT, FailureInfo.SET_RESERVE_FACTOR_BOUNDS_CHECK);
1248            }
1249    
1250            uint oldReserveFactorMantissa = reserveFactorMantissa;
1251            reserveFactorMantissa = newReserveFactorMantissa;
1252    
1253            emit NewReserveFactor(oldReserveFactorMantissa, newReserveFactorMantissa);
1254    
1255            return uint(Error.NO_ERROR);
1256        }


1391        function _setInterestRateModel(InterestRateModel newInterestRateModel) override public returns (uint) {
1392            uint error = accrueInterest();
1393            if (error != uint(Error.NO_ERROR)) {
1394                // accrueInterest emits logs on errors, but on top of that we want to log the fact that an attempted change of interest rate model failed
1395                return fail(Error(error), FailureInfo.SET_INTEREST_RATE_MODEL_ACCRUE_INTEREST_FAILED);
1396            }
1397            // _setInterestRateModelFresh emits interest-rate-model-update-specific logs on errors, so we don't need to.
1398            return _setInterestRateModelFresh(newInterestRateModel);
1399        }


1407        function _setInterestRateModelFresh(InterestRateModel newInterestRateModel) internal returns (uint) {
1408    
1409            // Used to store old model for use in the event that is emitted on success
1410            InterestRateModel oldInterestRateModel;
1411    
1412            // Check caller is admin
1413            if (msg.sender != admin) {
1414                return fail(Error.UNAUTHORIZED, FailureInfo.SET_INTEREST_RATE_MODEL_OWNER_CHECK);
1415            }
1416    
1417            // We fail gracefully unless market's block timestamp equals current block timestamp
1418            if (accrualBlockTimestamp != getBlockTimestamp()) {
1419                return fail(Error.MARKET_NOT_FRESH, FailureInfo.SET_INTEREST_RATE_MODEL_FRESH_CHECK);
1420            }
1421    
1422            // Track the market's current interest rate model
1423            oldInterestRateModel = interestRateModel;
1424    
1425            // Ensure invoke newInterestRateModel.isInterestRateModel() returns true
1426            require(newInterestRateModel.isInterestRateModel(), "marker method returned false");
1427    
1428            // Set the interest rate model to newInterestRateModel
1429            interestRateModel = newInterestRateModel;
1430    
1431            // Emit NewMarketInterestRateModel(oldInterestRateModel, newInterestRateModel)
1432            emit NewMarketInterestRateModel(oldInterestRateModel, newInterestRateModel);
1433    
1434            return uint(Error.NO_ERROR);
1435        }


1443        function _setProtocolSeizeShare(uint newProtocolSeizeShareMantissa) override external nonReentrant returns (uint) {
1444            uint error = accrueInterest();
1445            if (error != uint(Error.NO_ERROR)) {
1446                // accrueInterest emits logs on errors, but on top of that we want to log the fact that an attempted change of protocol seize share failed
1447                return fail(Error(error), FailureInfo.SET_PROTOCOL_SEIZE_SHARE_ACCRUE_INTEREST_FAILED);
1448            }
1449            // _setProtocolSeizeShareFresh emits protocol-seize-share-update-specific logs on errors, so we don't need to.
1450            return _setProtocolSeizeShareFresh(newProtocolSeizeShareMantissa);
1451        }


1459        function _setProtocolSeizeShareFresh(uint newProtocolSeizeShareMantissa) internal returns (uint) {
1460    
1461            // Used to store old share for use in the event that is emitted on success
1462            uint oldProtocolSeizeShareMantissa;
1463    
1464            // Check caller is admin
1465            if (msg.sender != admin) {
1466                return fail(Error.UNAUTHORIZED, FailureInfo.SET_PROTOCOL_SEIZE_SHARE_OWNER_CHECK);
1467            }
1468    
1469            // We fail gracefully unless market's block timestamp equals current block timestamp
1470            if (accrualBlockTimestamp != getBlockTimestamp()) {
1471                return fail(Error.MARKET_NOT_FRESH, FailureInfo.SET_PROTOCOL_SEIZE_SHARE_FRESH_CHECK);
1472            }
1473    
1474            // Track the market's current protocol seize share
1475            oldProtocolSeizeShareMantissa = protocolSeizeShareMantissa;
1476    
1477            // Set the protocol seize share to newProtocolSeizeShareMantissa
1478            protocolSeizeShareMantissa = newProtocolSeizeShareMantissa;
1479    
1480            // Emit NewProtocolSeizeShareMantissa(oldProtocolSeizeShareMantissa, newProtocolSeizeShareMantissa)
1481            emit NewProtocolSeizeShare(oldProtocolSeizeShareMantissa, newProtocolSeizeShareMantissa);
1482    
1483            return uint(Error.NO_ERROR);
1484        }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MToken.sol#L1459:1484

L018 - Empty receive()/payable fallback() function does not authorize requests:

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth))). Having no access control on the function means that someone may send Ether to the contract, and have no way to get anything back out, which is a loss of funds. If the concern is having to spend a small amount of gas to check the sender against an immutable address, the code should at least have a function to rescue unused Ether.

File: 2023-07-moonwell/src/core/router/WETHRouter.sol


65          receive() external payable {}

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/router/WETHRouter.sol#L65:65

L019 - Contracts are designed to receive ETH but do not implement function for withdrawal:

The following contracts can receive ETH but can not withdraw, ETH is occasionally sent by users will be stuck in those contracts. This functionality also applies to baseTokens resulting in locked tokens and loss of funds.

File: 2023-07-moonwell/src/core/router/WETHRouter.sol


65          receive() external payable {}

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/router/WETHRouter.sol#L65:65

L020 - Functions calling contracts/addresses with transfer hooks are missing reentrancy guards:

Even if the function follows the best practice of check-effects-interaction, not using a reentrancy guard when there may be transfer hooks will open the users of this protocol up to read-only reentrancies with no way to protect against it, except by block-listing the whole protocol.

File: 2023-07-moonwell/src/core/router/WETHRouter.sol


36              IERC20(address(mToken)).safeTransfer(


46              IERC20(address(mToken)).safeTransferFrom(

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/router/WETHRouter.sol#L46:46

L021 - Some ERC20 revert on zero value transfer:

Example: https://github.com/d-xo/weird-erc20#revert-on-zero-value-transfers.

Click to show 10 findings

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


1237                token.safeTransfer(_user, _amount);


483                 token.safeTransfer(comptroller.admin(), _amount);


478                 token.safeTransfer(

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L478:478

File: 2023-07-moonwell/src/core/Comptroller.sol


967                 token.transfer(admin, _amount);


965                 token.transfer(admin, token.balanceOf(address(this)));

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L965:965

File: 2023-07-moonwell/src/core/MErc20.sol


225             token.transfer(to, amount);


190             token.transferFrom(from, address(this), amount);


152         	token.transfer(admin, balance);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MErc20.sol#L152:152

File: 2023-07-moonwell/src/core/router/WETHRouter.sol


36              IERC20(address(mToken)).safeTransfer(


46              IERC20(address(mToken)).safeTransferFrom(

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/router/WETHRouter.sol#L46:46

NC001 - TODO Left in the code:

TODOs may signal that a feature is missing or not ready for audit, consider resolving the issue and removing the TODO comment

File: 2023-07-moonwell/test/proposals/mips/mip00.sol


addresses.getAddress("GUARDIAN") /// TODO figure out what the pause guardian is on Base, then replace it

/// TODO calculate initial exchange rate

https://github.com/code-423n4/2023-07-moonwell/tree/main/test/proposals/mips/mip00.sol#L210:210

NC002 - External calls in an unbounded for-loop may result in a DoS:

Consider limiting the number of iterations in for-loops that make external calls.

Click to show 18 findings

File: 2023-07-moonwell/test/proposals/mips/mip00.sol


275                 for (uint256 i = 0; i < cTokenConfigs.length; i++) {


334                 for (uint256 i = 0; i < cTokenConfigs.length; i++) {


443                 for (uint256 i = 0; i < emissionConfig.length; i++) {


706                     for (uint256 i = 0; i < cTokenConfigs.length; i++) {


799                     for (uint256 i = 0; i < emissionConfig.length; i++) {

https://github.com/code-423n4/2023-07-moonwell/tree/main/test/proposals/mips/mip00.sol#L799:799

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


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++) {


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++) {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L1163:1163

File: 2023-07-moonwell/src/core/Comptroller.sol


574             for (uint i = 0; i < assets.length; i++) {


795             for (uint i = 0; i < allMarkets.length; i ++) {


1031            for (uint i = 0; i < mTokens.length; i++) {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L1031:1031

File: 2023-07-moonwell/src/core/Governance/TemporalGovernor.sol


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


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


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

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Governance/TemporalGovernor.sol#L394:394

NC003 - Constants should be defined rather than using magic numbers:

Even assembly can benefit from using readable constants instead of hex/numeric literals.

Click to show 38 findings

File: 2023-07-moonwell/test/proposals/mips/mip00.sol


217                         (ERC20(config.tokenAddress).decimals() + 8)) * 2;


578                 uint256 initialExchangeRateUsdc = (10 ** (6 + 8)) * 2;


579                 uint256 initialExchangeRateEth = (10 ** (18 + 8)) * 2;


627                 assertEq(distributor.emissionCap(), 100e18);


628                 assertEq(distributor.initialIndexConstant(), 1e36);


650                 bytes32 _ADMIN_SLOT = 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103;


663                 bytes32 _IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;


768                                 (config.jrm.baseRatePerYear * 1e18) /


770                                     1e18


774                                 (config.jrm.multiplierPerYear * 1e18) /


776                                     1e18


780                                 (config.jrm.jumpMultiplierPerYear * 1e18) /


782                                     1e18


818                         assertEq(marketConfig.supplyGlobalIndex, 1e36);


819                         assertEq(marketConfig.borrowGlobalIndex, 1e36);

https://github.com/code-423n4/2023-07-moonwell/tree/main/test/proposals/mips/mip00.sol#L819:819

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


110             emissionCap = 100e18;

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L110:110

File: 2023-07-moonwell/src/core/Oracles/ChainlinkCompositeOracle.sol


109                 expectedDecimals > uint8(0) && expectedDecimals <= uint8(18),


140                 expectedDecimals > uint8(0) && expectedDecimals <= uint8(18),

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkCompositeOracle.sol#L140:140

File: 2023-07-moonwell/src/core/Oracles/ChainlinkOracle.sol


85              uint256 decimalDelta = uint256(18).sub(uint256(token.decimals()));


106             uint256 decimalDelta = uint256(18).sub(feed.decimals());

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkOracle.sol#L106:106

File: 2023-07-moonwell/src/core/IRModels/JumpRateModel.sol


53              baseRatePerTimestamp = baseRatePerYear.mul(1e18).div(timestampsPerYear).div(1e18);


54              multiplierPerTimestamp = multiplierPerYear.mul(1e18).div(timestampsPerYear).div(1e18);


55              jumpMultiplierPerTimestamp = jumpMultiplierPerYear.mul(1e18).div(timestampsPerYear).div(1e18);


74              return borrows.mul(1e18).div(cash.add(borrows).sub(reserves));


88                  return util.mul(multiplierPerTimestamp).div(1e18).add(baseRatePerTimestamp);


90                  uint normalRate = kink.mul(multiplierPerTimestamp).div(1e18).add(baseRatePerTimestamp);


92                  return excessUtil.mul(jumpMultiplierPerTimestamp).div(1e18).add(normalRate);


105             uint oneMinusReserveFactor = uint(1e18).sub(reserveFactorMantissa);


107             uint rateToPool = borrowRate.mul(oneMinusReserveFactor).div(1e18);


108             return utilizationRate(cash, borrows, reserves).mul(rateToPool).div(1e18);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/IRModels/JumpRateModel.sol#L108:108

File: 2023-07-moonwell/src/core/IRModels/WhitePaperInterestRateModel.sol


38              baseRatePerTimestamp = baseRatePerYear.mul(1e18).div(timestampsPerYear).div(1e18);


39              multiplierPerTimestamp = multiplierPerYear.mul(1e18).div(timestampsPerYear).div(1e18);


57              return borrows.mul(1e18).div(cash.add(borrows).sub(reserves));


69              return ur.mul(multiplierPerTimestamp).div(1e18).add(baseRatePerTimestamp);


81              uint oneMinusReserveFactor = uint(1e18).sub(reserveFactorMantissa);


83              uint rateToPool = borrowRate.mul(oneMinusReserveFactor).div(1e18);


84              return utilizationRate(cash, borrows, reserves).mul(rateToPool).div(1e18);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/IRModels/WhitePaperInterestRateModel.sol#L84:84

File: 2023-07-moonwell/src/core/Governance/TemporalGovernor.sol


130             return bytes32(bytes20(addr)) >> 96;

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Governance/TemporalGovernor.sol#L130:130

NC004 - Numeric values having to do with time should use time units for readability:

There are units for seconds, minutes, hours, days, and weeks, and since they're defined, they should be used.

File: 2023-07-moonwell/src/core/IRModels/WhitePaperInterestRateModel.sol


20          uint public constant timestampsPerYear = 31536000;

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/IRModels/WhitePaperInterestRateModel.sol#L20:20

NC005 - Events that mark critical parameter changes should contain both the old and the new value:

This should especially be done if the new value is not required to be different from the old value.

Click to show 6 findings

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


631             emit RewardsPaused();


638             emit RewardsUnpaused();

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L638:638

File: 2023-07-moonwell/src/core/Comptroller.sol


789             emit MarketListed(mToken);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L789:789

File: 2023-07-moonwell/src/core/Governance/TemporalGovernor.sol


197             emit GuardianPauseGranted(block.timestamp);


220             emit GuardianRevoked(oldGuardian);


258             emit PermissionlessUnpaused(block.timestamp);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Governance/TemporalGovernor.sol#L258:258

NC006 - Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18):

While the compiler knows to optimize away the exponentiation, it's still better coding practice to use idioms that do not require compiler optimization, if they exist.

Click to show 9 findings

File: 2023-07-moonwell/test/proposals/mips/mip00.sol


216                     uint256 initialExchangeRate = (10 **
217                         (ERC20(config.tokenAddress).decimals() + 8)) * 2;


578                 uint256 initialExchangeRateUsdc = (10 ** (6 + 8)) * 2;


579                 uint256 initialExchangeRateEth = (10 ** (18 + 8)) * 2;

https://github.com/code-423n4/2023-07-moonwell/tree/main/test/proposals/mips/mip00.sol#L579:579

File: 2023-07-moonwell/src/core/Oracles/ChainlinkCompositeOracle.sol


113             int256 scalingFactor = int256(10 ** uint256(expectedDecimals)); /// calculate expected decimals for end quote


145             int256 scalingFactor = int256(10 ** uint256(expectedDecimals * 2)); /// calculate expected decimals for end quote


209                     price * (10 ** uint256(expectedDecimals - priceDecimals)).toInt256();


212                     price / (10 ** uint256(priceDecimals - expectedDecimals)).toInt256();

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkCompositeOracle.sol#L212:212

File: 2023-07-moonwell/src/core/Oracles/ChainlinkOracle.sol


88                  return price.mul(10 ** decimalDelta);


109                 return uint256(answer).mul(10 ** decimalDelta);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkOracle.sol#L109:109

NC007 - Event is not properly indexed:

Index event fields make the field more quickly accessible to off-chain tools that parse events. This is especially useful when it comes to filtering based on an address. 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). Where applicable, each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three applicable fields, all of the applicable fields should be indexed.

Click to show 12 findings

File: 2023-07-moonwell/src/core/Comptroller.sol


20          event MarketEntered(MToken mToken, address account);


23          event MarketExited(MToken mToken, address account);


38          event NewPauseGuardian(address oldPauseGuardian, address newPauseGuardian);


50          event NewBorrowCapGuardian(address oldBorrowCapGuardian, address newBorrowCapGuardian);


56          event NewSupplyCapGuardian(address oldSupplyCapGuardian, address newSupplyCapGuardian);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L56:56

File: 2023-07-moonwell/src/core/Oracles/ChainlinkOracle.sol


30          event PricePosted(
31              address asset,
32              uint256 previousPriceMantissa,
33              uint256 requestedPriceMantissa,
34              uint256 newPriceMantissa
35          );


38          event NewAdmin(address oldAdmin, address newAdmin);


41          event FeedSet(address feed, string symbol);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkOracle.sol#L41:41

File: 2023-07-moonwell/src/core/Unitroller.sol


16          event NewPendingImplementation(address oldPendingImplementation, address newPendingImplementation);


21          event NewImplementation(address oldImplementation, address newImplementation);


26          event NewPendingAdmin(address oldPendingAdmin, address newPendingAdmin);


31          event NewAdmin(address oldAdmin, address newAdmin);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Unitroller.sol#L31:31

NC008 - Function ordering does not follow the Solidity style guide:

According to the Solidity style guide, functions should be laid out in the following order :constructor(), receive(), fallback(), external, public, internal, private, but the cases below do not follow this pattern.

Click to show 34 findings

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


333         function getCurrentEmissionCap() external view returns (uint256) {


382         function _addEmissionConfig(

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L382:382

File: 2023-07-moonwell/src/core/Comptroller.sol


154         function exitMarket(address mTokenAddress) override external returns (uint) {


292         function redeemVerify(address mToken, address redeemer, uint redeemAmount, uint redeemTokens) override pure external {


542         function getHypotheticalAccountLiquidity(


629         function liquidateCalculateSeizeTokens(address mTokenBorrowed, address mTokenCollateral, uint actualRepayAmount) override external view returns (uint, uint) {


689         function _setCloseFactor(uint newCloseFactorMantissa) external returns (uint) {


807         function _setMarketBorrowCaps(MToken[] calldata mTokens, uint[] calldata newBorrowCaps) external {


959         function _rescueFunds(address _tokenAddress, uint _amount) external {


998         function claimReward() public {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L998:998

File: 2023-07-moonwell/src/core/Oracles/ChainlinkOracle.sol


118         function setUnderlyingPrice(


167         function assetPrices(address asset) external view returns (uint256) {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkOracle.sol#L167:167

File: 2023-07-moonwell/src/core/Unitroller.sol


136         fallback() external {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Unitroller.sol#L136:136

File: 2023-07-moonwell/src/core/MErc20Delegator.sol


82          function mint(uint mintAmount) override external returns (uint) {


326         function getCash() override external view returns (uint) {


350         function seize(address liquidator, address borrower, uint seizeTokens) override external returns (uint) {


392         function _setReserveFactor(uint newReserveFactorMantissa) override external returns (uint) {


443         function _setProtocolSeizeShare(uint newProtocolSeizeShareMantissa) override external returns (uint) {


471         function delegateToImplementation(bytes memory data) public returns (bytes memory) {


496         fallback () external payable {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MErc20Delegator.sol#L496:496

File: 2023-07-moonwell/src/core/Governance/TemporalGovernor.sol


96          function isTrustedSender(


137         function setTrustedSenders(


242         function permissionlessUnpause() external whenPaused {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Governance/TemporalGovernor.sol#L242:242

File: 2023-07-moonwell/src/core/MErc20.sol


46          function mint(uint mintAmount) override external returns (uint) {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MErc20.sol#L46:46

File: 2023-07-moonwell/src/core/MToken.sol


136         function transfer(address dst, uint256 amount) override external nonReentrant returns (bool) {


236         function borrowRatePerTimestamp() override external view returns (uint) {


319         function exchangeRateCurrent() override public nonReentrant returns (uint) {


376         function getCash() override external view returns (uint) {


1048        function seize(address liquidator, address borrower, uint seizeTokens) override external nonReentrant returns (uint) {


1145        function _setPendingAdmin(address payable newPendingAdmin) override external returns (uint) {


1219        function _setReserveFactor(uint newReserveFactorMantissa) override external nonReentrant returns (uint) {


1326        function _reduceReserves(uint reduceAmount) override external nonReentrant returns (uint) {


1391        function _setInterestRateModel(InterestRateModel newInterestRateModel) override public returns (uint) {


1443        function _setProtocolSeizeShare(uint newProtocolSeizeShareMantissa) override external nonReentrant returns (uint) {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MToken.sol#L1443:1443

NC009 - Imports could be organized more systematically:

This issue arises when the contract's interface is not imported first, followed by each of the interfaces it uses, followed by all other files.

File: 2023-07-moonwell/src/core/MToken.sol


9       import "./IRModels/InterestRateModel.sol";

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MToken.sol#L9:9

NC010 - Constants in comparisons should appear on the left side:

This issue arises when constants in comparisons appear on the right side, which can lead to typo bugs.

Click to show 50 findings

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


837                 userSupplyIndex == 0 && _globalSupplyIndex >= initialIndexConstant


876                 userBorrowIndex == 0 && _globalBorrowIndex >= initialIndexConstant


948             if (deltaTimestamps == 0 || _emissionsPerSecond == 0) {


1220            if (_amount == 0) {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L1220:1220

File: 2023-07-moonwell/src/core/Comptroller.sol


129             if (marketToJoin.accountMembership[borrower] == true) {


158             require(oErr == 0, "exitMarket: getAccountSnapshot failed"); // semi-opaque error code


161             if (amountOwed != 0) {


167             if (allowed != 0) {


228             if (supplyCap != 0) {


298             if (redeemTokens == 0 && redeemAmount > 0) {


332             if (oracle.getUnderlyingPrice(MToken(mToken)) == 0) {


338             if (borrowCap != 0) {


412             if (shortfall == 0) {


579                 if (oErr != 0) { // semi-opaque error code, we assume NO_ERROR == 0 is invariant between upgrades


587                 if (vars.oraclePriceMantissa == 0) {


633             if (priceBorrowedMantissa == 0 || priceCollateralMantissa == 0) {


728             if (newCollateralFactorMantissa != 0 && oracle.getUnderlyingPrice(mToken) == 0) {


813             require(numMarkets != 0 && numMarkets == numBorrowCaps, "invalid input");


850             require(numMarkets != 0 && numMarkets == numSupplyCaps, "invalid input");


914             require(msg.sender == admin || state == true, "only admin can unpause");


924             require(msg.sender == admin || state == true, "only admin can unpause");


933             require(msg.sender == admin || state == true, "only admin can unpause");


942             require(msg.sender == admin || state == true, "only admin can unpause");


951             require(unitroller._acceptImplementation() == 0, "change not authorized");


1038                if (suppliers == true) {


1046                if (borrowers == true) {


1077            require(_locked != 1, "ReentrancyGuard: reentrant call");

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L1077:1077

File: 2023-07-moonwell/src/core/Oracles/ChainlinkOracle.sol


79              if (prices[address(token)] != 0) {


103             require(updatedAt != 0, "Round is in incompleted state");

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkOracle.sol#L103:103

File: 2023-07-moonwell/src/core/MErc20Delegator.sol


497             require(msg.value == 0,"MErc20Delegator:fallback: cannot send value to fallback");

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MErc20Delegator.sol#L497:497

File: 2023-07-moonwell/src/core/IRModels/JumpRateModel.sol


70              if (borrows == 0) {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/IRModels/JumpRateModel.sol#L70:70

File: 2023-07-moonwell/src/core/IRModels/WhitePaperInterestRateModel.sol


53              if (borrows == 0) {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/IRModels/WhitePaperInterestRateModel.sol#L53:53

File: 2023-07-moonwell/src/core/Governance/TemporalGovernor.sol


332                 queuedTransactions[vm.hash].queueTime == 0,


356                     queuedTransactions[vm.hash].queueTime != 0,


364             } else if (queuedTransactions[vm.hash].queueTime == 0) {


417             require(targets.length != 0, "TemporalGovernor: Empty proposal");

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Governance/TemporalGovernor.sol#L417:417

File: 2023-07-moonwell/src/core/MToken.sol


33              require(accrualBlockTimestamp == 0 && borrowIndex == 0, "market may only be initialized once");


71              if (allowed != 0) {


295             if (borrowSnapshot.principal == 0) {


342             if (_totalSupply == 0) {


501             if (allowed != 0) {


616             require(redeemTokensIn == 0 || redeemAmountIn == 0, "one of redeemTokensIn or redeemAmountIn must be zero");


668             if (allowed != 0) {


753             if (allowed != 0) {


868             if (allowed != 0) {


971             if (allowed != 0) {


991             if (repayAmount == 0) {


1077            if (allowed != 0) {

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MToken.sol#L1077:1077

File: 2023-07-moonwell/src/core/router/WETHRouter.sol


34              require(mToken.mint(msg.value) == 0, "WETHRouter: mint failed");


53                  mToken.redeem(mTokenRedeemAmount) == 0,

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/router/WETHRouter.sol#L53:53

NC011 - else-block not required:

One level of nesting can be removed by not having an else block when the if-block returns

Click to show 10 findings

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


1235            if (_amount > 0 && _amount <= currentTokenHoldings) {
1236                // Ensure we use SafeERC20 to revert even if the reward token isn't ERC20 compliant
1237                token.safeTransfer(_user, _amount);
1238                return 0;
1239            } else {
1240                // If we've hit here it means we weren't able to emit the reward and we should emit an event
1241                // instead of failing.
1242                emit InsufficientTokensToEmit(_user, _rewardToken, _amount);
1243    
1244                // By default, return the same amount as what's left over to send, we accrue reward but don't send them out
1245                return _amount;
1246            }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L1235:1246

File: 2023-07-moonwell/src/core/Comptroller.sol


614             if (vars.sumCollateral > vars.sumBorrowPlusEffects) {
615                 return (Error.NO_ERROR, vars.sumCollateral - vars.sumBorrowPlusEffects, 0);
616             } else {
617                 return (Error.NO_ERROR, 0, vars.sumBorrowPlusEffects - vars.sumCollateral);
618             }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Comptroller.sol#L614:618

File: 2023-07-moonwell/src/core/Oracles/ChainlinkCompositeOracle.sol


207             if (priceDecimals < expectedDecimals) {
208                 return
209                     price * (10 ** uint256(expectedDecimals - priceDecimals)).toInt256();
210             } else if (priceDecimals > expectedDecimals) {
211                 return
212                     price / (10 ** uint256(priceDecimals - expectedDecimals)).toInt256();
213             }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkCompositeOracle.sol#L207:213

File: 2023-07-moonwell/src/core/Oracles/ChainlinkOracle.sol


62              if (keccak256(abi.encodePacked(symbol)) == nativeToken) { /// @dev this branch should never get called as native tokens are not supported on this deployment
63                  return getChainlinkPrice(getFeed(symbol));
64              } else {
65                  return getPrice(mToken);
66              }


87              if (decimalDelta > 0) {
88                  return price.mul(10 ** decimalDelta);
89              } else {
90                  return price;
91              }


108             if (decimalDelta > 0) {
109                 return uint256(answer).mul(10 ** decimalDelta);
110             } else {
111                 return uint256(answer);
112             }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkOracle.sol#L108:112

File: 2023-07-moonwell/src/core/IRModels/JumpRateModel.sol


87              if (util <= kink) {
88                  return util.mul(multiplierPerTimestamp).div(1e18).add(baseRatePerTimestamp);
89              } else {
90                  uint normalRate = kink.mul(multiplierPerTimestamp).div(1e18).add(baseRatePerTimestamp);
91                  uint excessUtil = util.sub(kink);
92                  return excessUtil.mul(jumpMultiplierPerTimestamp).div(1e18).add(normalRate);
93              }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/IRModels/JumpRateModel.sol#L87:93

File: 2023-07-moonwell/src/core/MToken.sol


342             if (_totalSupply == 0) {
343                 /*
344                  * If there are no tokens minted:
345                  *  exchangeRate = initialExchangeRate
346                  */
347                 return (MathError.NO_ERROR, initialExchangeRateMantissa);
348             } else {
349                 /*
350                  * Otherwise:
351                  *  exchangeRate = (totalCash + totalBorrows - totalReserves) / totalSupply
352                  */
353                 uint totalCash = getCashPrior();
354                 uint cashPlusBorrowsMinusReserves;
355                 Exp memory exchangeRate;
356                 MathError mathErr;
357     
358                 (mathErr, cashPlusBorrowsMinusReserves) = addThenSubUInt(totalCash, totalBorrows, totalReserves);
359                 if (mathErr != MathError.NO_ERROR) {
360                     return (mathErr, 0);
361                 }
362     
363                 (mathErr, exchangeRate) = getExp(cashPlusBorrowsMinusReserves, _totalSupply);
364                 if (mathErr != MathError.NO_ERROR) {
365                     return (mathErr, 0);
366                 }
367     
368                 return (MathError.NO_ERROR, exchangeRate.mantissa);
369             }


627             if (redeemTokensIn > 0) {
628                 /*
629                  * We calculate the exchange rate and the amount of underlying to be redeemed:
630                  *  redeemTokens = redeemTokensIn
631                  *  redeemAmount = redeemTokensIn x exchangeRateCurrent
632                  */
633                 if (redeemTokensIn == type(uint).max) {
634                     vars.redeemTokens = accountTokens[redeemer];
635                 } else {
636                     vars.redeemTokens = redeemTokensIn;
637                 }
638     
639                 (vars.mathErr, vars.redeemAmount) = mulScalarTruncate(Exp({mantissa: vars.exchangeRateMantissa}), vars.redeemTokens);
640                 if (vars.mathErr != MathError.NO_ERROR) {
641                     return failOpaque(Error.MATH_ERROR, FailureInfo.REDEEM_EXCHANGE_TOKENS_CALCULATION_FAILED, uint(vars.mathErr));
642                 }
643             } else {
644                 /*
645                  * We get the current exchange rate and calculate the amount to be redeemed:
646                  *  redeemTokens = redeemAmountIn / exchangeRate
647                  *  redeemAmount = redeemAmountIn
648                  */
649                 if (redeemAmountIn == type(uint).max) {
650                     vars.redeemTokens = accountTokens[redeemer];
651     
652                     (vars.mathErr, vars.redeemAmount) = mulScalarTruncate(Exp({mantissa: vars.exchangeRateMantissa}), vars.redeemTokens);
653                     if (vars.mathErr != MathError.NO_ERROR) {
654                         return failOpaque(Error.MATH_ERROR, FailureInfo.REDEEM_EXCHANGE_TOKENS_CALCULATION_FAILED, uint(vars.mathErr));
655                     }
656                 } else {
657                     vars.redeemAmount = redeemAmountIn;
658     
659                     (vars.mathErr, vars.redeemTokens) = divScalarByExpTruncate(redeemAmountIn, Exp({mantissa: vars.exchangeRateMantissa}));
660                     if (vars.mathErr != MathError.NO_ERROR) {
661                         return failOpaque(Error.MATH_ERROR, FailureInfo.REDEEM_EXCHANGE_AMOUNT_CALCULATION_FAILED, uint(vars.mathErr));
662                     }
663                 }
664             }


649                 if (redeemAmountIn == type(uint).max) {
650                     vars.redeemTokens = accountTokens[redeemer];
651     
652                     (vars.mathErr, vars.redeemAmount) = mulScalarTruncate(Exp({mantissa: vars.exchangeRateMantissa}), vars.redeemTokens);
653                     if (vars.mathErr != MathError.NO_ERROR) {
654                         return failOpaque(Error.MATH_ERROR, FailureInfo.REDEEM_EXCHANGE_TOKENS_CALCULATION_FAILED, uint(vars.mathErr));
655                     }
656                 } else {
657                     vars.redeemAmount = redeemAmountIn;
658     
659                     (vars.mathErr, vars.redeemTokens) = divScalarByExpTruncate(redeemAmountIn, Exp({mantissa: vars.exchangeRateMantissa}));
660                     if (vars.mathErr != MathError.NO_ERROR) {
661                         return failOpaque(Error.MATH_ERROR, FailureInfo.REDEEM_EXCHANGE_AMOUNT_CALCULATION_FAILED, uint(vars.mathErr));
662                     }
663                 }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MToken.sol#L649:663

NC012 - Events may be emitted out of order due to reentrancy:

Ensure that events follow the best practice of check-effects-interaction, and are emitted before external calls

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


486             emit FundsRescued(_tokenAddress, _amount);

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L486:486

NC013 - If-statement can be converted to a ternary:

The code can be made more compact while also increasing readability by converting the following if-statements to ternaries (e.g. foo += (x > y) ? a : b)

Click to show 4 findings

File: 2023-07-moonwell/src/core/MultiRewardDistributor/MultiRewardDistributor.sol


937                 if (_currentTimestamp < _rewardEndTime) {
938                     deltaTimestamps = sub_(_rewardEndTime, _currentTimestamp);
939                 } else {
940                     // Otherwise just set deltaTimestamps to 0 to ensure that we short circuit
941                     // in the next step
942                     deltaTimestamps = 0;
943                 }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L937:943

File: 2023-07-moonwell/src/core/Oracles/ChainlinkOracle.sol


79              if (prices[address(token)] != 0) {
80                  price = prices[address(token)];
81              } else {
82                  price = getChainlinkPrice(getFeed(token.symbol()));
83              }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/Oracles/ChainlinkOracle.sol#L79:83

File: 2023-07-moonwell/src/core/MToken.sol


82              if (spender == src) {
83                  startingAllowance = type(uint).max;
84              } else {
85                  startingAllowance = transferAllowances[src][spender];
86              }


1020            if (address(mTokenCollateral) == address(this)) {
1021                seizeError = seizeInternal(address(this), liquidator, borrower, seizeTokens);
1022            } else {
1023                seizeError = mTokenCollateral.seize(liquidator, borrower, seizeTokens);
1024            }

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MToken.sol#L1020:1024

NC014 - Import declarations should import specific identifiers, rather than the whole file:

Using import declarations of the form import {<identifier_name>} from 'some/file.sol' avoids polluting the symbol namespace making flattened files smaller, and speeds up compilation

File: 2023-07-moonwell/src/core/MErc20.sol


4       import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

https://github.com/code-423n4/2023-07-moonwell/tree/main/src/core/MErc20.sol#L4:4

NC015 - Public functions not called by the contract should be declared external instead:

Contracts are allowed to override their parents' functions and change the visibility from external to public.

Click to show 27 findings

File: 2023-07-moonwell/test/proposals/mips/mip00.sol


77          function deploy(Addresses addresses, address) public {
78              /// ------- TemporalGovernor -------
79      
80              console.log(
81                  "deploying governor with wormhole chain id: ",
82                  chainIdToWormHoleId[block.chainid],
83                  " as owner"
84              );
85              console.log(
86                  "governor owner: ",
87                  addresses.getAddress("MOONBEAM_TIMELOCK")
88              );
89      
90              localInit(addresses);
91      
92              {
93                  TemporalGovernor.TrustedSender[]
94                      memory trustedSenders = new TemporalGovernor.TrustedSender[](1);
95                  trustedSenders[0].chainId = chainIdToWormHoleId[block.chainid];
96                  trustedSenders[0].addr = addresses.getAddress("MOONBEAM_TIMELOCK");
97      
98                  require(
99                      addresses.getAddress("WORMHOLE_CORE") != address(0),
100                     "MIP00: WORMHOLE_CORE not set"
101                 );
102     
103                 /// this will be the governor for all the contracts
104                 TemporalGovernor governor = new TemporalGovernor(
105                     addresses.getAddress("WORMHOLE_CORE"),
106                     proposalDelay,
107                     permissionlessUnpauseTime,
108                     trustedSenders
109                 );
110     
111                 addresses.addAddress("TEMPORAL_GOVERNOR", address(governor));
112             }
113     
114             deployAndMint(addresses);
115             init(addresses);
116     
117             /// ------- Reward Distributor -------
118     
119             {
120                 MultiRewardDistributor distributor = new MultiRewardDistributor();
121                 addresses.addAddress(
122                     "MULTI_REWARD_DISTRIBUTOR",
123                     address(distributor)
124                 );
125             }
126     
127             {
128                 /// ------- Unitroller/Comptroller -------
129     
130                 Unitroller unitroller = new Unitroller();
131                 Comptroller comptroller = new Comptroller();
132     
133                 unitroller._setPendingImplementation(address(comptroller));
134                 comptroller._become(unitroller);
135     
136                 addresses.addAddress("COMPTROLLER", address(comptroller));
137                 addresses.addAddress("UNITROLLER", address(unitroller));
138     
139                 ProxyAdmin proxyAdmin = new ProxyAdmin();
140     
141                 bytes memory initData = abi.encodeWithSignature(
142                     "initialize(address,address)",
143                     address(unitroller),
144                     addresses.getAddress("GUARDIAN") /// TODO figure out what the pause guardian is on Base, then replace it
145                 );
146     
147                 TransparentUpgradeableProxy mrdProxy = new TransparentUpgradeableProxy(
148                         addresses.getAddress("MULTI_REWARD_DISTRIBUTOR"),
149                         address(proxyAdmin),
150                         initData
151                     );
152     
153                 addresses.addAddress("MRD_PROXY", address(mrdProxy));
154                 addresses.addAddress("MRD_PROXY_ADMIN", address(proxyAdmin));
155             }
156     
157             /// ------ MTOKENS -------
158     
159             {
160                 MErc20Delegate mTokenLogic = new MErc20Delegate();
161                 addresses.addAddress("MTOKEN_IMPLEMENTATION", address(mTokenLogic));
162             }
163     
164             Configs.CTokenConfiguration[]
165                 memory cTokenConfigs = getCTokenConfigurations(block.chainid);
166     
167             uint256 cTokenConfigsLength = cTokenConfigs.length;
168             //// create all of the CTokens according to the configuration in Config.sol
169             unchecked {
170                 for (uint256 i = 0; i < cTokenConfigsLength; i++) {
171                     Configs.CTokenConfiguration memory config = cTokenConfigs[i];
172     
173                     /// ----- Jump Rate IRM -------
174                     {
175                         address irModel = address(
176                             new JumpRateModel(
177                                 config.jrm.baseRatePerYear,
178                                 config.jrm.multiplierPerYear,
179                                 config.jrm.jumpMultiplierPerYear,
180                                 config.jrm.kink
181                             )
182                         );
183     
184                         addresses.addAddress(
185                             string(
186                                 abi.encodePacked(
187                                     "JUMP_RATE_IRM_",
188                                     config.addressesString
189                                 )
190                             ),
191