Only whitelisted addresses can invest in projects, despite project admin's intention to allow anyone to invest

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-06-vultisig/blob/main/src/base/ILOWhitelist.sol#L12-L1 https://github.com/code-423n4/2024-06-vultisig/blob/main/src/base/ILOWhitelist.sol#L27-L38 https://github.com/code-423n4/2024-06-vultisig/blob/main/src/ILOPool.sol#L126-L176

Vulnerability details

Impact

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

Project admins have the authority to decide whether only whitelisted addresses can invest into the project (essentially private investors), or if anyone can invest.

The problem is that when a project admin sets the ability for anyone to invest into the project, the function ILOPool::buy, which is the function called by investors to invest into the project, doesn't take this into account.

Only whitelisted addresses can invest into projects permanently. The impact is that project creators must always know of potential investors ahead of time, as they must manually whitelist each address. This means that projects can essentially only have private investors, rather than the ability for anyone to invest.

Proof of Concept

Project admins can either directly whitelist addresses that can invest into the project or allow anyone to invest

ILOWhitelist.sol#L12-L14

    function setOpenToAll(bool openToAll) external override onlyProjectAdmin{
        _setOpenToAll(openToAll); //@audit anyone can invest if true
    }

ILOWhitelist.sol#L27-L38

    function batchWhitelist(address[] calldata users) external override onlyProjectAdmin{
        for (uint256 i = 0; i < users.length; i++) {
            _setWhitelist(users[i]);
        }
    }

    /// @inheritdoc IILOWhitelist
    function batchRemoveWhitelist(address[] calldata users) external override onlyProjectAdmin{
        for (uint256 i = 0; i < users.length; i++) {
            _removeWhitelist(users[i]);
        }
    }

The problem is that the function ILOPool::buy, which is the function that allows anyone to invest, only takes into account whitelisted users, not if the admin intends to allow anyone to invest

ILOPool.sol#L126-L176

    function buy(uint256 raiseAmount, address recipient)
        external override 
        returns (
            uint256 tokenId,
            uint128 liquidityDelta
        )
    {
@>      require(_isWhitelisted(recipient), "UA");
        require(block.timestamp > saleInfo.start && block.timestamp < saleInfo.end, "ST");
        // check if raise amount over capacity
        require(saleInfo.hardCap - totalRaised >= raiseAmount, "HC");
        totalRaised += raiseAmount;

        require(totalSold() <= saleInfo.maxSaleAmount, "SA");

        ...

        TransferHelper.safeTransferFrom(RAISE_TOKEN, msg.sender, address(this), raiseAmount);

        emit Buy(recipient, tokenId, raiseAmount, liquidityDelta);
    }

There is no check if isOpenToAll() is set to true. This means that if the admin intends to allow anyone to invest, they must individually whitelist each address. However, this is problematic if there any hundrends or thousands of users that would like to invest. The investors must also be known to the project admin ahead of time, as the address has to be known to the project admin for them to whitelist the investors.

Tools Used

Manual review

Recommended Mitigation Steps

If the project admin intends to allow anyone to invest, take that into account:

    function buy(uint256 raiseAmount, address recipient)
        external override 
        returns (
            uint256 tokenId,
            uint128 liquidityDelta
        )
    {
-       require(_isWhitelisted(recipient), "UA");
+       require(_isWhitelisted(recipient) || isOpenToAll(), "UA");
        require(block.timestamp > saleInfo.start && block.timestamp < saleInfo.end, "ST");
        // check if raise amount over capacity
        require(saleInfo.hardCap - totalRaised >= raiseAmount, "HC");
        totalRaised += raiseAmount;

        require(totalSold() <= saleInfo.maxSaleAmount, "SA");

        ...

        TransferHelper.safeTransferFrom(RAISE_TOKEN, msg.sender, address(this), raiseAmount);

        emit Buy(recipient, tokenId, raiseAmount, liquidityDelta);
    }

Assessed type

Invalid Validation

[crypticdefense]

@alex-ppg This finding displays how despite a project admin's intention of allowing anyone to invest by setting _openToAll = true, only whitelisted addresses can invest. Due to the fact that many projects may have the intention to have hundreds or thousands of investors, they would have to manually whitelist each address, which is problematic and can cause huge gas fees if they actually decide to whitelist individually. I believe this finding should be valid.

[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.

Two distinct whitelist systems exist within the system. The open-to-all validation check is properly incorporated into the system as seen here, rendering the submission invalid.

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.