Moonwell Audit

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. ```solidity 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 ```solidity 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 ```solidity 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](https://docs.chain.link/data-feeds/l2-sequencer-feeds#arbitrum). 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. ```solidity 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 ```solidity 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](https://docs.chain.link/architecture-overview/off-chain-reporting) 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. ```solidity 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. <details> <summary>Click to show 6 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 </details> ## 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](https://docs.soliditylang.org/en/v0.8.13/abi-spec.html#non-standard-packed-mode) (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](https://ethereum.stackexchange.com/questions/30912/how-to-compare-strings-in-solidity#answer-82739). If all arguments are strings and or bytes, `bytes.concat()` should be used instead. <details> <summary>Click to show 4 findings</summary> ```solidity 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 </details> ## 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 ```solidity 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. ```solidity 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. ```solidity 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. ```solidity 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'. <details> <summary>Click to show 4 findings</summary> ```solidity 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 ```solidity 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 </details> ## 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. <details> <summary>Click to show 5 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 </details> ## 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. ```solidity 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. <details> <summary>Click to show 29 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 </details> ## 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. ```solidity 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. ```solidity 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. ```solidity 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 ```solidity 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 ```solidity 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](https://blog.soliditylang.org/2023/05/10/solidity-0.8.20-release-announcement/#important-note), 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](https://docs.soliditylang.org/en/v0.8.20/using-the-compiler.html?ref=zaryabs.com#setting-the-evm-version-to-target) [version](https://book.getfoundry.sh/reference/config/solidity-compiler#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. ```solidity 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 ```solidity 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 ```solidity 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](https://github.com/nomad-xyz/ExcessivelySafeCall) library instead. ```solidity 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 ```solidity 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. <details> <summary>Click to show 37 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 </details> ## 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. ```solidity 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. ```solidity 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](https://chainsecurity.com/curve-lp-oracle-manipulation-post-mortem/) with no way to protect against it, except by block-listing the whole protocol. ```solidity 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. <details> <summary>Click to show 10 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 </details> ## 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 ```solidity 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. <details> <summary>Click to show 18 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 </details> ## NC003 - Constants should be defined rather than using magic numbers: Even assembly can benefit from using readable constants instead of hex/numeric literals. <details> <summary>Click to show 38 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 </details> ## 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. ```solidity 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. <details> <summary>Click to show 6 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 </details> ## 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. <details> <summary>Click to show 9 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 </details> ## 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. <details> <summary>Click to show 12 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 </details> ## NC008 - Function ordering does not follow the Solidity style guide: According to the [Solidity style guide](https://docs.soliditylang.org/en/v0.8.17/style-guide.html#order-of-functions), 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. <details> <summary>Click to show 34 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 </details> ## 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. ```solidity 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. <details> <summary>Click to show 50 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 </details> ## NC011 - else-block not required: One level of nesting can be removed by not having an else block when the if-block returns <details> <summary>Click to show 10 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 ```solidity 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 </details> ## 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 ```solidity 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) <details> <summary>Click to show 4 findings</summary> ```solidity 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 ```solidity 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 ```solidity 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 </details> ## 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 ```solidity 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. <details> <summary>Click to show 27 findings</summary> ```solidity 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