SuperVault depositToVault use unchecked transferFrom with an arbitrary ERC20 asset

[code423n4]

Lines of code

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

Vulnerability details

Whenever an asset ERC20 do not revert on transfer failure, the such transfer failure will produce an accounting discrepancy as the system will go on with operations as if transfer was successful.

The end result depends on the current state of the contract.

As funds are to be pulled in and such a failure without revert happened, if the contract balance has enough funds the user will be credited with someone else's funds, i.e. double counting happens.

This can end up with contract insolvency for the users who had properly deposited the funds.

Placing severity to be high as once such an asset is supported by the core, the depositToVault can be run by a malicious user repeatedly to bloat his balance and then empty contract holdings with a subsequent withdraw.

Proof of Concept

depositToVault has no access controls and use unchecked transferFrom for an arbitrary token:

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

token.transferFrom(msg.sender, address(this), amount);

Some ERC20 do not revert on transfer failure:

https://github.com/d-xo/weird-erc20#no-revert-on-failure

This gives an attacker a way to credit himself without actual fund transfer.

Recommended Mitigation Steps

Require transfer success as it is done elsewhere in the contract or employ OpenZeppelin's SafeERC20

[m19]

Duplicate of #124

[gzeoneth]

Since the transfer should be one that accepted by the core protocol and according to https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/config/deployment.ts#L65 it seems that all asset will revert upon failed transfer, this listing of new asset can be guarded by owner, downgrading to Low / QA.

[gzeoneth]

Considered with #165