Mimo August 2022 contest - Rolezn's results

Bridging the chasm between the DeFi world and the world of regulated financial institutions.

General Information

Platform: Code4rena

Start Date: 02/08/2022

Pot Size: $50,000 USDC

Total HM: 12

Participants: 69

Period: 5 days

Judge: gzeon

Total Solo HM: 5

Id: 150

League: ETH

Mimo DeFi

Findings Distribution

Researcher Performance

Rank: 27/69

Findings: 2

Award: $131.26

🌟 Selected for report: 0

🚀 Solo Findings: 0

(1) Missing Checks for Address(0x0)

Severity: Low

Proof Of Concept

function transferOwnership(address newOwner) external override {

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L117

function deployFor(address owner) public override returns (IMIMOProxy proxy) {

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxyFactory.sol#L45

function deployFor(address owner) public override returns (IMIMOProxy proxy) {

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxyRegistry.sol#L45

Consider adding zero-address checks in the mentioned codebase.

(2) Transferownership Should Be Two Step

Severity: Low

The owner is the authorized user in the solidity contracts. Usually, an owner can be updated with transferOwnership function. However, the process is only completed with single transaction. If the address is updated incorrectly, an owner functionality will be lost forever.

Proof Of Concept

function transferOwnership(address newOwner) external override {

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L117

proxy.transferOwnership(owner);

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxyFactory.sol#L50

Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.

(3) Unused Receive() Function Will Lock Ether In Contract

Severity: Low

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert

Proof Of Concept

receive() external payable {}

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L38

The function should call another function, otherwise it should revert

(4) Constants Should Be Defined Rather Than Using Magic Numbers

Severity: Non-Critical

Proof Of Concept

uint256 targetRatio = autoVault.targetRatio + 1e15; // add 0.1% to account for rounding

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoRebalance.sol#L180

if (_operationTracker[vaultId] > block.timestamp - 1 days) {

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoRebalance.sol#L244

if (_operationTracker[rbData.vaultId] > block.timestamp - 1 days) {

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/MIMOManagedRebalance.sol#L160

minGasReserve = 5_000;

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L30

(5) Event Is Missing Indexed Fields

Severity: Non-Critical

Each event should use three indexed fields if there are three or more fields. In addition, each event should have at least one indexed fields to allow easier filtering of logs.

Proof Of Concept

event AutomationSet(uint256 vaultId, AutomatedVault autoVault);

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/interfaces/IMIMOAutoAction.sol#L25

event ManagerSet(address manager, bool isManager);

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/interfaces/IMIMOManagedAction.sol#L22

event ManagementSet(uint256 vaultId, ManagedVault managedVault);

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/interfaces/IMIMOManagedAction.sol#L23

event Execute(address indexed target, bytes data, bytes response);

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/interfaces/IMIMOProxy.sol#L9

event DeployProxy(address indexed deployer, address indexed owner, address proxy);

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/interfaces/IMIMOProxyFactory.sol#L11

(6) Missing event for critical parameter change

Severity: Non-Critical

Proof Of Concept

function setPermission(

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L104

(7) Implementation contract may not be initialized

OpenZeppelin recommends that the initializer modifier be applied to constructors. Per OZs Post implementation contract should be initialized to avoid potential griefs or exploits. https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680/5

Severity: Non-Critical

Proof Of Concept

contract MIMOEmptyVault is MIMOSwap, MIMOFlashloan, IMIMOEmtpyVault

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMOEmptyVault.sol#L14

contract MIMOFlashloan is IMIMOFlashloan

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMOFlashloan.sol#L15

contract MIMOLeverage is MIMOFlashloan, MIMOSwap, IMIMOLeverage

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMOLeverage.sol#L14

contract MIMORebalance is MIMOFlashloan, MIMOSwap, IMIMORebalance

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMORebalance.sol#L14

contract MIMOSwap is IMIMOSwap

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMOSwap.sol#L16

contract MIMOVaultActions is IMIMOVaultActions

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMOVaultActions.sol#L16

contract MIMOAutoAction is IMIMOAutoAction

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoAction.sol#L9

contract MIMOAutoRebalance is MIMOAutoAction, MIMOFlashloan, IMIMOAutoRebalance

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoRebalance.sol#L23

contract MIMOManagedAction is IMIMOManagedAction

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/MIMOManagedAction.sol#L9

contract MIMOManagedRebalance is MIMOManagedAction, MIMOFlashloan, IMIMOManagedRebalance

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/MIMOManagedRebalance.sol#L16

contract MIMOProxy is IMIMOProxy, Initializable, BoringBatchable

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L12

contract MIMOProxyFactory is IMIMOProxyFactory

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxyFactory.sol#L12

contract MIMOProxyRegistry is IMIMOProxyRegistry

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxyRegistry.sol#L12

(8) Use a more recent version of Solidity

Severity: Non-Critical

Use a solidity version of at least 0.8.12 to get string.concat() instead of abi.encodePacked(<str>,<str>) Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions

Proof Of Concept

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMOEmptyVault.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMOFlashloan.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMOLeverage.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMORebalance.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMOSwap.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMOVaultActions.sol#L3

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoAction.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoRebalance.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/interfaces/IMIMOAutoAction.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/interfaces/IMIMOAutoRebalance.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/interfaces/IMIMOEmptyVault.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/interfaces/IMIMOFlashloan.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/interfaces/IMIMOLeverage.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/interfaces/IMIMOProxyAction.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/interfaces/IMIMORebalance.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/interfaces/IMIMOSwap.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/interfaces/IMIMOVaultActions.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/MIMOManagedAction.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/MIMOManagedRebalance.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/interfaces/IMIMOManagedAction.sol#L2

Found old version 0.8.10 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/interfaces/IMIMOManagedRebalance.sol#L2

Found old version 0.8.4 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L2

Found old version 0.8.4 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxyFactory.sol#L2

Found old version 0.8.4 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxyRegistry.sol#L2

Found old version 0.8.4 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/interfaces/IMIMOProxy.sol#L2

Found old version 0.8.4 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/interfaces/IMIMOProxyFactory.sol#L2

Found old version 0.8.4 of Solidity https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/interfaces/IMIMOProxyRegistry.sol#L2

Consider updating to a more recent solidity version.

(9) Redundant Cast

Severity: Non-Critical

The type of the variable is the same as the type to which the variable is being cast

Proof Of Concept

uint256 toVaultMcr = _a.config().collateralMinCollateralRatio(address(toCollateral));

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoRebalance.sol#L181

(1) Abi.encode() Is Less Efficient Than Abi.encodepacked()

Severity: Gas Optimizations

Proof Of Concept

bytes memory params = abi.encode(msg.sender, swapAmount, swapData);

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMOLeverage.sol#L54

bytes memory params = abi.encode(msg.sender, rbData, swapData);

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMORebalance.sol#L49

_takeFlashLoan(flData, abi.encode(vaultOwner, autoFee, rbData, swapData));

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoRebalance.sol#L73

_takeFlashLoan(flData, abi.encode(vaultsData.vaultOwner(rbData.vaultId), managerFee, rbData, swapData));

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/MIMOManagedRebalance.sol#L65

(2) <Array>.length Should Not Be Looked Up In Every Loop Of A For-loop

Severity: Gas Optimizations

The overheads outlined below are PER LOOP, excluding the first loop

storage arrays incur a Gwarmaccess (100 gas) memory arrays use MLOAD (3 gas) calldata arrays use CALLDATALOAD (3 gas)

Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

Proof Of Concept

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

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L132

(3) Empty Blocks Should Be Removed Or Emit Something

Severity: Gas Optimizations

The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting. If the contract is meant to be extended, the contract should be abstract and the function signatures be added without any default implementation. If the block is an empty if-statement block to avoid doing subsequent checks in the else-if/else conditions, the else-if/else conditions should be nested under the negation of the if-statement, because they involve different classes of checks, which may lead to the introduction of errors when the code is later modified (if(x){}else if(y){...}else{...} => if(!x){if(y){...}else{...}})

Proof Of Concept

) external virtual override returns (bool) {}

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/MIMOFlashloan.sol#L44

receive() external payable {}

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L38

(4) ++i Costs Less Gas Than i++, Especially When It’s Used In For-loops (--i/i-- Too)

Severity: Gas Optimizations

Saves 6 gas per loop

Proof Of Concept

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

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L132

For example, use ++i instead of i++

(5) It Costs More Gas To Initialize Variables To Zero Than To Let The Default Of Zero Be Applied

Severity: Gas Optimizations

Proof Of Concept

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

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L132

(6) Using Private Rather Than Public For Constants, Saves Gas

Severity: Gas Optimizations

If needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table

Proof Of Concept

uint256 public constant override VERSION = 1; https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxyFactory.sol#L19

Set variable to private.

(7) Help The Optimizer By Saving A Storage Variable’s Reference Instead Of Repeatedly Fetching It

Severity: Gas Optimizations

To help the optimizer, declare a storage type variable and use it instead of repeatedly fetching the reference in a map or an array. The effect can be quite significant. As an example, instead of repeatedly calling someMap[someIndex], save its reference like this: SomeStruct storage someStruct = someMap[someIndex] and use it.

Proof Of Concept

return _automatedVaults[vaultId];

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoAction.sol#L58

return _operationTracker[vaultId];

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoAction.sol#L65

AutomatedVault memory autoVault = _automatedVaults[vaultId];

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoRebalance.sol#L55

AutomatedVault memory autoVault = _automatedVaults[vaultId];

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoRebalance.sol#L55

return _getAmounts(_automatedVaults[vaultId], vaultState, toCollateral);

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoRebalance.sol#L153

if (_operationTracker[vaultId] > block.timestamp - 1 days) {

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/automated/MIMOAutoRebalance.sol#L244

return _managedVaults[vaultId];

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/MIMOManagedAction.sol#L71

return _operationTracker[vaultId];

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/MIMOManagedAction.sol#L78

return _managers[manager];

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/MIMOManagedAction.sol#L85

return _permissions[envoy][target][selector];

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L48

_permissions[envoy][target][selector] = permission;

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L113

result = _proxies[proxy];

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxyFactory.sol#L34

proxy = _currentProxies[owner];

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxyRegistry.sol#L34

IMIMOProxy currentProxy = _currentProxies[owner];

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxyRegistry.sol#L46

IMIMOProxy currentProxy = _currentProxies[owner];

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxyRegistry.sol#L46

(8) ++i/i++ Should Be Unchecked{++i}/unchecked{i++} When It Is Not Possible For Them To Overflow, As Is The Case When Used In For- And While-loops

Severity: Gas Optimizations

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas PER LOOP

Proof Of Concept

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

https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L132

(9) Using Bools For Storage Incurs Overhead

Severity: Gas Optimizations

Booleans are more expensive than uint256 or any type that takes up a full word because each write operation emits an extra SLOAD to first read the slot's contents, replace the bits taken up by the boolean, and then write back. This is the compiler's defense against contract upgrades and pointer aliasing, and it cannot be disabled.

Proof Of Concept

mapping(address => bool) internal _managers; https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/actions/managed/MIMOManagedAction.sol#L17

mapping(address => mapping(address => mapping(bytes4 => bool))) internal _permissions; https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L24

mapping(address => bool) internal _proxies; https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxyFactory.sol#L24

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter