Althea Liquid Infrastructure - Myrault's results

Lines of code

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

Vulnerability details

Impact

Any user can cause the distribute function in the LiquidInfrastructureERC20.sol contract to become unusable, making the contract no longer able to distribute rewards.

Proof of Concept

In the LiquidInfrastructureERC20.sol contract, there is an address[] private holders; array used to represent all addresses to which rewards can be distributed. The contract provides the ability to add accounts to the holders array in the _beforeTokenTransfer hook function.

/**
     * Implements the lock during distributions, adds `to` to the list of holders when needed
     * @param from token sender
     * @param to  token receiver
     * @param amount  amount sent
     */
    function _beforeTokenTransfer(
        address from,
        address to,
        uint256 amount
    ) internal virtual override {
        require(!LockedForDistribution, "distribution in progress");
        if (!(to == address(0))) {
            require(
                isApprovedHolder(to), // @audit-info: check if the account is approved to hold the ERC20 token
                "receiver not approved to hold the token"
            );
        }
        if (from == address(0) || to == address(0)) {
            _beforeMintOrBurn();
        }
        bool exists = (this.balanceOf(to) != 0);
        if (!exists) {
            holders.push(to); // @audit-info: push the address to holders <-------- here
        }
    }

If this account is approved to hold the ERC20 token and has no balance, it will be pushed to the holders array. This leads to a problem. Any account can transfer 0 amount to an Approved holder, forcing the address to be copied multiple times in the holders array.

In the constructor of the contract, the addresses in the parameter _approvedHolders are set to approved, and this HolderAllowlist can be seen by any account on the chain that has been approved in the mapping (users can view historical transactions of contract deployment Get which addresses have been approved)

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

The holders array will not check whether there are a large number of duplicate addresses in the array during the process of distributing rewards. Therefore, any account can transfer 0 amounts to an approved holder address multiple times, causing it to be copied infinitely in the holders array.

Since we can transfer 0 to any address in the holders array that has been approved and has a balance of 0, so far the attacker has the ability to make the length of the holers array infinite. In theory, all the logic of the for loop iteration of the holders will revert due to insufficient gas. But I won’t focus on that here.

In the contract, rewards are distributed to all holders mainly through the distribute function:

function distribute(uint256 numDistributions) public nonReentrant {
    // ...
    // ...
    for (i = nextDistributionRecipient; i < limit; i++) {
        address recipient = holders[i];
        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)) { // @audit-info: will revert here
                    receipts[j] = entitlement;
                }
            }

            emit Distribution(recipient, distributableERC20s, receipts);
        }
    }
    nextDistributionRecipient = i;

    if (nextDistributionRecipient == holders.length) {
        _endDistribution();
    }
}

In the process of executing this sentence if (toDistribute.transfer(recipient, entitlement)) {, since the rewards collected by the contract are limited, the rewards are not enough to pay the attacker additional copied addresses. This sentence will definitely be revert , causing the contract to no longer be able to distribute rewards.

Remix PoC

The vulnerability can be triggered by directly deploying the LiquidInfrastructureERC20 contract on remix，but first we need to prepare all the parameters for the execution of the LiquidInfrastructureERC20 constructor:

1. Create an additional ERC20 contract named MyToken. The main purpose is to pass in the address of this ERC20 contract as the _distributableErc20s parameter in the LiquidInfrastructureERC20 constructor.The address I created here is 0xa42b1378D1A84b153eB3e3838aE62870A67a40EA

contract MyToken is ERC20 {
    constructor() ERC20("MyToken", "MTK") {
        _mint(0x5B38Da6a701c568545dCfcB03FcB875f56beddC4, 10000000000000000);
    }
}

2. Create an arbitrary NFT contract and pass it into the constructor of LiquidInfrastructureERC20 as the _managedNFTs parameter, for example ["0x2E9d30761DB97706C536A112B9466433032b28e3"]

3. Prepare an address array to pass into the constructor as the _APPROVEDHOLDERS parameter, for example: ["0x5B38Da6a701c568545dCfcB03FcB875f56beddC4","0x78731D3Ca6b7E34aC0F824c42a7cC18A495cabaB","0x000000000000 000000000000000000000000011","0x000000000000000000000000000000000000022","0x000000000000000000000000000000000000 00000000000000000000000000000000000044"]

Then create this contract.In order to conveniently print some debugging information, you can add some logs to the program:

diff --git a/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol b/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol
index 4722279..da2d9f2 100644
--- a/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol
+++ b/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol
@@ -10,6 +10,8 @@ import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
 import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
 import "./LiquidInfrastructureNFT.sol";

+import "hardhat/console.sol";
+
 /**
  * @title Liquid Infrastructure ERC20
  * @author Christian Borst <christian@althea.systems>
@@ -213,6 +215,11 @@ contract LiquidInfrastructureERC20 is
         uint i;
         for (i = nextDistributionRecipient; i < limit; i++) {
             address recipient = holders[i];
+
+           console.log("i is %s",i);
+            console.log("recipient is ");
+            console.logAddress(recipient);
+
             if (isApprovedHolder(recipient)) {
                 uint256[] memory receipts = new uint256[](
                     distributableERC20s.length
@@ -221,7 +228,9 @@ contract LiquidInfrastructureERC20 is
                     IERC20 toDistribute = IERC20(distributableERC20s[j]);
                     uint256 entitlement = erc20EntitlementPerUnit[j] *
                         this.balanceOf(recipient);
+console.log("erc20EntitlementPerUnit[j] is %s,this.balanceOf(recipient) is %s,entitlement is %s",erc20EntitlementPerUnit[j],this.balanceOf(recipient),entitlement);
                     if (toDistribute.transfer(recipient, entitlement)) {
+                       console.log("success in transfer !!! !!!");
                         receipts[j] = entitlement;
                     }
                 }

The constructor parameters for creating a contract are as follows:

_NAME: "t1"
_SYMBOL: "t1"
_MANAGEDNFTS: ["0x2E9d30761DB97706C536A112B9466433032b28e3"]
_APPROVEDHOLDERS: ["0x5B38Da6a701c568545dCfcB03FcB875f56beddC4","0x78731D3Ca6b7E34aC0F824c42a7cC18A495cabaB","0x000000000000 000000000000000000000000011","0x000000000000000000000000000000000000022","0x000000000000000000000000000000000000 00000000000000000000000000000000000044"]
_MINDISTRIBUTIONPERIOD: "10"
_DISTRIBUTABLEERC20S: ["0xa42b1378D1A84b153eB3e3838aE62870A67a40EA"]

Before distributing rewards, the LiquidInfrastructureERC20 contract will execute the _beginDistribution function to calculate the distribution equity:

function _beginDistribution() internal {
		// ...
		// ...

        // 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; // @audit-info: Calculate the entitlement
            erc20EntitlementPerUnit.push(entitlement);
        }

        nextDistributionRecipient = 0;
        emit DistributionStarted();
    }

Here uint256 entitlement = balance / supply; is used to calculate the distribution equity. We can simulate it on remix through the following operations, let the balance variable be 8000 to simulate that this contract has collected a total of 8000 rewards from the nft contracts.At the same time, for easier mathematical calculation, we assume that the supply variable is 1000 at this time:

1. Use MyToken to transfer 8000 to LiquidInfrastructureERC20
2. Use LiquidInfrastructureERC20 mint 1000 to account 0x78731D3Ca6b7E34aC0F824c42a7cC18A495cabaB

Here we explain the role of several accounts in _APPROVEDHOLDERS. The address 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4 is used to simulate an account that the attacker can unlimited push to the holders array, because in the _beforeTokenTransfer hook function, the balance of the LiquidInfrastructureERC20 token required by the account must be 0, only after this condition is met, the attacker can push this address infinitely to the holders array.

Therefore we cannot mint 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4. However, it must be ensured that the totalSupply of the LiquidInfrastructureERC20 contract cannot be 0, so we choose to give 0x78731D3Ca6b7E34aC0F824c42a7cC18A495cabaB mint 1000 to meet the condition.

In this case, entitlement is 8000/1000 which is 8.

Then we switch any account and transfer 0 to the 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4 account multiple times, so that the 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4 address will appear many times in the holders array.I pushed about 8 of them. . (Since 1000 was previously mint to 0x78731D3Ca6b7E34aC0F824c42a7cC18A495cabaB, it will be added to the holders array first. At this time, holders[0] is 0x78731D3Ca6b7E34aC0F824c42a7cC18A495cabaB, and the others are 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4)

At this point, consider again some conditions in the distribute function:

function distribute(uint256 numDistributions) public nonReentrant {
    // ...
    // ...
    for (i = nextDistributionRecipient; i < limit; i++) {
        address recipient = holders[i];
        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)) { // @audit-info: will revert here
                    receipts[j] = entitlement;
                }
            }

            emit Distribution(recipient, distributableERC20s, receipts);
        }
    }
    nextDistributionRecipient = i;

    if (nextDistributionRecipient == holders.length) {
        _endDistribution();
    }
}

uint256 entitlement = erc20EntitlementPerUnit[j] *this.balanceOf(recipient);

This sentence is used to calculate the final reward amount received by each holder. However, if this.balanceOf(recipient) is 0, then the 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4 maliciously copied by the attacker actually receives 0 during each for loop transfer, so it cannot complete DOS attack.

I would like to add that an attacker can create a large number of addresses 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4 in the holders array immediately after the contract is deployed, that is, after the constructor is executed.

However, in this case, this.balanceOf(recipient) of the address is 0, so these malicious address is temporarily unable to consume the rewards collected by the contract. Therefore, the DOS attack scenario will occur after the address receives some tokens and has the rights to reward distribution on a later day.

So in order to obtain the allocated rewards normally, this.balanceOf(recipient) should not be 0 on a later day. So we simulate this by letting 0x78731D3Ca6b7E34aC0F824c42a7cC18A495cabaB transfer 5 tokens to 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4, which is to simulate that under normal circumstances, this address can be allocated rewards normally.

Finally, call the distributeToAllHolders function the transaction is revert, and DOS is successful. The relevant debugging log results are as follows:

i is 0
recipient is
0x78731D3Ca6b7E34aC0F824c42a7cC18A495cabaB
erc20EntitlementPerUnit[j] is 8,this.balanceOf(recipient) is 995,entitlement is 7960
success in transfer !!! !!!
i is 1
recipient is
0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
erc20EntitlementPerUnit[j] is 8,this.balanceOf(recipient) is 5,entitlement is 40
success in transfer !!! !!!
i is 2
recipient is
0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
erc20EntitlementPerUnit[j] is 8,this.balanceOf(recipient) is 5,entitlement is 40
transact to LiquidInfrastructureERC20.distributeToAllHolders errored: Error occured: revert.

revert
	The transaction has been reverted to the initial state.
Reason provided by the contract: "ERC20: transfer amount exceeds balance".
Debug the transaction to get more information.

It can be seen from the debugging log that holders[0], which is 0x78731D3Ca6b7E34aC0F824c42a7cC18A495cabaB, received a total of 995*8, which is 7960 rewards. At this time, there are only 40 left in the contract, and from holders[1] to holders[7] are all addresses 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4 that we maliciously added, and each address will consume 40 rewards, which ultimately leads to insufficient rewards for distribution.

Tools Used

Manual review,Remix

Recommended Mitigation Steps

You can consider adding a check. The amount of all transfers must be greater than 0.

Assessed type

DoS