Once refund is triggered, no one else can receive refunds

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-06-vultisig/blob/main/src/ILOManager.sol#L201-L207 https://github.com/code-423n4/2024-06-vultisig/blob/main/src/ILOPool.sol#L363-L373 https://github.com/code-423n4/2024-06-vultisig/blob/main/src/ILOPool.sol#L337-L347 https://github.com/code-423n4/2024-06-vultisig/blob/main/src/ILOPool.sol#L271-L272 https://github.com/code-423n4/2024-06-vultisig/blob/main/src/ILOPool.sol#L350-L360

Vulnerability details

Impact

Project admins can create multiple ILOPools for their project, which contains mechanism such as vesting, sale management, ERC721 integration, etc.

Users can invest in projects by calling ILOPool::buy, which transfers RAISE_TOKEN from the investor to the pool, in return for liquidity.

The project admin can also transfer SALE_TOKEN to the pool, which will be used to mint liquidity when the project is launched.

If the project has not launched yet and the refund deadline has passed, investors and the project admin can receive refunds for the amount of tokens they had transferred.

The problem is that each refund function has a refundable() modifier which requires that a refund has not yet triggered. When this modifier is executed, it sets _refundTriggered = true. This means that when the modifier is executed again, it will revert because _refundTriggered = true. Since each refund function has this modifier, all calls to refund will revert.

In addition, when a refund is triggered, the project can never launch. This will cause the RAISE_TOKEN and SALE_TOKEN to be permanently locked in the contract.

Proof of Concept

ILOManager.sol#L201-L207)

    function claimRefund(address uniV3PoolAddress) external override onlyProjectAdmin(uniV3PoolAddress) returns(uint256 totalRefundAmount) {
        require(_cachedProject[uniV3PoolAddress].refundDeadline < block.timestamp, "RFT");
        address[] memory initializedPools = _initializedILOPools[uniV3PoolAddress];
        for (uint256 i = 0; i < initializedPools.length; i++) {
@>          totalRefundAmount += IILOPool(initializedPools[i]).claimProjectRefund(_cachedProject[uniV3PoolAddress].admin);
        }
    }

This function allows the project admin to receive refunds. A call to ILOPool::claimProjectRefund is made:

ILOPool.sol#L363-L373

@>  function claimProjectRefund(address projectAdmin) external override refundable() OnlyManager() returns(uint256 refundAmount) {
        return _refundProject(projectAdmin);
    }

    function _refundProject(address projectAdmin) internal returns (uint256 refundAmount) {
        refundAmount = IERC20(SALE_TOKEN).balanceOf(address(this));
        if (refundAmount > 0) {
            TransferHelper.safeTransfer(SALE_TOKEN, projectAdmin, refundAmount);
            emit ProjectRefund(projectAdmin, refundAmount);
        }
    }

Note the refundable() modifier that is used:

ILOPool.sol#L337-L347

    modifier refundable() {
@>      if (!_refundTriggered) {
            // if ilo pool is lauch sucessfully, we can not refund anymore
            require(!_launchSucceeded, "PL");
            IILOManager.Project memory _project = IILOManager(MANAGER).project(_cachedUniV3PoolAddress);
            require(block.timestamp >= _project.refundDeadline, "RFT");

@>          _refundTriggered = true;
        }
        _;
    }

If the refund has already triggered, then no other refunds can be claimed. This is so that the project admin can't continue claiming refunds. In addition, _refundTriggered is used in ILOPool::launch to ensure that the project cannot be launch if a refund has triggered.

ILOPool.sol#L271-L272

    // when refund triggered, we can not launch pool anymore
    require(!_refundTriggered, "IRF");

Investors can also claim refunds by calling ILOPool::claimRefund

ILOPool.sol#L350-L360

@>  function claimRefund(uint256 tokenId) external override refundable() isAuthorizedForToken(tokenId) {
        uint256 refundAmount = _positions[tokenId].raiseAmount;
        address tokenOwner = ownerOf(tokenId);

        delete _positions[tokenId];
        delete _positionVests[tokenId];
        _burn(tokenId);

        TransferHelper.safeTransfer(RAISE_TOKEN, tokenOwner, refundAmount);
        emit UserRefund(tokenOwner, tokenId,refundAmount);
    }

We can see the same refundable() modifier is used.

This means that only one user can receive a refund, because when the refund modifier is first triggered, _refundTriggered will be set to true, which will DoS any other calls to claim refunds.

Example:

1. Project admin claims refund
2. refundable() modifier is triggered, setting _refundTriggered = true
3. Investor attempts to claim refund
4. refundable() modifier reverts transaction since _refundTriggered = true
5. Investor funds are permanently locked

Tools Used

Manual review

Recommended Mitigation Steps

Consider including a separate refundable() modifier for investors that claim refunds.

-   function claimRefund(uint256 tokenId) external override refundable() isAuthorizedForToken(tokenId) {
+   function claimRefund(uint256 tokenId) external override refundableUser() isAuthorizedForToken(tokenId) {
        uint256 refundAmount = _positions[tokenId].raiseAmount;
        address tokenOwner = ownerOf(tokenId);
+       totalRaised -= refundAmount;
        delete _positions[tokenId];
        delete _positionVests[tokenId];
        _burn(tokenId);

        TransferHelper.safeTransfer(RAISE_TOKEN, tokenOwner, refundAmount);
        emit UserRefund(tokenOwner, tokenId,refundAmount);
    }

Since the user's mapping is deleted once they have claimed a refund, they will not receive any tokens if they attempt to claim more than once. In addition, it may not be necessary to utilize _refundTriggered when investors claim refunds, otherwise all it takes is one investor to claim a refund to permanently DoS launches. Therefore the new refundableUser() modifier should have the following:

    modifier refundableUser() {
-       if (!_refundTriggered) {
            // if ilo pool is lauch sucessfully, we can not refund anymore
            require(!_launchSucceeded, "PL");
            IILOManager.Project memory _project = IILOManager(MANAGER).project(_cachedUniV3PoolAddress);
            require(block.timestamp >= _project.refundDeadline, "RFT");

-           _refundTriggered = true;
-       }
        _;
    }

Assessed type

Error

[crypticdefense]

@alex-ppg I believe this should be a valid finding because the exact same refund modifier is used for the claimProjectRefund and claimRefund functions, which can only be triggered once:

    modifier refundable() {
@>      if (!_refundTriggered) {
            // if ilo pool is lauch sucessfully, we can not refund anymore
            require(!_launchSucceeded, "PL");
            IILOManager.Project memory _project = IILOManager(MANAGER).project(_cachedUniV3PoolAddress);
            require(block.timestamp >= _project.refundDeadline, "RFT");

@>          _refundTriggered = true;
        }
        _;
    }

Therefore the scenario described in the finding where if a project admin decides to claim a refund, investors will no longer be able to claim any refunds. The investors' tokens will be permanently locked in the contract, as the project will also not be able to launch if the project admin claims a refund.

[alex-ppg]

Hey @crypticdefense, thank you for the PJQA contribution. I will preface all validation repository finding responses by stating that they are not evaluated by judges directly and are only evaluated by the validators if they are deemed unsatisfactory.

I am unsure as to why the code would revert. The if block effectively optimizes the refundable modifier to validate whether a refund should be triggered once. Afterward, the if block will not be executed and the code will continue as expected. As such, no vulnerability can be observed based on the report's contents.

This paragraph is included in all of my responses and confirms that no further feedback is expected in this submission as PJQA has concluded. You are free to refute any of my statements factually, however, I strongly implore you to do this with actual code references and examples.