Race Condition in `initialize` Function Leading to Unauthorized Manager Assignment

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-06-vultisig/blob/cb72b1e9053c02a58d874ff376359a83dc3f0742/src/ILOPool.sol#L61

Vulnerability details

Proof of Concept

The initialize function in the ILOPool contract contains a race condition that can allow a malicious actor to set themselves as the MANAGER if they call the function before the intended user. The initialize function sets the MANAGER without verifying the caller's identity, which creates a vulnerability where any user can call initialize first and assign themselves as the MANAGER. This is evident from the following code snippet:

 function initialize(InitPoolParams calldata params) external override whenNotInitialized() {
        _nextId = 1;
        // initialize imutable state
        MANAGER = msg.sender;
        IILOManager.Project memory _project = IILOManager(MANAGER).project(params.uniV3Pool);

        WETH9 = IILOManager(MANAGER).WETH9();
        RAISE_TOKEN = _project.raiseToken;
        SALE_TOKEN = _project.saleToken;
        _cachedUniV3PoolAddress = params.uniV3Pool;
        _cachedPoolKey = _project._cachedPoolKey;
        TICK_LOWER = params.tickLower;
        TICK_UPPER = params.tickUpper;
        SQRT_RATIO_LOWER_X96 = params.sqrtRatioLowerX96;
        SQRT_RATIO_UPPER_X96 = params.sqrtRatioUpperX96;
        SQRT_RATIO_X96 = _project.initialPoolPriceX96;

        // rounding up to make sure that the number of sale token is enough for sale
        (uint256 maxSaleAmount,) = _saleAmountNeeded(params.hardCap);
        // initialize sale
        saleInfo = SaleInfo({
            hardCap: params.hardCap,
            softCap: params.softCap,
            maxCapPerUser: params.maxCapPerUser,
            start: params.start,
            end: params.end,
            maxSaleAmount: maxSaleAmount
        });

        _validateSharesAndVests(_project.launchTime, params.vestingConfigs);
        // initialize vesting
        for (uint256 index = 0; index < params.vestingConfigs.length; index++) {
            _vestingConfigs.push(params.vestingConfigs[index]);
        }

        emit ILOPoolInitialized(
            params.uniV3Pool,
            TICK_LOWER,
            TICK_UPPER,
            saleInfo,
            params.vestingConfigs
        );
    }

The vulnerability arises from the following lines:

MANAGER = msg.sender;  // Vulnerability: No verification of msg.sender

In this line, the msg.sender is assigned to the MANAGER variable without any checks to verify the identity of the caller. This allows any user who calls the initialize function first to become the MANAGER.

Impact

If a malicious user calls the initialize function before the intended deployer, they will become the MANAGER of the contract. This unauthorized access allows the attacker to have full control over the contract, including managing the project's funds and configurations. The attacker can:

1. Mismanage project funds.
2. Manipulate sale parameters.
3. Affect the vesting schedules.
4. Cause financial loss and reputational damage to the project.

Tools Used

Manual

Recommended Mitigation Steps

Add an authorization check in the initialize function to ensure that only the intended deployer or a specific address can call it. This can be achieved by passing the intended MANAGER address as a parameter to the constructor and restricting the initialize function to be callable only by this address.

Assessed type

Context

[Odhiambo526]

@alex-ppg Please let me know why this is invalid. Thanks!

[alex-ppg]

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

These types of vulnerabilities are generally considered QA and are usually caught by static analyzers, as is the case of this particular scenario in L-16. As such, the submission is considered OOS and ineligible for a 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. You are free to refute any of my statements factually, however, I strongly implore you to do this with actual code references and examples.