Mimo DeFi contest - delfin454000's results

Lines of code

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/liquidityMining/v2/PARMinerV2.sol#L58 https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/liquidityMining/v2/PARMinerV2.sol#L125

Vulnerability details

Impact

When changing the allowance value from an existing non-zero value, certain tokens (e.g., USDT) must first be approved by zero (before approving the actual allowance). Otherwise the token will not work.

Proof of Concept

There are two instances of missing zero approval. The _par.approve function is called without setting the allowance to zero. Similarly for collateralToken.approve:

liquidityMining/v2/PARMinerV2.sol:58

liquidityMining/v2/PARMinerV2.sol:125

Tools Used

Manual analysis

Recommended Mitigation Steps

Set the allowance to zero before each of the approve() calls, as follows:

    _par.approve(address(_a.parallel().core()), 0);
    _par.approve(address(_a.parallel().core()), uint256(-1));

    collateralToken.approve(proxy, 0);
    collateralToken.approve(proxy, collateralToken.balanceOf(address(this)));

Typos

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/inception/AdminInceptionVault.sol#L118

    @dev This function cn only be called by the InceptionVaultsCore.

Change cn to can

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/liquidityMining/v2/PARMinerV2.sol#L160

    Reapplies the boost of the user, useful a whale's vMIMO has decreased but their boost is still the original value

Change useful a to useful if a

The same typo (duplicate the) occurs in both lines below: https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/liquidityMining/v2/interfaces/IGenericMinerV2.sol#L28

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/liquidityMining/v2/interfaces/IGenericMinerV2.sol#L32

  /// It emits with the user's address and the the value after the change.

Remove duplicate the

The same typo (an) occurs in both lines below: https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/oracles/BalancerV2LPOracle.sol#L76

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/oracles/GUniLPOracle.sol#L79

   * retrieved combined with an phase to ensure that round IDs get larger as

Change an to a

The same typo (treshold) occurs in all six lines below: https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/inception/AdminInceptionVault.sol#L94

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/inception/AdminInceptionVault.sol#L160

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

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

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

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

Change treshold to threshold

Issue: Should use != 0 instead of > 0 in a require statement if the variable is an unsigned integer (uint) Explanation: != 0 should be used where possible since > 0 costs more gas.

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

  function deposit(uint256 _amount) public override {
    require(_amount > 0, "IV100");
    _inceptionCollateral.safeTransferFrom(msg.sender, address(this), _amount);

    _addCollateralToVault(_amount);
  }

Change _amount > 0 to _amount != 0

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/oracles/GUniLPOracle.sol#L112

    require(rA > 0 || rB > 0, "C100");

Change rA > 0 || rB > 0 to rA != 0 || rB != 0

Issue: Use of '&&' within a require function Explanation: Dividing the require into separate require messages instead of using '&&' will save gas

The two lines below contain identical code: https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/liquidityMining/v2/GenericMinerV2.sol#L58

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

    require(boostConfig.a >= 1 && boostConfig.d > 0 && boostConfig.maxBoost >= 1, "LM004");

Change to:

    require(boostConfig.a >= 1, "LM004");
    require(boostConfig.d > 0, "LM004");
    require(boostConfig.maxBoost >= 1, "LM004");

The two lines below contain identical code: https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/liquidityMining/v2/GenericMinerV2.sol#L70

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/liquidityMining/v2/PARMinerV2.sol#L71

    require(newBoostConfig.a >= 1 && newBoostConfig.d > 0 && newBoostConfig.maxBoost >= 1, "LM004");

Change to:

    require(newBoostConfig.a >= 1, "LM004");
    require(newBoostConfig.d > 0, "LM004");
    require(newBoostConfig.maxBoost >= 1, "LM004");

The two lines below contain identical code: https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/liquidityMining/v2/GenericMinerV2.sol#L331

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/liquidityMining/v2/PARMinerV2.sol#L426

    require(multiplier >= 1e18 && multiplier <= _boostConfig.maxBoost, "LM103");

Change to:

    require(multiplier >= 1e18, "LM103");
    require(multiplier <= _boostConfig.maxBoost, "LM103");

Issue: Variables should not be initialized to their default values Explanation: Initializing uint256 variables to their default value of zero is unnecessary and costs gas.

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

    uint256 insuranceAmount = 0;

Change uint256 insuranceAmount = 0; to uint256 insuranceAmount;