Upgraded Q -> 2 from #132 [1709983798124]

[c4-judge]

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

Lines of code

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

Vulnerability details

Impact

Funds will be stuck in the contract as there are no recovery methods to take out shares of holders which owns the token but was disapproved at a later stage.

Proof of Concept

Main concept of LiquidInfrastructureERC20 is to distribute the revenue accured from NFTs to it’s holders.

Owner can use disapproveHolder to remove already approved holder from allowlist. It can be called anytime which is a normal behaviour.

    function disapproveHolder(address holder) public onlyOwner {
        require(isApprovedHolder(holder), "holder not approved");
        HolderAllowlist[holder] = false;
    }

During distribution, it is validated that only the approved holder can receive the tokens.

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

          emit Distribution(recipient, distributableERC20s, receipts);
      }

Now consider the following scenario:

1. Alice was approved initially and owns some LiquidInfrastructureERC20 tokens.
2. Owner wants to remove Alice because of some reason so Owner calls disapproveHolder which is a normal behaviour.
3. As Alice already owns the tokens, her share of distribution tokens will always be stuck in the contract which no one can ever withdraw.

Tools Used

VS Code

Recommended Mitigation Steps

I would recommend to add a function with onlyOwner access to withdraw the share of accounts who are unapproved but still owns the token.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Funds will be stuck in the contract as there are no recovery methods to take out shares of holders which owns the token but was disapproved at a later stage.

Proof of Concept

Main concept of LiquidInfrastructureERC20 is to distribute the revenue accured from NFTs to it's holders.

Owner can use disapproveHolder to remove already approved holder from allowlist. It can be called anytime which is a normal behaviour.

    function disapproveHolder(address holder) public onlyOwner {
        require(isApprovedHolder(holder), "holder not approved");
        HolderAllowlist[holder] = false;
    }

During distribution, it is validated that only the approved holder can receive the tokens.

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

          emit Distribution(recipient, distributableERC20s, receipts);
      }

Now consider the following scenario:

1. Alice was approved initially and owns some LiquidInfrastructureERC20 tokens.
2. Owner wants to remove Alice because of some reason so Owner calls disapproveHolder which is a normal behaviour.
3. As Alice already owns the tokens, her share of distribution tokens will always be stuck in the contract which no one can ever withdraw.

Tools Used

VS Code

Recommended Mitigation Steps

I would recommend to add a function with onlyOwner access to withdraw the share of accounts who are unapproved but still owns the token.

Assessed type

Other

[c4-judge]

0xA5DF marked the issue as duplicate of #703

[c4-judge]

0xA5DF marked the issue as partial-75

[c4-judge]

0xA5DF marked the issue as satisfactory