Althea Liquid Infrastructure - SpicyMeatball's results

Lines of code

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

Vulnerability details

Impact

Every time tokens are burned, address(0) is pushed into holder array. This will further inflate already significant holders array and increase the likelihood of OOG reverts.

Proof of Concept

Because address(0) doesn't hold any liquidity tokens, every time there is a transfer with to == address(0), the check below https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L142

        bool exists = (this.balanceOf(to) != 0);
        if (!exists) {
            holders.push(to);
        }
    }

will push address(0) into holders array.

Coded POC for Foundry We need to make holders array public to run this test https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L48

+ address[] public holders;

import {LiquidInfrastructureERC20, ERC20} from "../contracts/LiquidInfrastructureERC20.sol";
import {LiquidInfrastructureNFT} from "../contracts/LiquidInfrastructureNFT.sol";
import {Test} from "forge-std/Test.sol";
import "forge-std/console.sol";

contract MockToken is ERC20 {
    constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
    function mint(address to, uint256 amount) external {
        _mint(to, amount);
    }
}

contract C4 is Test {
    LiquidInfrastructureERC20 liqERC20;
    MockToken usdc;
    address alice;
    address bob;
    function setUp() public {
        alice = address(0xa11cE);
        bob = address(0xb0b);
        usdc = new MockToken("USDC", "USDC");
        address[] memory rewards = new address[](1);
        rewards[0] = address(usdc);
        address[] memory approved = new address[](3);
        approved[0] = address(this);
        approved[1] = alice;
        approved[2] = bob;
        address[] memory nfts = new address[](3);
        nfts[0] = address(new LiquidInfrastructureNFT("NAME"));
        nfts[1] = address(new LiquidInfrastructureNFT("NAME"));
        nfts[2] = address(new LiquidInfrastructureNFT("NAME"));
        liqERC20 = new LiquidInfrastructureERC20("LIQ", "LIQ", nfts, approved, 10, rewards);
        for(uint256 i=0; i<nfts.length; i++) {
            usdc.mint(nfts[i], 1_000_000 * 1e18);
            LiquidInfrastructureNFT(nfts[i]).setThresholds(rewards, new uint256[](1));
            LiquidInfrastructureNFT(nfts[i]).transferFrom(address(this), address(liqERC20), 1);
        }
    }

    function testZeroAddressPush() public {
        liqERC20.mint(alice, 10_000 * 1e18);
        vm.startPrank(alice);
        liqERC20.burn(1);
        liqERC20.burn(1);
        liqERC20.burn(1);
        assertEq(liqERC20.holders(0), alice);
        assertEq(liqERC20.holders(1), address(0));
        assertEq(liqERC20.holders(2), address(0));
        assertEq(liqERC20.holders(3), address(0));
    }
}

Tools Used

Foundry

Recommended Mitigation Steps

+       if (!exists && to != address(0)) {
            holders.push(to);
        }
    }

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

Rewards that were intended for holders who were disapproved will remain in the contract.

Proof of Concept

To calculate reward shares for all holders, the contract uses the following formula: total reward * holder balance / total supply https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L270-L277

>>      uint256 supply = this.totalSupply();
        for (uint i = 0; i < distributableERC20s.length; i++) {
            uint256 balance = IERC20(distributableERC20s[i]).balanceOf(
                address(this)
            );
>>          uint256 entitlement = balance / supply;
            erc20EntitlementPerUnit.push(entitlement);
        }

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

>>          if (isApprovedHolder(recipient)) {
                uint256[] memory receipts = new uint256[](
                    distributableERC20s.length
                );
                for (uint j = 0; j < distributableERC20s.length; j++) {
                    IERC20 toDistribute = IERC20(distributableERC20s[j]);
>>                  uint256 entitlement = erc20EntitlementPerUnit[j] *
                        this.balanceOf(recipient);
                    if (toDistribute.transfer(recipient, entitlement)) {
                        receipts[j] = entitlement;
                    }
                }

Note, if holder is disapproved we don't transfer anything. Since we are using total supply this.totalSupply(), disapproved users will still have entitlement allocated to them, but the contract won't transfer it and as a result part of rewards will remain in the contract undistributed.

Coded POC for Foundry

import {LiquidInfrastructureERC20, ERC20} from "../contracts/LiquidInfrastructureERC20.sol";
import {LiquidInfrastructureNFT} from "../contracts/LiquidInfrastructureNFT.sol";
import {Test} from "forge-std/Test.sol";
import "forge-std/console.sol";

contract MockToken is ERC20 {
    constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
    function mint(address to, uint256 amount) external {
        _mint(to, amount);
    }
}

contract C4 is Test {
    LiquidInfrastructureERC20 liqERC20;
    MockToken usdc;
    address alice;
    address bob;
    function setUp() public {
        alice = address(0xa11cE);
        bob = address(0xb0b);
        usdc = new MockToken("USDC", "USDC");
        address[] memory rewards = new address[](1);
        rewards[0] = address(usdc);
        address[] memory approved = new address[](3);
        approved[0] = address(this);
        approved[1] = alice;
        approved[2] = bob;
        address[] memory nfts = new address[](3);
        nfts[0] = address(new LiquidInfrastructureNFT("NAME"));
        nfts[1] = address(new LiquidInfrastructureNFT("NAME"));
        nfts[2] = address(new LiquidInfrastructureNFT("NAME"));
        liqERC20 = new LiquidInfrastructureERC20("LIQ", "LIQ", nfts, approved, 10, rewards);
        for(uint256 i=0; i<nfts.length; i++) {
            usdc.mint(nfts[i], 1_000_000 * 1e18);
            LiquidInfrastructureNFT(nfts[i]).setThresholds(rewards, new uint256[](1));
            LiquidInfrastructureNFT(nfts[i]).transferFrom(address(this), address(liqERC20), 1);
        }
    }

    function testTokenStuck() public {
        liqERC20.mint(alice, 10_000 * 1e18);
        liqERC20.mint(bob, 10_000 * 1e18);
        liqERC20.withdrawFromAllManagedNFTs();
        assertEq(usdc.balanceOf(address(liqERC20)), 3_000_000 * 1e18);
        vm.roll(11);
        // disapprove bob
        liqERC20.disapproveHolder(bob);
        liqERC20.distribute(2);
        // bob's reward is stuck
        assertEq(usdc.balanceOf(address(liqERC20)), 1_500_000 * 1e18);
    }
}

Tools Used

Foundry

Recommended Mitigation Steps

We can burn liqudity tokens from disapproved holder, or if it is too harsh, we can add a holderSupply storage variable and use it instead of this.totalSupply() to calculate shares.

Assessed type

Token-Transfer

Judge has assessed an item in Issue #51 as 2 risk. The relevant finding follows:

Updating distributableERC20s array during distribution phase may break the entitlement accounting

https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L205 When distribute function is called, the contract precalculates entitlement values for tokens stored in distributableERC20s array

        // Calculate the entitlement per token held
        uint256 supply = this.totalSupply();
        for (uint i = 0; i < distributableERC20s.length; i++) {
            uint256 balance = IERC20(distributableERC20s[i]).balanceOf(
                address(this)
            );
            uint256 entitlement = balance / supply;
>>          erc20EntitlementPerUnit.push(entitlement);
        }

Note that the entitlements are pushed into the array in the same order that tokens are stored in distributableERC20s. If the contract owner attempts to change distributable tokens with setDistributableERC20s during the distribution phase

    function setDistributableERC20s(
        address[] memory _distributableERC20s
    ) public onlyOwner {
        distributableERC20s = _distributableERC20s;
    }

, and new tokens are stored in a different order, wrong amounts will be distributed. https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L220

                for (uint j = 0; j < distributableERC20s.length; j++) {
>>                  IERC20 toDistribute = IERC20(distributableERC20s[j]);
>>                  uint256 entitlement = erc20EntitlementPerUnit[j] *
                        this.balanceOf(recipient);
                    if (toDistribute.transfer(recipient, entitlement)) {
                        receipts[j] = entitlement;
                    }
                }

Consider adding this check to setDistributableERC20s

require(!LockedForDistribution,"distribution is in process");

Lines of code

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

Vulnerability details

Impact

withdrawFromManagedNFTs may skip withdrawal from some NFTs under a certain circumstances.

Proof of Concept

Consider a situation, LiquidInfrastructureERC20.sol manages 10 NFTs. Owner decides to release NFT#1 and NFT#2, he wants to collect revenues from NFT#1 and transfer NFT#2 to receiver with uncollected revenues. So he calls withdrawFromManagedNFTs(1) and then releaseManagedNFT(nft1, receiver), releaseManagedNFT(nft2, receiver).

Let's check releaseManagedNFT function

    function releaseManagedNFT(
        address nftContract,
        address to
    ) public onlyOwner nonReentrant {
        LiquidInfrastructureNFT nft = LiquidInfrastructureNFT(nftContract);
        nft.transferFrom(address(this), to, nft.AccountId());

        // Remove the released NFT from the collection
        for (uint i = 0; i < ManagedNFTs.length; i++) {
            address managed = ManagedNFTs[i];
            if (managed == nftContract) {
                // Delete by copying in the last element and then pop the end
>>              ManagedNFTs[i] = ManagedNFTs[ManagedNFTs.length - 1];
>>              ManagedNFTs.pop();
                break;
            }
        }
        // By this point the NFT should have been found and removed from ManagedNFTs
        require(true, "unable to find released NFT in ManagedNFTs");       

note how last NFT is switched with a released one, this may cause troubles in our case, since nextWithdrawal counter is set to 2

    function withdrawFromManagedNFTs(uint256 numWithdrawals) public {
        require(!LockedForDistribution, "cannot withdraw during distribution");

        if (nextWithdrawal == 0) {
            emit WithdrawalStarted();
        }

        uint256 limit = Math.min(
            numWithdrawals + nextWithdrawal,
            ManagedNFTs.length
        );
        uint256 i;
>>      for (i = nextWithdrawal; i < limit; i++) {
            LiquidInfrastructureNFT withdrawFrom = LiquidInfrastructureNFT(
                ManagedNFTs[i]
            );

            (address[] memory withdrawERC20s, ) = withdrawFrom.getThresholds();
            withdrawFrom.withdrawBalancesTo(withdrawERC20s, address(this));
            emit Withdrawal(address(withdrawFrom));
        }
>>      nextWithdrawal = i;

        if (nextWithdrawal == ManagedNFTs.length) {
            nextWithdrawal = 0;
            emit WithdrawalFinished();
        }
    }

Next time withdrawal is called NFT#9 and NFT#10 will not send their balances to the liquidity contract. One might assume that it's not a big deal, the owner can call the withdrawal function again. But in real life the contract may manage a hundreds of NFTs and calling withdrawal again may incur additional costs.

Coded POC for Foundry

import {LiquidInfrastructureERC20, ERC20} from "../contracts/LiquidInfrastructureERC20.sol";
import {LiquidInfrastructureNFT} from "../contracts/LiquidInfrastructureNFT.sol";
import {Test} from "forge-std/Test.sol";
import "forge-std/console.sol";

contract MockToken is ERC20 {
    constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
    function mint(address to, uint256 amount) external {
        _mint(to, amount);
    }
}

contract C4 is Test {
    LiquidInfrastructureERC20 liqERC20;
    MockToken usdc;
    address alice;
    address bob;
    function setUp() public {
        alice = address(0xa11cE);
        bob = address(0xb0b);
        usdc = new MockToken("USDC", "USDC");
        address[] memory rewards = new address[](1);
        rewards[0] = address(usdc);
        address[] memory approved = new address[](3);
        approved[0] = address(this);
        approved[1] = alice;
        approved[2] = bob;
        address[] memory nfts = new address[](3);
        nfts[0] = address(new LiquidInfrastructureNFT("NAME"));
        nfts[1] = address(new LiquidInfrastructureNFT("NAME"));
        nfts[2] = address(new LiquidInfrastructureNFT("NAME"));
        liqERC20 = new LiquidInfrastructureERC20("LIQ", "LIQ", nfts, approved, 10, rewards);
        for(uint256 i=0; i<nfts.length; i++) {
            usdc.mint(nfts[i], 1_000_000 * 1e18);
            LiquidInfrastructureNFT(nfts[i]).setThresholds(rewards, new uint256[](1));
            LiquidInfrastructureNFT(nfts[i]).transferFrom(address(this), address(liqERC20), 1);
        }
    }

    function testWithdrawSkip() public {
        address toRelease = liqERC20.ManagedNFTs(0);
        liqERC20.withdrawFromManagedNFTs(1);
        liqERC20.releaseManagedNFT(toRelease, alice);
        liqERC20.withdrawFromAllManagedNFTs();
        // assertion fail
        assertEq(usdc.balanceOf(address(liqERC20)), 3_000_000 * 1e18);// 2M != 3M
    }
}

Tools Used

Foundry

Recommended Mitigation Steps

Disable NFT release if nextWithdrawal != 0

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

If nextWithdrawal > ManagedNFTs.length, the contract won't be able to withdraw revenue from managed NFTs, because nextWithdrawal can't reset.

        uint256 limit = Math.min(
            numWithdrawals + nextWithdrawal,
            ManagedNFTs.length
        );
        uint256 i;
>>      for (i = nextWithdrawal; i < limit; i++) {
            LiquidInfrastructureNFT withdrawFrom = LiquidInfrastructureNFT(
                ManagedNFTs[i]
            );

            (address[] memory withdrawERC20s, ) = withdrawFrom.getThresholds();
            withdrawFrom.withdrawBalancesTo(withdrawERC20s, address(this));
            emit Withdrawal(address(withdrawFrom));
        }
        nextWithdrawal = i;

>>      if (nextWithdrawal == ManagedNFTs.length) {
            nextWithdrawal = 0;
            emit WithdrawalFinished();
        }

Proof of Concept

How nextWithdrawal can become greater than ManagedNFTs length? Multiple causes:

• withdrawFromManagedNFTs was called for 8 of 10 NFTs, and then 3 NFTs were released
• same as above but malicious user frontruns the NFT release with withdrawFromManagedNFTs transaction
• this may not be the case in the current implementation but if transferFrom will be switched to safeTransferFrom,as bot finding suggests, https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/4naly3er-report.md#l-1-unsafe-erc20-operations https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L418 the malicious user can call withdrawFromManagedNFTs from the ERC721 callback while ManagedNFTs length is not updated.

Coded POC for onERC721Received callback reentrancy case

import {LiquidInfrastructureERC20, ERC20} from "../contracts/LiquidInfrastructureERC20.sol";
import {LiquidInfrastructureNFT} from "../contracts/LiquidInfrastructureNFT.sol";
import {Test} from "forge-std/Test.sol";
import "forge-std/console.sol";

contract Exploit {
    LiquidInfrastructureERC20 target;
    constructor(LiquidInfrastructureERC20 _target) {
        target = _target;
    }
    function onERC721Received(address, address, uint256, bytes memory) public virtual returns (bytes4) {
        // set counter
        target.withdrawFromManagedNFTs(2);
        return this.onERC721Received.selector;
    }
}

contract MockToken is ERC20 {
    constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
    function mint(address to, uint256 amount) external {
        _mint(to, amount);
    }
}

contract C4 is Test {
    LiquidInfrastructureERC20 liqERC20;
    MockToken usdc;
    address alice;
    address bob;
    function setUp() public {
        alice = address(0xa11cE);
        bob = address(0xb0b);
        usdc = new MockToken("USDC", "USDC");
        address[] memory rewards = new address[](1);
        rewards[0] = address(usdc);
        address[] memory approved = new address[](3);
        approved[0] = address(this);
        approved[1] = alice;
        approved[2] = bob;
        address[] memory nfts = new address[](3);
        nfts[0] = address(new LiquidInfrastructureNFT("NAME"));
        nfts[1] = address(new LiquidInfrastructureNFT("NAME"));
        nfts[2] = address(new LiquidInfrastructureNFT("NAME"));
        liqERC20 = new LiquidInfrastructureERC20("LIQ", "LIQ", nfts, approved, 10, rewards);
        for(uint256 i=0; i<nfts.length; i++) {
            usdc.mint(nfts[i], 1_000_000 * 1e18);
            LiquidInfrastructureNFT(nfts[i]).setThresholds(rewards, new uint256[](1));
            LiquidInfrastructureNFT(nfts[i]).transferFrom(address(this), address(liqERC20), 1);
        }
    }

    function testWithdrawDOS() public {
        Exploit exploit = new Exploit(liqERC20);
        address nft = liqERC20.ManagedNFTs(0);
        address toRelease1 = liqERC20.ManagedNFTs(1);
        address toRelease2 = liqERC20.ManagedNFTs(2);

        liqERC20.withdrawFromAllManagedNFTs();
        assertEq(usdc.balanceOf(address(liqERC20)), 3_000_000 * 1e18);
        uint256 balBefore = usdc.balanceOf(address(liqERC20));
        liqERC20.releaseManagedNFT(toRelease2, address(exploit));
        liqERC20.releaseManagedNFT(toRelease1, alice);
        // new rewards are ready
        usdc.mint(nft, 1_000_000 * 1e18);
        liqERC20.withdrawFromAllManagedNFTs();
        uint256 balAfter = usdc.balanceOf(address(liqERC20));
        // 1 mil wasn't withdrawn
        assertEq(balBefore, balAfter);
    }

}

Tools Used

Foundry

Recommended Mitigation Steps

Consider modifying the check https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L382

+       if (nextWithdrawal >= ManagedNFTs.length) {
            nextWithdrawal = 0;
            emit WithdrawalFinished();
        }

Assessed type

DoS

LiquidInfrastructureERC20.sol doesn't have the ability to call recoverAccount

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

Occassionally devices and wallets can be lost, in which case the owner of this contract can call recoverAccount() to begin a recovery process which will finish after the transaction completes. Asynchronously the x/microtx module will ignore thresholds and transfer all of the Liquid Account's balances to this NFT, which may be withdrawn normally with withdrawBalances().

Currently there is no way to call recoverAccount(), in case of emergency, if NFT belongs to LiquidInfrastructureERC20.sol contract.

Some ERC20 tokens may become stuck inside LiquidInfrastructureERC20.sol

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

>>              for (uint j = 0; j < distributableERC20s.length; j++) {
                    IERC20 toDistribute = IERC20(distributableERC20s[j]);
                    uint256 entitlement = erc20EntitlementPerUnit[j] *
                        this.balanceOf(recipient);
                    if (toDistribute.transfer(recipient, entitlement)) {
                        receipts[j] = entitlement;
                    }
                }

When withdrawFromManagedNFTs is called all tokens from thresholdsErc20s array https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureNFT.sol#L39 are transferred to the LiquidInfrastructureERC20.sol address https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L376-L377

        for (i = nextWithdrawal; i < limit; i++) {
            LiquidInfrastructureNFT withdrawFrom = LiquidInfrastructureNFT(
                ManagedNFTs[i]
            );

>>          (address[] memory withdrawERC20s, ) = withdrawFrom.getThresholds();
>>          withdrawFrom.withdrawBalancesTo(withdrawERC20s, address(this));
            emit Withdrawal(address(withdrawFrom));
        }

However, there is no check if tokens from withdrawERC20s are in distributableERC20s array. As a result some tokens may be undistributed.

No duplicates check in addManagedNFT

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

    function addManagedNFT(address nftContract) public onlyOwner {
        LiquidInfrastructureNFT nft = LiquidInfrastructureNFT(nftContract);
        address nftOwner = nft.ownerOf(nft.AccountId());
        require(
            nftOwner == address(this),
            "this contract does not own the new ManagedNFT"
        );
>>      ManagedNFTs.push(nftContract);
        emit AddManagedNFT(nftContract);
    }

When the owner adds a new liquidity NFT there is no check if it's already present in ManagedNFTs array. If it is later released, only one duplicate address is removed.

    function releaseManagedNFT(
        address nftContract,
        address to
    ) public onlyOwner nonReentrant {
        LiquidInfrastructureNFT nft = LiquidInfrastructureNFT(nftContract);
        nft.transferFrom(address(this), to, nft.AccountId());

        // Remove the released NFT from the collection
        for (uint i = 0; i < ManagedNFTs.length; i++) {
            address managed = ManagedNFTs[i];
>>          if (managed == nftContract) {
                // Delete by copying in the last element and then pop the end
                ManagedNFTs[i] = ManagedNFTs[ManagedNFTs.length - 1];
                ManagedNFTs.pop();
>>              break;
            }
        }

This will cause a temporarily DOS on withdrawFromManagedNFTs call since liquidity ERC20 doesn't own the NFT. https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L377

Updating distributableERC20s array during distribution phase may break the entitlement accounting

https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L205 When distribute function is called, the contract precalculates entitlement values for tokens stored in distributableERC20s array

        // Calculate the entitlement per token held
        uint256 supply = this.totalSupply();
        for (uint i = 0; i < distributableERC20s.length; i++) {
            uint256 balance = IERC20(distributableERC20s[i]).balanceOf(
                address(this)
            );
            uint256 entitlement = balance / supply;
>>          erc20EntitlementPerUnit.push(entitlement);
        }

Note that the entitlements are pushed into the array in the same order that tokens are stored in distributableERC20s. If the contract owner attempts to change distributable tokens with setDistributableERC20s during the distribution phase

    function setDistributableERC20s(
        address[] memory _distributableERC20s
    ) public onlyOwner {
        distributableERC20s = _distributableERC20s;
    }

, and new tokens are stored in a different order, wrong amounts will be distributed. https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L220

                for (uint j = 0; j < distributableERC20s.length; j++) {
>>                  IERC20 toDistribute = IERC20(distributableERC20s[j]);
>>                  uint256 entitlement = erc20EntitlementPerUnit[j] *
                        this.balanceOf(recipient);
                    if (toDistribute.transfer(recipient, entitlement)) {
                        receipts[j] = entitlement;
                    }
                }

Consider adding this check to setDistributableERC20s

require(!LockedForDistribution,"distribution is in process");

Blacklisted approved holder may temporarily stop the distribution

Some tokens (e.g. USDT, USDC) have a blackist which prevents transferring tokens from or to certain accounts. If one of the approved holders is blacklisted, distribution will revert.

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

Some tokens (e.g. USDT) don't return bool value on transfer

NOT MENTIONED IN https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/bot-report.md#m-03-unsafe-use-of-transfertransferfrom-with-ierc20

As a result some receipts won't be included in Distribution event

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

                for (uint j = 0; j < distributableERC20s.length; j++) {
                    IERC20 toDistribute = IERC20(distributableERC20s[j]);
                    uint256 entitlement = erc20EntitlementPerUnit[j] *
                        this.balanceOf(recipient);
>>                  if (toDistribute.transfer(recipient, entitlement)) {
                        receipts[j] = entitlement;
                    }
                }

>>              emit Distribution(recipient, distributableERC20s, receipts);

Double check in distribute function

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

Since distribute function is the only entry point for _beginDistribution, and we've already checked !LockedForDistribution, we can discard the same check in _beginDistribution.

constructor doesn't check if the contract is managed NFT owner

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

    constructor(
        string memory _name,
        string memory _symbol,
>>      address[] memory _managedNFTs,
        address[] memory _approvedHolders,
        uint256 _minDistributionPeriod,
        address[] memory _distributableErc20s
    ) ERC20(_name, _symbol) Ownable() {
        ManagedNFTs = _managedNFTs;
        LastDistribution = block.number;

        for (uint i = 0; i < _approvedHolders.length; i++) {
            HolderAllowlist[_approvedHolders[i]] = true;
        }

        MinDistributionPeriod = _minDistributionPeriod;

        distributableERC20s = _distributableErc20s;

        emit Deployed();
    }

Consider adding the same check as in addManagedNFT

    function addManagedNFT(address nftContract) public onlyOwner {
        LiquidInfrastructureNFT nft = LiquidInfrastructureNFT(nftContract);
        address nftOwner = nft.ownerOf(nft.AccountId());
>>      require(
            nftOwner == address(this),
            "this contract does not own the new ManagedNFT"
        );
        ManagedNFTs.push(nftContract);
        emit AddManagedNFT(nftContract);
    }