Althea Liquid Infrastructure - rokinot's results

Lines of code

https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L359

Vulnerability details

In LiquidInfrastructureERC20.sol, the function withdrawFromManagedNFTs() serves to withdraw ERC20 assets from the managed NFTs that are held by the contract. This function has no control access, therefore anyone can call it. This function will pull numWithdrawals funds and save the current index. However, if the owner of the contract removes an amount of managed NFTs that put the current index out of range, the function will always revert.

Impact

The owner will DoS the withdrawal of assets to the contract, regardless of intention. Submitting as a medium since in theory the admin could add mock liquid infra contracts just to restart the loop.

Proof of Concept

Assume a LiquidInfrastructureERC20 holds 5 ManagedNFTs contracts, and the distribution process has begun User calls withdrawFromManagedNFTs(4) The owner proceeds to remove the last 2 elements from the managed NFTs array, leaving 3 managed NFTs remaining, with indexes 0, 1 and 2. Next time withdrawFromManagedNFTs() is called, the function will revert because it's trying to access index 3, when the array only allows access up to index 2.

Tools Used

Manual code reading

Recommended Mitigation Steps

Before the loop begins, add a check to see if nextWithdrawal is higher than numWithdrawals + nextWithdrawal or ManagedNFTs.length.

...
        uint256 limit = Math.min(
            numWithdrawals + nextWithdrawal,
            ManagedNFTs.length
        );

        if (nextWithdrawal > limit)
            nextWithdrawal = limit;

        uint256 i;
...

Assessed type

DoS