Loss of funds

[c4-bot-5]

Lines of code

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

Vulnerability details

In the ILOPool::buy function of the smart contract, there is a potential issue where if the liquidityDelta is computed to be zero, the user's funds will be transferred to the contract without them receiving any liquidity. This can occur when the raiseAmount is a small number, leading to zero liquidity due to the computation involving liquidityDelta.

Impact

The impact of this issue is significant as it can lead to a loss of funds for users. Specifically, users may transfer funds to the contract expecting to receive liquidity in return, but due to the zero liquidityDelta, they receive no liquidity. This results in users raising money without getting the corresponding shares, effectively losing their funds.

Proof of Concept (PoC)

Here is a simplified proof of concept demonstrating the issue:

function buy(uint256 raiseAmount) external {

    .................

    uint128 liquidityDelta = uint128(
        FullMath.mulDiv(liquidityDelta, _vestingConfigs[0].shares, BPS)
    );

    // increase investor's liquidity
    _position.liquidity += liquidityDelta;

    // update total liquidity locked for vest and assigning vesting schedules
    _positionVests[tokenId].totalLiquidity = _position.liquidity;

    // transfer fund into contract
    TransferHelper.safeTransferFrom(
        RAISE_TOKEN,
        msg.sender,
        address(this),
        raiseAmount
    );

    ............
}

If liquidityDelta is zero, the user's raiseAmount will be transferred to the contract, but the user's liquidity will not increase, leading to a loss of funds.

Mitigation

To mitigate this issue, a check should be added after the final computation to ensure that liquidityDelta is not zero before proceeding with the transfer of funds. Here is the revised code with the mitigation:

function buy(uint256 raiseAmount) external {
    uint128 liquidityDelta = uint128(
        FullMath.mulDiv(liquidityDelta, _vestingConfigs[0].shares, BPS)
    );

+   require(liquidityDelta > 0, "Liquidity delta must be greater than zero");

    // increase investor's liquidity
    _position.liquidity += liquidityDelta;

    // update total liquidity locked for vest and assigning vesting schedules
    _positionVests[tokenId].totalLiquidity = _position.liquidity;

    // transfer fund into contract
    TransferHelper.safeTransferFrom(
        RAISE_TOKEN,
        msg.sender,
        address(this),
        raiseAmount
    );
}

This additional check ensures that the liquidityDelta is greater than zero before increasing the investor's liquidity and transferring funds. This prevents the situation where funds are raised without the user receiving any liquidity, thereby protecting users from losing their funds.

Assessed type

Token-Transfer

[PeterSRWeb3]

@alex-ppg I think this can be marked as QA, because outlines real issue

[alex-ppg]

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

The likelihood of the liquidityDelta being so small as to amount to 0 due to truncation is effectively inexistent as the user would not perform the purchase in such a case.

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.