Althea Liquid Infrastructure - DanielArmstrong's results

Lines of code

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

Vulnerability details

Impact

Since zero addresses are inserted into holders array repeatedly, the length of holders array becomes extremely large. Thus, transfer function call of LiquidInfrastructureERC20 token will revert due to the lack of gas (DoS). And also, call of other several functions involving distributeToAllHolders function will revert due to the lack gas too (DoS).

Proof of Concept

LiquidInfrastructureERC20.sol#_beforeTokenTransfer function is the following.

    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();
        }
142:    bool exists = (this.balanceOf(to) != 0);
        if (!exists) {
            holders.push(to);
        }
    }

When _beforeTokenTransfer function is called by burn function, to = address(0) holds. Since the balance of zero address always be zero, this.balanceOf(to) = 0 and exist = false holds in L142, and so zero address is pushed into holders array. Thus, whenever burn function is called, zero addresses are pushed into holders array repeatedly.

On the other hand, the zero addresses which are pushed above may be popped in the following LiquidInfrastructureERC20.sol#_afterTokenTransfer function which is called by mint function.

    function _afterTokenTransfer(
        address from,
        address to,
        uint256 amount
    ) internal virtual override {
        bool stillHolding = (this.balanceOf(from) != 0);
        if (!stillHolding) {
            for (uint i = 0; i < holders.length; i++) {
                if (holders[i] == from) {
                    // Remove the element at i by copying the last one into its place and removing the last element
174:                holders[i] = holders[holders.length - 1];
                    holders.pop();
                }
            }
        }
    }

But if holders[holders.length - 1] = from in L174, the holder moved into holders[i] (=from) will not be removed. That is, at the end of the loop, froms could be remained in holders array. If zero addresses are located at the end of the array, at most half of the zero addresses are not removed from holders array after the mint function call.

Due to the two above issues, when burn function is called many times and mint function is called extremely small times, the length of holders array will become too large to process with zero addresses. Then, transfer function call of LiquidInfrastructureERC20 token will revert due to the lack of gas (DoS). And also, call of other several functions involving distributeToAllHolders function will revert due to the lack gas too (DoS).

Tools Used

Manual Review

Recommended Mitigation Steps

Modify LiquidInfrastructureERC20.sol#_beforeTokenTransfer function as follows.

    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);
        }
    }

And modify LiquidInfrastructureERC20.sol#_afterTokenTransfer function as follows.

    function _afterTokenTransfer(
        address from,
        address to,
        uint256 amount
    ) internal virtual override {
        bool stillHolding = (this.balanceOf(from) != 0);
        if (!stillHolding) {
            for (uint i = 0; i < holders.length; i++) {
                if (holders[i] == from) {
                    // Remove the element at i by copying the last one into its place and removing the last element
                    holders[i] = holders[holders.length - 1];
                    holders.pop();
++                  i--;
                }
            }
        }
    }

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

If distributableERC20s is changed during distribution, this can lead to wrong distribution.

Proof of Concept

LiquidInfrastructureERC20.sol#setDistributableERC20s function which sets distributed tokens is as follows.

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

As we can see above, there isn't any check. If this function is called during distribution, after this time unintended tokens are distributed to users. This process can be caused by front-running of malicious user.

Tools Used

Manual Review

Recommended Mitigation Steps

LiquidInfrastructureERC20.sol#setDistributableERC20s function has to be modified as follows.

    function setDistributableERC20s(
        address[] memory _distributableERC20s
    ) public onlyOwner {
+       require(!LockedForDistribution, "Still distributting");
        distributableERC20s = _distributableERC20s;
    }

Assessed type

Invalid Validation