c4-bot-5 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-althea-liquid-infrastructure/blob/main/liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol?plain=1#L224-L226

Vulnerability details

Impact

LiquidInfrastructureERC20.distribute() transfers tokens to holders via the following line of code:

if (toDistribute.transfer(recipient, entitlement)) {
    receipts[j] = entitlement;
}

If the transfer ever returns false, then the distribute() function will silently fail and continue distributing tokens. This is problematic since if a transfer ever fails for a given distribution round, then the holders will lose out for that given distribution round.

It should be noted that the core of this issue is not the usage of transfer(), but how the protocol handles a token transfer failure and it's inability to properly pay recipients for that given round due to lack of accounting.

Proof of Concept

When a distribution occurs via LiquidInfrastructureERC20.distribute(), tokens are transferred to holders via ERC20.transfer().

// From LiquidInfrastructureERC20.distribute()
if (toDistribute.transfer(recipient, entitlement)) {
    receipts[j] = entitlement;
}

If transfer() returns false, the protocol silently fails and prevents the recipients from receiving the reward token. Once distribution is complete, the LiquidInfrastructureERC20 contract will experience a sleep state until the next distribution round begins. During this sleep time, the makeup of the holders and their Liquid Infrastructure ERC20 token balances will change. Once this next round begins, the missed rewards from the first round will be distributed however the holder makeup will change, leading to those missed rewards being incorrectly distributed.

To explain this further, let's take the following forge test which shows how this incorrect distribution occurs:

// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.12;

import "forge-std/Test.sol";
import "forge-std/console2.sol";

import "../liquid-infrastructure/contracts/LiquidInfrastructureERC20.sol";
import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";

contract MockERC721 is ERC721 {
    constructor() ERC721("Mock NFT", "MNFT") {}

    function mint(address to, uint256 tokenId) public {
        _mint(to, tokenId);
    }
}

contract MockERC20 is ERC20 {
    constructor() ERC20("Mock Token", "MT") {}

    function mint(address to, uint256 tokenId) public {
        _mint(to, tokenId);
    }
}

contract LiquidInfrastructureERC20Test is Test {
    LiquidInfrastructureERC20 private liquidInfraERC20;
    MockERC721 private mockNFT;
    MockERC20 private mockToken;
    address public receiver = address(0x07);
    address public receiver_2 = address(0x08);
    address public receiver_3 = address(0x09);

    function setUp() public {
        mockNFT = new MockERC721();
        address[] memory managedNFTs = new address[](1);
        managedNFTs[0] = address(mockNFT);

        address[] memory approvedHolders = new address[](3);
        approvedHolders[0] = receiver;  
        approvedHolders[1] = receiver_2; 
        approvedHolders[2] = receiver_3; 

        uint256 minDistributionPeriod = 10; // Example value, adjust as needed

        mockToken = new MockERC20();
        address[] memory distributableERC20s = new address[](1);
        distributableERC20s[0] = address(mockToken);

        liquidInfraERC20 = new LiquidInfrastructureERC20(
            "Liquid Infrastructure Token",
            "LIT",
            managedNFTs,
            approvedHolders,
            minDistributionPeriod,
            distributableERC20s
        );
    }

    function test_missedRewardsForFailure() public {
        uint256 amount = 1e18;
        liquidInfraERC20.mint(receiver, amount);
        liquidInfraERC20.mint(receiver_2, amount);

        // Simulate some time has passed
        vm.roll(11);
        mockToken.mint(address(liquidInfraERC20), 6e18);

        // simulate transfer returning false
        vm.mockCall(address(mockToken), abi.encodeWithSelector(ERC20.transfer.selector), abi.encode(false));
        liquidInfraERC20.distributeToAllHolders();
        vm.clearMockedCalls();

        // a new holder receives tokens
        liquidInfraERC20.mint(receiver_3, amount);

        vm.roll(22);
        liquidInfraERC20.distributeToAllHolders();

        // AUDIT: the tokens are distributed evenly among holders when really receiver and receiver_2 should have gained 3e18 each in the first round.
        assertEq(mockToken.balanceOf(address(receiver)), 2e18);
        assertEq(mockToken.balanceOf(address(receiver_2)), 2e18);
        assertEq(mockToken.balanceOf(address(receiver_3)), 2e18);
    }
}

Tools Used

Manual inspection

Recommended Mitigation Steps

The protocol should consider reverting distribution if the transfer token fails. Alternatively, the team can consider storing in the contract or via emitted events that a transfer failed and how much failed to send. The protocol team can then implement a separate function to re-try the delivery of reward tokens to the appropriate holders.