Consistently check account balance before and after transfers for Fee-On-Transfer discrepancies

[code423n4]

Lines of code

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/inception/AdminInceptionVault.sol#L81
https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/inception/AdminInceptionVault.sol#L151
https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/liquidityMining/v2/DemandMinerV2.sol#L67
https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/supervaults/contracts/SuperVault.sol#L129
https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/supervaults/contracts/SuperVault.sol#L274
https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/supervaults/contracts/SuperVault.sol#L290

Vulnerability details

Impact

Wrong amount allowed / deposited / accounted

Proof of Concept

core/contracts/inception/AdminInceptionVault.sol:
   81:     asset.safeTransferFrom(msg.sender, address(this), _depositAmount);
  151:     asset.safeTransferFrom(msg.sender, address(this), _amount);

core/contracts/liquidityMining/v2/DemandMinerV2.sol:
  67:     _token.safeTransferFrom(msg.sender, address(this), amount);

supervaults/contracts/SuperVault.sol:
  129:     IERC20(asset).transferFrom(msg.sender, address(this), depositAmount);
  274:     token.transferFrom(msg.sender, address(this), amount);
  290:     token.transferFrom(msg.sender, address(this), depositAmount);

Arbitrary ERC20 tokens can be passed
With a transfer, the received amount should be calculated every time to take into consideration a possible fee-on-transfer or deflation.
Also, it's a good practice for the future of the solution.

Recommended Mitigation Steps

Use the balance before and after the transfer to calculate the received amount instead of assuming that it would be equal to the amount passed as a parameter.

[m19]

While technically true, we do not think this is a medium risk issue:

• AdminInceptionVault calls to VaultsCore, which will error if the funds are not available
• SuperVault calls to VaultsCore, which will error if the funds are not available
• Any LP tokens with "fee-on-transfer" will be highly unlikely for the DemandMiner

[gzeoneth]

I agree with the sponsor there doesn't seems to be a pathway for a fee-on-transfer token to not revert in every instance of transfer except for the one in DemandMinerV2, which in the doc stated:

Liquidity Mining V2
The next version of reward contracts for the users of the protocol. In the first iteration of liquidity mining contracts, users earned MIMO tokens on their debt and staked LP and PAR tokens.

where the LP and PAR tokens are not fee-on-transfer. Downgrading this to Low / QA.

[gzeoneth]

Considered with #153