code-423n4 / 2024-02-althea-liquid-infrastructure-findings

3 stars 1 forks source link

Upgraded Q -> 2 from #51 [1709971279347] #765

Closed c4-judge closed 8 months ago

c4-judge commented 8 months ago

Judge has assessed an item in Issue #51 as 2 risk. The relevant finding follows:

Updating distributableERC20s array during distribution phase may break the entitlement accounting

https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L205 When distribute function is called, the contract precalculates entitlement values for tokens stored in distributableERC20s array

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

Note that the entitlements are pushed into the array in the same order that tokens are stored in distributableERC20s. If the contract owner attempts to change distributable tokens with setDistributableERC20s during the distribution phase

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

, and new tokens are stored in a different order, wrong amounts will be distributed. https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol#L220

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

Consider adding this check to setDistributableERC20s

require(!LockedForDistribution,"distribution is in process");
c4-judge commented 8 months ago

0xA5DF marked the issue as duplicate of #87

c4-judge commented 8 months ago

0xA5DF marked the issue as satisfactory