Mimo August 2022 contest - brgltd's results

[L-01] Missing constructor and modifier for contracts using Initializable

Impact

OpenZeppelin recommends adding an empty constructor with the initializer modifier in order to avoid exploits.

File: contracts/proxy/MIMOProxy.sol#L12
12: contract MIMOProxy is IMIMOProxy, Initializable, BoringBatchable {

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

Recommended Mitigations Steps

Add an empty constructor with the initializer modifier.

OpenZeppelin's main concern is for contracts using UUPSUpgradeable. However, I would still recommend adding it for all contracts using Initializable as an extra layer of security to avoid having to send a transaction to invoke initialize() after the contract is deployed to ensure no one else can initialize.

constructor() initializer {}

[NC-01] Remove floating pragma

Locking the pragma will make sure that the contract does not get deployed using outdated compiler versions.

File: contracts/proxy/MIMOProxy.sol
2: pragma solidity >=0.8.4;

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

File: contracts/proxy/MIMOProxyFactory.sol
2: pragma solidity >=0.8.4;

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

File: contracts/proxy/interfaces/IMIMOProxyFactory.sol
2: pragma solidity ^0.8.4;

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

File: contracts/proxy/interfaces/IMIMOProxyRegistry.sol
2: pragma solidity ^0.8.4;

https://github.com/code-423n4/2022-08-mimo/blob/main/contracts/proxy/interfaces/IMIMOProxyRegistry.sol

[NC-02] Empty receive function

The function should revert if the intention is not to receive ether, or add a functionality for the received ether.

File: contracts/proxy/MIMOProxy.sol
38: receive() external payable {}

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

[G-01] Use custom errors rather than revert()/require() strings to save gas

File: contracts/libraries/Errors.sol
11: string public constant INVALID_AGGREGATOR = "1";

https://github.com/code-423n4/2022-08-mimo/blob/main/contracts/libraries/Errors.sol

File: contracts/actions/MIMOSwap.sol
47: require(proxy != address(0), Errors.INVALID_AGGREGATOR);
48: require(router != address(0), Errors.INVALID_AGGREGATOR);

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

[G-02] Use calldata instead of memory for read-only function arguments saves gas

Modifying FlashLoanData and params from memory to calldata will save gas, even if the function is internal.

File: contracts/actions/MIMOFlashloan.sol
51: function _takeFlashLoan(FlashLoanData memory flData, bytes memory params) internal {

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

[G-03] Using private rather than public for constants will save gas

If needed, the values can be inspected on the souce code.

File: contracts/proxy/MIMOProxyFactory.sol
uint256 public constant override VERSION = 1;

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