Althea Liquid Infrastructure - aslanbek's results

Lines of code

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

Vulnerability details

Vulnerability Details

Reward per share (entitlement) is calculated from rewardToken.balanceOf(this) / totalSupply():

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

Then, in distribute, rewards are distributed only to approved holders:

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

If a part of LiquidInfrastructureERC20's totalSupply is held by disapproved holders, they will not receive any rewards. These rewards are not forwarded to approved holders, but instead are left in the contract.

Proof of Concept

1. Alice and Bob each have 50e18 LiquidInfrastructureERC20s;

2. Bob became disapproved;

3. 100e18 DAI are distributed: entitlement = DAI.balanceOf(this) / totalSupply() = 100e18 / 100e18 = 1;

4. Bob does not receive anything (as intended); Alice receives 50e18 * 1 = 50e18 DAI; 50e18 DAI are left in the contract.

Impact

1. Approved holders do not receive a percentage of allocated rewards.
2. These rewards are stuck in the contract for indefinite amount of time (they can be fully unstuck only once the total balance of disapproved accounts is zero and there's another distribution of these reward tokens).

Recommended Mitigation Steps

Instead of totalSupply(), entitlement calculation should use the total balance of approved accounts, which should be tracked in functions approveHolder, disapproveHolder, and _transfer.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

    function setDistributableERC20s(
        address[] memory _distributableERC20s
    ) public onlyOwner {
        distributableERC20s = _distributableERC20s;
    }

setDistributableERC20s can be called by the owner during a distribution, which would be reasonable to call an owner mistake. However, it is possible to damage the system even if setDistributableERC20s is called outside of distribution:

1. Imagine there are 2 holders of LiERC20, both holding 0.5e18 tokens; the current array of distributable ERC20s is [DAI]. The owner wants to change it to [USDC, USDT], so he makes sure all rewards are distributed, and then calls setDistributableERC20s.

2. An attacker frontruns the owner, by donating 1e18 DAI and calling distribute(1), which sends 0.5e18 DAI to the first holder.

3. Then, the owner's transaction is mined: distributions and transfers become bricked, as the next distribute will attempt to transfer 0.5e18 USDC to the next holder, but the contract does not have any USDC, so the distribution will revert.

4. To lift the DoS, the owner will need to set the array back to [DAI], then finish the distribution, and setDistributableERC20s to [USDC, USDT] again.

The budget for the attack will be quite different depending if the "precision loss in calculating entitlement" issue is fixed:

a) If not fixed, the attacker will have to donate the same amount of DAI as the total supply of LiquidInfrastructureERC20, which is unfeasible (otherwise entitlement would round down to zero, and distributions of 0 tokens will not revert).

b) However, if the issue is fixed in a reasonable way, the required budget would be a fraction of a cent.

Recommended Mitigation Steps

    function setDistributableERC20s(
        address[] memory _distributableERC20s
    ) public onlyOwner {
+       require(!LockedForDistribution, "distribution in progress");
        distributableERC20s = _distributableERC20s;
    }

OR

    function setDistributableERC20s(
        address[] memory _distributableERC20s
    ) public onlyOwner {
+       if (LockedForDistribution) distributeToAllHolders();
        distributableERC20s = _distributableERC20s;
    }

Assessed type

DoS