Partial Migration Failure Risk

Summary

The migrateDeposits() function in the ZivoeITO.sol contract is vulnerable to a partial migration failure due to the way it handles the transfer of multiple stablecoins. If one of the stablecoins fails to transfer, the entire migration process will be blocked, leaving the other stablecoins stuck in the contract.

Vulnerability Detail

In ZivoeITO.sol:331, the contract loops through an array of stablecoin addresses and transfers (using safeTransfer) the balance of each stablecoin to the DAO address. However, if one of the transfers fails, the transaction will revert, and the remaining stablecoins will be stuck in the contract.

Impact

If one of the stablecoins fails to transfer during the migration process, the entire migration will be blocked, and the remaining stablecoins will be locked in the contract. This could lead to a loss of access to those funds and potential issues for the DAO and other users given that there is no alternate process to withdraw the tokens individually.

Code Snippet

emit DepositsMigrated(
    IERC20(stables[0]).balanceOf(address(this)),
    IERC20(stables[1]).balanceOf(address(this)),
    IERC20(stables[2]).balanceOf(address(this)),
    IERC20(stables[3]).balanceOf(address(this))
);
for (uint256 i = 0; i < stables.length; i++) {
    // @audit-issue M-03 one failure in one of the stables makes impossible to migrate the other assets. As simple as one of the contracts being blacklisted and it will get all the other stablecoins stuck in the contract.
    IERC20(stables[i]).safeTransfer(IZivoeGlobals_ITO(GBL).DAO(), IERC20(stables[i]).balanceOf(address(this)));
}
ITO_IZivoeYDL(IZivoeGlobals_ITO(GBL).YDL()).unlock();
ITO_IZivoeTranches(IZivoeGlobals_ITO(GBL).ZVT()).unlock();

Tool used

Manual Review

Recommendation

To mitigate this issue, the contract should implement a more robust migration process that can handle individual stablecoin transfer failures without blocking the entire migration. One possible solution is to use a try-catch block to handle individual transfers and continue with the migration even if one of the transfers fails.

Here's an example of how the migrateDeposits() function can be updated:

emit DepositsMigrated(
    IERC20(stables[0]).balanceOf(address(this)),
    IERC20(stables[1]).balanceOf(address(this)),
    IERC20(stables[2]).balanceOf(address(this)),
    IERC20(stables[3]).balanceOf(address(this))
);
for (uint256 i = 0; i < stables.length; i++) {
    try IERC20(stables[i]).safeTransfer(IZivoeGlobals_ITO(GBL).DAO(), IERC20(stables[i]).balanceOf(address(this))) {
        // Transfer successful, continue with the migration
    } catch {
        // Transfer failed, log the issue and continue with the migration
        emit MigrationFailure(stables[i], IERC20(stables[i]).balanceOf(address(this)));
    }
}
ITO_IZivoeYDL(IZivoeGlobals_ITO(GBL).YDL()).unlock();
ITO_IZivoeTranches(IZivoeGlobals_ITO(GBL).ZVT()).unlock();

By implementing this change, the contract can continue the migration process even if one of the stablecoin transfers fails, ensuring that the other stablecoins are not stuck in the contract.

pseudonaut commented 4 months ago

Not relevant

sherlock-admin3 commented 4 months ago

1 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, no scenario when transfer can fail

sherlock-audit / 2024-03-zivoe-judging

Tychai0s - Partial Migration Failure Risk #122