Mimo DeFi contest - defsec's results

Judge has assessed an item in Issue #124 as Medium risk. The relevant finding follows:

C4-006 : # The Contract Should Approve(0) first Impact Some tokens (like USDT L199) do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved.

IERC20(token).approve(address(operator), 0); IERC20(token).approve(address(operator), amount); Proof of Concept Navigate to the following contracts. 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::97 => asset.approve(address(lendingPool), flashloanRepayAmount); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::129 => IERC20(asset).transferFrom(msg.sender, address(this), depositAmount); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::149 => IERC20(toCollateral).approve(address(a.core()), depositAmount); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::199 => par.approve(address(a.core()), par.balanceOf(address(this))); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::233 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this)))); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::237 => collateral.transfer(msg.sender, collateral.balanceOf(address(this))); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::247 => require(asset.transfer(msg.sender, amount)); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::255 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this)))); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::264 => require(token.transfer(msg.sender, token.balanceOf(address(this)))); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::273 => token.approve(address(a.core()), amount); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::274 => token.transferFrom(msg.sender, address(this), amount); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::289 => token.approve(address(a.core()), depositAmount); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::290 => token.transferFrom(msg.sender, address(this), depositAmount); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::292 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this)))); //par 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::313 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this)))); //par 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::326 => token.approve(address(a.core()), 2**256 - 1); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::345 => token.approve(proxy, amount); 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::370 => require(ga.mimo().transfer(msg.sender, ga.mimo().balanceOf(address(this))));

2022-04-mimo-main/core/contracts/liquidityMining/v2/GenericMinerV2.sol::220 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100"); 2022-04-mimo-main/core/contracts/liquidityMining/v2/GenericMinerV2.sol::223 => require(_par.transfer(_user, pendingPAR), "LM100"); 2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::58 => _par.approve(address(_a.parallel().core()), uint256(-1)); 2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::125 => collateralToken.approve(proxy, collateralToken.balanceOf(address(this))); 2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::320 => require(_par.transfer(_user, pendingPAR), "LM100"); 2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::323 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100"); 2022-04-mimo-main/core/contracts/liquidityMining/v2/VotingMinerV2.sol::44 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100"); 2022-04-mimo-main/core/contracts/liquidityMining/v2/VotingMinerV2.sol::47 => require(_par.transfer(_user, pendingPAR), "LM100");

When trying to re-approve an already approved token, all transactions revert and the protocol cannot be used. Tools Used None

Recommended Mitigation Steps Approve with a zero amount first before setting the actual amount.

Judge has assessed an item in Issue #124 as Medium risk. The relevant finding follows:

C4-007 : # USE SAFEERC20.SAFEAPPROVE Impact This is probably an oversight since SafeERC20 was imported and safeTransfer() was used for ERC20 token transfers. Nevertheless, note that approve() will fail for certain token implementations that do not return a boolean value (). Hence it is recommend to use safeApprove().

Proof of Concept 2022-04-mimo-main/core/contracts/liquidityMining/GenericMiner.sol::47 => require(a.mimo().transfer(_user, pending)); 2022-04-mimo-main/core/contracts/liquidityMining/GenericMiner.sol::102 => require(a.mimo().transfer(user, pending)); 2022-04-mimo-main/core/contracts/liquidityMining/GenericMiner.sol::129 => require(a.mimo().transfer(user, pending)); 2022-04-mimo-main/core/contracts/liquidityMining/MIMOBuyBack.sol::36 => PAR.approve(address(balancer), 2256 - 1); 2022-04-mimo-main/core/contracts/liquidityMining/MIMOBuyBack.sol::51 => require(MIMO.transfer(destination, MIMO.balanceOf(address(this)))); 2022-04-mimo-main/core/contracts/liquidityMining/MIMOBuybackUniswapV2.sol::35 => PAR.approve(address(router), 2256 - 1); 2022-04-mimo-main/core/contracts/liquidityMining/MIMOBuybackUniswapV2.sol::50 => require(MIMO.transfer(destination, MIMO.balanceOf(address(this)))); 2022-04-mimo-main/core/contracts/liquidityMining/MinerPayer.sol::91 => a.mimo().transfer(_payee, payment); 2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::77 => require(a.mimo().transfer(_user, pending)); 2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::91 => require(par.transfer(_user, pending)); 2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::184 => require(a.mimo().transfer(user, pending)); 2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::187 => require(par.transfer(user, pendingPAR)); 2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::232 => require(a.mimo().transfer(user, pending)); 2022-04-mimo-main/core/contracts/liquidityMining/PolygonDistributor.sol::47 => a.mimo().approve(erc20Predicate, payment); 2022-04-mimo-main/core/contracts/liquidityMining/VotingMiner.sol::33 => require(a.mimo().transfer(_user, pending)); 2022-04-mimo-main/core/contracts/liquidityMining/v2/GenericMinerV2.sol::220 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100"); 2022-04-mimo-main/core/contracts/liquidityMining/v2/GenericMinerV2.sol::223 => require(_par.transfer(_user, pendingPAR), "LM100"); 2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::58 => _par.approve(address(_a.parallel().core()), uint256(-1)); 2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::125 => collateralToken.approve(proxy, collateralToken.balanceOf(address(this))); 2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::320 => require(_par.transfer(_user, pendingPAR), "LM100"); 2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::323 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100"); 2022-04-mimo-main/core/contracts/liquidityMining/v2/VotingMinerV2.sol::44 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100"); 2022-04-mimo-main/core/contracts/liquidityMining/v2/VotingMinerV2.sol::47 => require(_par.transfer(_user, pendingPAR), "LM100");

Tools Used Manual Code Review

Recommended Mitigation Steps Update to _token.safeApprove(spender, type(uint256).max)

C4-001 :Use safeTransfer/safeTransferFrom consistently instead of transfer/transferFrom

Impact - LOW

Impact

It is good to add a require() statement that checks the return value of token transfers or to use something like OpenZeppelin’s safeTransfer/safeTransferFrom unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.

Reference: This similar medium-severity finding from Consensys Diligence Audit of Fei Protocol: https://consensys.net/diligence/audits/2021/01/fei-protocol/#unchecked-return-value-for-iweth-transfer-call

Proof of Concept

1. Navigate to the following contract.

2. transfer/transferFrom functions are used instead of safe transfer/transferFrom on the following contracts.

  2022-04-mimo-main/core/contracts/liquidityMining/MinerPayer.sol::91 => a.mimo().transfer(_payee, payment);
  2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::77 => require(a.mimo().transfer(_user, pending));
  2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::91 => require(par.transfer(_user, pending));
  2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::184 => require(a.mimo().transfer(user, pending));
  2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::187 => require(par.transfer(user, pendingPAR));
  2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::232 => require(a.mimo().transfer(user, pending));
  2022-04-mimo-main/core/contracts/liquidityMining/PolygonDistributor.sol::47 => a.mimo().approve(erc20Predicate, payment);
  2022-04-mimo-main/core/contracts/liquidityMining/VotingMiner.sol::33 => require(a.mimo().transfer(_user, pending));
  2022-04-mimo-main/core/contracts/liquidityMining/v2/GenericMinerV2.sol::220 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/GenericMinerV2.sol::223 => require(_par.transfer(_user, pendingPAR), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::58 => _par.approve(address(_a.parallel().core()), uint256(-1));
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::125 => collateralToken.approve(proxy, collateralToken.balanceOf(address(this)));
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::320 => require(_par.transfer(_user, pendingPAR), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::323 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/VotingMinerV2.sol::44 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/VotingMinerV2.sol::47 => require(_par.transfer(_user, pendingPAR), "LM100");

Tools Used

Code Review

Recommended Mitigation Steps

Consider using safeTransfer/safeTransferFrom or require() consistently.

C4-002 : Use of Block.timestamp

Impact - Non-Critical

Block timestamps have historically been used for a variety of applications, such as entropy for random numbers (see the Entropy Illusion for further details), locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.

Proof of Concept

1. Navigate to the following contract.

https://github.com/code-423n4/2022-04-mimo/blob/main/core/contracts/inception/InceptionVaultsCore.sol#L51

Tools Used

Manual Code Review

Recommended Mitigation Steps

Block timestamps should not be used for entropy or generating random numbers—i.e., they should not be the deciding factor (either directly or through some derivation) for winning a game or changing an important state.

Time-sensitive logic is sometimes required; e.g., for unlocking contracts (time-locking), completing an ICO after a few weeks, or enforcing expiry dates. It is sometimes recommended to use block.number and an average block time to estimate times; with a 10 second block time, 1 week equates to approximately, 60480 blocks. Thus, specifying a block number at which to change a contract state can be more secure, as miners are unable to easily manipulate the block number.

C4-003 : # Missing Re-entrancy Guard

Impact - LOW

The re-entrancy guard is missing on the some of the functions. The external interactions can cause to the re-entrancy vulnerability.

Proof of Concept

1. Navigate to the following contract.

https://github.com/code-423n4/2022-04-mimo/blob/main/core/contracts/liquidityMining/PARMiner.sol#L52

https://github.com/code-423n4/2022-04-mimo/blob/main/core/contracts/liquidityMining/PARMiner.sol#L61

Tools Used

Code Review

Recommended Mitigation Steps

Follow the check effect interaction pattern or put re-entrancy guard.

C4-004 : Incompatibility With Rebasing/Deflationary/Inflationary tokens

Impact - LOW

PrePo protocol do not appear to support rebasing/deflationary/inflationary tokens whose balance changes during transfers or over time. The necessary checks include at least verifying the amount of tokens transferred to contracts before and after the actual transfer to infer any fees/interest.

Proof of Concept

1. Navigate to the following contract.

 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::97 => asset.approve(address(lendingPool), flashloanRepayAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::129 => IERC20(asset).transferFrom(msg.sender, address(this), depositAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::149 => IERC20(toCollateral).approve(address(a.core()), depositAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::199 => par.approve(address(a.core()), par.balanceOf(address(this)));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::233 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this))));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::237 => collateral.transfer(msg.sender, collateral.balanceOf(address(this)));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::247 => require(asset.transfer(msg.sender, amount));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::255 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this))));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::264 => require(token.transfer(msg.sender, token.balanceOf(address(this))));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::273 => token.approve(address(a.core()), amount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::274 => token.transferFrom(msg.sender, address(this), amount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::289 => token.approve(address(a.core()), depositAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::290 => token.transferFrom(msg.sender, address(this), depositAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::292 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this)))); //par
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::313 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this)))); //par
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::326 => token.approve(address(a.core()), 2**256 - 1);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::345 => token.approve(proxy, amount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::370 => require(ga.mimo().transfer(msg.sender, ga.mimo().balanceOf(address(this))));

Tools Used

Manual Code Review

Recommended Mitigation Steps

• Ensure that to check previous balance/after balance equals to amount for any rebasing/inflation/deflation
• Add support in contracts for such tokens before accepting user-supplied tokens
• Consider supporting deflationary / rebasing / etc tokens by extra checking the balances before/after or strictly inform your users not to use such tokens if they don't want to lose them.

C4-005 : # Pragma Version

Impact

In the contracts, floating pragmas should not be used. Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

## Proof of Concept

https://swcregistry.io/docs/SWC-103

All Contracts

Tools Used

Manual code review

Recommended Mitigation Steps

Lock the pragma version: delete pragma solidity 0.8.10 in favor of pragma solidity 0.8.10

C4-006 : # The Contract Should Approve(0) first

Impact

Some tokens (like USDT L199) do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved.

IERC20(token).approve(address(operator), 0);
IERC20(token).approve(address(operator), amount);

Proof of Concept

1. Navigate to the following contracts.

 2022-04-mimo-main/supervaults/contracts/SuperVault.sol::97 => asset.approve(address(lendingPool), flashloanRepayAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::129 => IERC20(asset).transferFrom(msg.sender, address(this), depositAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::149 => IERC20(toCollateral).approve(address(a.core()), depositAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::199 => par.approve(address(a.core()), par.balanceOf(address(this)));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::233 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this))));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::237 => collateral.transfer(msg.sender, collateral.balanceOf(address(this)));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::247 => require(asset.transfer(msg.sender, amount));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::255 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this))));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::264 => require(token.transfer(msg.sender, token.balanceOf(address(this))));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::273 => token.approve(address(a.core()), amount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::274 => token.transferFrom(msg.sender, address(this), amount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::289 => token.approve(address(a.core()), depositAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::290 => token.transferFrom(msg.sender, address(this), depositAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::292 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this)))); //par
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::313 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this)))); //par
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::326 => token.approve(address(a.core()), 2**256 - 1);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::345 => token.approve(proxy, amount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::370 => require(ga.mimo().transfer(msg.sender, ga.mimo().balanceOf(address(this))));

  2022-04-mimo-main/core/contracts/liquidityMining/v2/GenericMinerV2.sol::220 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/GenericMinerV2.sol::223 => require(_par.transfer(_user, pendingPAR), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::58 => _par.approve(address(_a.parallel().core()), uint256(-1));
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::125 => collateralToken.approve(proxy, collateralToken.balanceOf(address(this)));
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::320 => require(_par.transfer(_user, pendingPAR), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::323 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/VotingMinerV2.sol::44 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/VotingMinerV2.sol::47 => require(_par.transfer(_user, pendingPAR), "LM100");

2. When trying to re-approve an already approved token, all transactions revert and the protocol cannot be used.

Tools Used

None

Recommended Mitigation Steps

Approve with a zero amount first before setting the actual amount.

C4-007 : # USE SAFEERC20.SAFEAPPROVE

Impact

This is probably an oversight since SafeERC20 was imported and safeTransfer() was used for ERC20 token transfers. Nevertheless, note that approve() will fail for certain token implementations that do not return a boolean value (). Hence it is recommend to use safeApprove().

Proof of Concept

  2022-04-mimo-main/core/contracts/liquidityMining/GenericMiner.sol::47 => require(a.mimo().transfer(_user, pending));
  2022-04-mimo-main/core/contracts/liquidityMining/GenericMiner.sol::102 => require(a.mimo().transfer(user, pending));
  2022-04-mimo-main/core/contracts/liquidityMining/GenericMiner.sol::129 => require(a.mimo().transfer(user, pending));
  2022-04-mimo-main/core/contracts/liquidityMining/MIMOBuyBack.sol::36 => PAR.approve(address(balancer), 2**256 - 1);
  2022-04-mimo-main/core/contracts/liquidityMining/MIMOBuyBack.sol::51 => require(MIMO.transfer(destination, MIMO.balanceOf(address(this))));
  2022-04-mimo-main/core/contracts/liquidityMining/MIMOBuybackUniswapV2.sol::35 => PAR.approve(address(router), 2**256 - 1);
  2022-04-mimo-main/core/contracts/liquidityMining/MIMOBuybackUniswapV2.sol::50 => require(MIMO.transfer(destination, MIMO.balanceOf(address(this))));
  2022-04-mimo-main/core/contracts/liquidityMining/MinerPayer.sol::91 => a.mimo().transfer(_payee, payment);
  2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::77 => require(a.mimo().transfer(_user, pending));
  2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::91 => require(par.transfer(_user, pending));
  2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::184 => require(a.mimo().transfer(user, pending));
  2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::187 => require(par.transfer(user, pendingPAR));
  2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::232 => require(a.mimo().transfer(user, pending));
  2022-04-mimo-main/core/contracts/liquidityMining/PolygonDistributor.sol::47 => a.mimo().approve(erc20Predicate, payment);
  2022-04-mimo-main/core/contracts/liquidityMining/VotingMiner.sol::33 => require(a.mimo().transfer(_user, pending));
  2022-04-mimo-main/core/contracts/liquidityMining/v2/GenericMinerV2.sol::220 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/GenericMinerV2.sol::223 => require(_par.transfer(_user, pendingPAR), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::58 => _par.approve(address(_a.parallel().core()), uint256(-1));
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::125 => collateralToken.approve(proxy, collateralToken.balanceOf(address(this)));
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::320 => require(_par.transfer(_user, pendingPAR), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::323 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/VotingMinerV2.sol::44 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/VotingMinerV2.sol::47 => require(_par.transfer(_user, pendingPAR), "LM100");

Tools Used

Manual Code Review

Recommended Mitigation Steps

Update to _token.safeApprove(spender, type(uint256).max)

C4-008 : # USE OF DEPRECATED _SETUPROLE FUNCTION

Impact

The contract SuperVault.sol make use of the deprecated function _setupRole from the AccessControl contract. As per the AccessControl.sol contract documentation, this function is deprecated: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/AccessControl.sol#L183

Using deprecated functions may eventually produce an unwanted behaviour, for example, if OpenZeppelin decides to remove or update the function.

Proof of Concept

  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::66 => _setupRole(DEFAULT_ADMIN_ROLE, _owner);

Tools Used

Manual testing

Recommended Mitigation Steps

It is recommended to use the _grantRole function instead.

C4-009 : Front-runnable Initializers

Impact - LOW

All contract initializers were missing access controls, allowing any user to initialize the contract. By front-running the contract deployers to initialize the contract, the incorrect parameters may be supplied, leaving the contract needing to be redeployed.

Proof of Concept

1. Navigate to the following contracts.

https://github.com/code-423n4/2022-04-mimo/blob/main/core/contracts/inception/InceptionVaultsCore.sol#L40

https://github.com/code-423n4/2022-04-mimo/blob/main/supervaults/contracts/SuperVault.sol#L49

2. initialize functions does not have access control. They are vulnerable to front-running.

Tools Used

Manual Code Review

Recommended Mitigation Steps

While the code that can be run in contract constructors is limited, setting the owner in the contract's constructor to the msg.sender and adding the onlyOwner modifier to all initializers would be a sufficient level of access control.

C4-010 : Missing events for only functions that change critical parameters

Impact - Non critical

The functions that change critical parameters should emit events. Events allow capturing the changed parameters so that off-chain tools/interfaces can register such changes with timelocks that allow users to evaluate them and consider if they would like to engage/exit based on how they perceive the changes as affecting the trustworthiness of the protocol or profitability of the implemented financial services. The alternative of directly querying on-chain contract state for such changes is not considered practical for most users/usages.

Missing events and timelocks do not promote transparency and if such changes immediately affect users’ perception of fairness or trustworthiness, they could exit the protocol causing a reduction in liquidity which could negatively impact protocol TVL and reputation.

Proof of Concept

1. Navigate to the following contract.

https://github.com/code-423n4/2022-04-mimo/blob/main/supervaults/contracts/SuperVault.sol#L264

https://github.com/code-423n4/2022-04-mimo/blob/main/supervaults/contracts/SuperVault.sol#L253

https://github.com/code-423n4/2022-04-mimo/blob/main/supervaults/contracts/SuperVault.sol#L244

See similar High-severity H03 finding OpenZeppelin’s Audit of Audius (https://blog.openzeppelin.com/audius-contracts-audit/#high) and Medium-severity M01 finding OpenZeppelin’s Audit of UMA Phase 4 (https://blog.openzeppelin.com/uma-audit-phase-4/)

Tools Used

None

Recommended Mitigation Steps

Add events to all functions that change critical parameters.

C4-001: Revert String Size Optimization

Impact

Shortening revert strings to fit in 32 bytes will decrease deploy time gas and will decrease runtime gas when the revert condition has been met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Proof of Concept

Revert strings > 32 bytes are here:

 2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::163 => require(value > 0, "STAKE_MUST_BE_GREATER_THAN_ZERO"); //TODO cleanup error message
  2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::183 => if (pending > 0) {
  2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::186 => if (pendingPAR > 0) {
  2022-04-mimo-main/core/contracts/liquidityMining/PARMiner.sol::200 => require(value > 0, "STAKE_MUST_BE_GREATER_THAN_ZERO"); //TODO cleanup error message

Tools Used

Manual Review

Recommended Mitigation Steps

Shorten the revert strings to fit in 32 bytes. That will affect gas optimization.

C4-002 : Free gas savings for using solidity 0.8.10+

Impact

Using newer compiler versions and the optimizer gives gas optimizations and additional safety checks are available for free.

Proof of Concept

All Contracts

Solidity 0.8.10 has a useful change which reduced gas costs of external calls which expect a return value: https://blog.soliditylang.org/2021/11/09/solidity-0.8.10-release-announcement/

Code Generator: Skip existence check for external contract if return data is expected. In this case, the ABI decoder will revert if the contract does not exist

All Contracts

Tools Used

None

Recommended Mitigation Steps

Consider to upgrade pragma to at least 0.8.10.

C4-003: > 0 can be replaced with != 0 for gas optimization

Impact

!= 0 is a cheaper operation compared to > 0, when dealing with uint.

Proof of Concept

1. Navigate to the following contracts.

   2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::52 => require(boostConfig.a >= 1 && boostConfig.d > 0 && boostConfig.maxBoost >= 1, "LM004");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::71 => require(newBoostConfig.a >= 1 && newBoostConfig.d > 0 && newBoostConfig.maxBoost >= 1, "LM004");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::254 => require(_value > 0, "LM101");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::267 => if (pendingPAR > 0) {
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::284 => require(_value > 0, "LM101");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::314 => if (_userInfo.stakeWithBoost > 0) {
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::319 => if (pendingPAR > 0 && !_restakePAR) {
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::322 => if (pendingMIMO > 0) {
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::333 => if (_userInfo.stakeWithBoost > 0) {
  2022-04-mimo-main/core/contracts/liquidityMining/v2/VotingMinerV2.sol::31 => if (_userInfo.stakeWithBoost > 0) {
  2022-04-mimo-main/core/contracts/liquidityMining/v2/VotingMinerV2.sol::43 => if (pendingMIMO > 0) {
  2022-04-mimo-main/core/contracts/liquidityMining/v2/VotingMinerV2.sol::46 => if (pendingPAR > 0) {
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::369 => if (ga.mimo().balanceOf(address(this)) > 0) {

Tools Used

Code Review

Recommended Mitigation Steps

Use "!=0" instead of ">0" for the gas optimization.

C4-004 : Using operator && used more gas

Impact

Using double require instead of operator && can save more gas.

Proof of Concept

1. Navigate to the following contracts.

2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::52 => require(boostConfig.a >= 1 && boostConfig.d > 0 && boostConfig.maxBoost >= 1, "LM004");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::71 => require(newBoostConfig.a >= 1 && newBoostConfig.d > 0 && newBoostConfig.maxBoost >= 1, "LM004");

Tools Used

Code Review

Recommended Mitigation Steps

Example

using &&:

function check(uint x)public view{
    require(x == 0 && x < 1 );
}
// gas cost 21630

using double require:

    require(x == 0 );
    require( x < 1);
    }
}
// gas cost 21622

C4-005 : There is no need to assign default values to variables

Impact - Gas Optimization

When a variable is declared solidity assigns the default value. In case the contract assigns the value again, it costs extra gas.

Example: uint x = 0 costs more gas than uint x without having any different functionality.

Proof of Concept

  2022-04-mimo-main/core/contracts/inception/InceptionVaultsCore.sol::218 => uint256 insuranceAmount = 0;

Tools Used

Code Review

Recommended Mitigation Steps

uint x = 0 costs more gas than uint x without having any different functionality.

C4-006 : Check if amount > 0 before token transfer can save gas

Impact

Since _amount can be 0. Checking if (_amount != 0) before the transfer can potentially save an external call and the unnecessary gas cost of a 0 token transfer.

Proof of Concept

  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::58 => _par.approve(address(_a.parallel().core()), uint256(-1));
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::125 => collateralToken.approve(proxy, collateralToken.balanceOf(address(this)));
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::320 => require(_par.transfer(_user, pendingPAR), "LM100");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::323 => require(_a.mimo().transfer(_user, pendingMIMO), "LM100");
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::97 => asset.approve(address(lendingPool), flashloanRepayAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::129 => IERC20(asset).transferFrom(msg.sender, address(this), depositAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::149 => IERC20(toCollateral).approve(address(a.core()), depositAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::199 => par.approve(address(a.core()), par.balanceOf(address(this)));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::233 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this))));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::237 => collateral.transfer(msg.sender, collateral.balanceOf(address(this)));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::247 => require(asset.transfer(msg.sender, amount));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::255 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this))));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::264 => require(token.transfer(msg.sender, token.balanceOf(address(this))));
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::273 => token.approve(address(a.core()), amount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::274 => token.transferFrom(msg.sender, address(this), amount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::289 => token.approve(address(a.core()), depositAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::290 => token.transferFrom(msg.sender, address(this), depositAmount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::292 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this)))); //par
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::313 => require(IERC20(a.stablex()).transfer(msg.sender, IERC20(a.stablex()).balanceOf(address(this)))); //par
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::326 => token.approve(address(a.core()), 2**256 - 1);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::345 => token.approve(proxy, amount);
  2022-04-mimo-main/supervaults/contracts/SuperVault.sol::370 => require(ga.mimo().transfer(msg.sender, ga.mimo().balanceOf(address(this))));

All Contracts

Tools Used

None

Recommended Mitigation Steps

Consider checking amount != 0.

C4-007: Use of constant keccak variables results in extra hashing (and so gas).

Impact

That would Increase gas costs on all privileged operations.

Proof of Concept

The following role variables are marked as constant.

 2022-04-mimo-main/core/contracts/liquidityMining/MIMOBuyBack.sol::11 => bytes32 public constant KEEPER_ROLE = keccak256("KEEPER_ROLE");
  2022-04-mimo-main/core/contracts/liquidityMining/MIMOBuybackUniswapV2.sol::11 => bytes32 public constant KEEPER_ROLE = keccak256("KEEPER_ROLE");
  2022-04-mimo-main/core/contracts/liquidityMining/MinerPayer.sol::20 => bytes32 public constant KEEPER_ROLE = keccak256("KEEPER_ROLE");

This results in the keccak operation being performed whenever the variable is used, increasing gas costs relative to just storing the output hash. Changing to immutable will only perform hashing on contract deployment which will save gas.

See: ethereum/solidity#9232 (https://github.com/ethereum/solidity/issues/9232#issuecomment-646131646)

Tools Used

Code Review

Recommended Mitigation Steps

Consider to change the variable to be immutable rather than constant.

C4-008 : Non-strict inequalities are cheaper than strict ones

Impact

Strict inequalities add a check of non equality which costs around 3 gas.

Proof of Concept

  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::52 => require(boostConfig.a >= 1 && boostConfig.d > 0 && boostConfig.maxBoost >= 1, "LM004");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::71 => require(newBoostConfig.a >= 1 && newBoostConfig.d > 0 && newBoostConfig.maxBoost >= 1, "LM004");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::254 => require(_value > 0, "LM101");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::267 => if (pendingPAR > 0) {
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::284 => require(_value > 0, "LM101");
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::314 => if (_userInfo.stakeWithBoost > 0) {
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::319 => if (pendingPAR > 0 && !_restakePAR) {
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::322 => if (pendingMIMO > 0) {
  2022-04-mimo-main/core/contracts/liquidityMining/v2/PARMinerV2.sol::333 => if (_userInfo.stakeWithBoost > 0) {

Tools Used

Code Review

Recommended Mitigation Steps

Use >= or <= instead of > and < when possible.