code-423n4 / 2024-06-vultisig-validation

2 stars 0 forks source link

frontrunning every call to "initProject" will avoid any creator from using it #493

Open c4-bot-1 opened 5 months ago

c4-bot-1 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-06-vultisig/blob/main/src/ILOManager.sol#L57-L65 https://github.com/code-423n4/2024-06-vultisig/blob/main/src/ILOManager.sol#L124-L134

Vulnerability details

Impact

DoS on the ILOManager ILOPool creation process

Proof of Concept

The initProject function can be called by anyone, this is its expected behavior. The _cacheProject is then called by this function to store all information related to the project. The problem with this function is that it always reverts if a project has already initialized (liquidity pool registered). This also sets the admin of the project as the caller, so it's important for the admin to call the function with his address to configure the ILOManager settings.

An attacker could easily call this function just before any other address calls it. This will avoid that user from becoming the admin of its project. Not only that, but the attacker will be the only one able to initialize a new ILOPool with that uniswap pool (see initILOPool and onlyProjectAdmin. The attacker only spends gas since he only needs to listen to the function call in the mempool.

This cannot be avoided even if the project owner initializes the pool in another tx. Since _initUniV3PoolIfNecessary doesnt revert if the pool was already created.

POC: paste this function for testing below these lines.

    function testDOSonInitProject() external {
        vm.prank(DUMMY_ADDRESS);
        address uniV3Pool = iloManager.initProject(
            IILOManager.InitProjectParams({
                saleToken: mockProject().saleToken,
                raiseToken: mockProject().raiseToken,
                fee: 10000,
                initialPoolPriceX96: mockProject().initialPoolPriceX96 + 1,
                launchTime: mockProject().launchTime
            })
        );

        IILOManager.InitPoolParams memory params = _getInitPoolParams();
        //as long as project hasnt been initialized with this pool
        //any pool can be used
        params.uniV3Pool = uniV3Pool;

        IILOPool iloPool = IILOPool(_initPool(DUMMY_ADDRESS, params));

        assertEq(iloManager.project(uniV3Pool).admin, DUMMY_ADDRESS);

        // owner can no longer create project
        vm.prank(PROJECT_OWNER);
        vm.expectRevert(bytes("RE"));
        iloManager.initProject(
            IILOManager.InitProjectParams({
                saleToken: mockProject().saleToken,
                raiseToken: mockProject().raiseToken,
                fee: 10000,
                initialPoolPriceX96: mockProject().initialPoolPriceX96 + 1,
                launchTime: mockProject().launchTime
            })
        );
    }

Tools Used

Manual review

Recommended Mitigation Steps

There are many solutions that can be taken

Assessed type

DoS

vararg commented 4 months ago

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

1) ILOManager is a single source of truth for the protocol, corrupting it will have impact on whole protocol as well. 2) There is no way to cleanup "_cachedProject" collection and it will affect all future users, and not only frontrunned one. 3) DoS attack on ILOManager.initProject can be implemented without astronomical resources. That will end up by blocking users to use the protocol which results that protocol would get any profits.

alex-ppg commented 3 months ago

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

Front-running attacks revolving around initialization are detected by the static analyzer (i.e. L-16 of the bot report), and this particular attack vector would be a failure on the Vultisig team when deploying. A deployment script will be able to initialize the contract without any possibility of front-running, and there is no different way than the one described to initialize proxy implementations.

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.