Any user can launch any project

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-06-vultisig/blob/cb72b1e9053c02a58d874ff376359a83dc3f0742/src/ILOManager.sol#L187-L198

Vulnerability details

Impact

The launch() function in the ILOManager contract does not restrict who can call it. This means any user can potentially launch any project, which can lead to unauthorized launches.

Vulnerability details

When a user calls initProject(), they beccome the project admin. This is done by the _cacheProject() call:

_project.admin = msg.sender;

The launch() function then is designed to initiate the launch of a project associated with a Uniswap V3 pool.

function launch(address uniV3PoolAddress) external override {
        require(block.timestamp > _cachedProject[uniV3PoolAddress].launchTime, "LT");
        (uint160 sqrtPriceX96, , , , , , ) = IUniswapV3Pool(uniV3PoolAddress).slot0();
        require(_cachedProject[uniV3PoolAddress].initialPoolPriceX96 == sqrtPriceX96, "UV3P");
        address[] memory initializedPools = _initializedILOPools[uniV3PoolAddress];
        require(initializedPools.length > 0, "NP");
        for (uint256 i = 0; i < initializedPools.length; i++) {
            IILOPool(initializedPools[i]).launch();
        }

        emit ProjectLaunch(uniV3PoolAddress);
    }

However, any user can call the launch() function. This is because there is no verification whatsoever on who the caller is. This can lead to potential unauthorized project launches.

Tools Used

Manual Review

Recommended Mitigation Steps

Add an access control to restrict the launch() function to only a specific authorized role.

-   function launch(address uniV3PoolAddress) external override {
+   function launch(address uniV3PoolAddress) external override onlyProjectAdmin(uniV3PoolAddress) {

Assessed type

Access Control

[Tigerfrake]

Hello judge, Can I get a reason why this was invalidated?

The ILOManager:launch() function as it is has no access control whatsoever which grants access to any user to invoke it for any project.

    function launch(address uniV3PoolAddress) external override {
        require(block.timestamp > _cachedProject[uniV3PoolAddress].launchTime, "LT");
        (uint160 sqrtPriceX96, , , , , , ) = IUniswapV3Pool(uniV3PoolAddress).slot0();
        require(_cachedProject[uniV3PoolAddress].initialPoolPriceX96 == sqrtPriceX96, "UV3P");
        address[] memory initializedPools = _initializedILOPools[uniV3PoolAddress];
        require(initializedPools.length > 0, "NP");
        for (uint256 i = 0; i < initializedPools.length; i++) {
>>          IILOPool(initializedPools[i]).launch();
        }
        emit ProjectLaunch(uniV3PoolAddress);
    }

Also, the ILOPool:launch() function called within it via the interface only checks if the function is being called from ILOManager via the OnlyManager() modifier but does not verify the caller.

    function launch() external override OnlyManager() {
     //...

        IILOManager.Project memory _project = IILOManager(MANAGER).project(uniV3PoolAddress);

     //...

        // transfer back leftover sale token to project admin
        _refundProject(_project.admin);

        _launchSucceeded = true;
    }

The only part where _project.admin comes into play is during refund:

// transfer back leftover sale token to project admin
_refundProject(_project.admin);

Now notice that the _project.admin is just passed there as a parameter.

Scenario:

• Alice owns projectA
• Bob calls ILOManager:launch() with uniV3PoolAddress linked to projectA.
• This will internally call ILOPool:launch() via the interface call, (IILOPool(initializedPools[i]).launch();)
• Here, the onlyManager() modifier checks if the call is from ILOManager contract which passes.
• The function executes then within it, it calls _refundProject(_project.admin); . This passes Alice's address because she's the admin for projectA meaning that the refund will be made to her.
• Finally the _launchSucceeded is set to true

As seen, Bob has launched projectA which ideally should be done by Alice herself.

Notice that claimRefund() also calls IILOPool(initializedPools[i]).claimProjectRefund() within it which just like IILOPool(initializedPools[i]).launch() also is decorated by the onlyManager() modifier. Expect that claimRefund() here, has access control provided by the onlyProjectAdmin(uniV3PoolAddress) modifier.

Kindly take another look. Thanks

[alex-ppg]

Hey @Tigerfrake, 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.

There is no harm that arises from a user calling launch via the manager as all state changes are validated locally on each ILOPool and all state changes are performed adequately. In reality, it might be wiser to open it up to anyone so as to ensure the project launches as soon as it is ready.

I understand that a discrepancy between the documentation and the implementation is observed, however, the discrepancy is of no actual impact. Given that no vulnerability or loss of functionality arises from this discrepancy (i.e. the feature is still accessible albeit by more roles and in a secure way), I consider this submission to be a QA level recommendation and thus ineligible for an HM reward.

This paragraph is included in all of my responses and confirms that no further feedback is expected in this submission as PJQA has concluded.