impossible to open a position with a large `marginTo`

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/protocol/ParticlePositionManager.sol#L212

Vulnerability details

Description

marginTo/From is a way to both cover your position and increase your premium when opening a position. There is however a unintended limit on how much marginTo you can provide when opening a position.

When doing the swap to increase leverage, the amountToMinimum (minimum received amount) is calculated:

ParticlePositionManager::openPosition:

File: contracts/protocol/ParticlePositionManager.sol

212:            collateralTo - cache.amountToBorrowed - params.marginTo, // amount needed to meet requirement

As you see here, a marginTo > collateralTo - cache.amountToBorrowed will underflow this calculation. Thus there is no way to provide a bigger margin than this.

Impact

A user cannot supply more margin than collateralTo - amountToBorrowed before the opening of the position reverts.

There is a workaround to supply no marginTo and then use addPremium to increase the premium but I believe this issue still breaks the intended purpose of marginTo in the openPosition call.

Proof of Concept

Test in OpenPosition.t.sol:

    function testOpenLongPositionTooMuchMargin() public {
        // test based on `testBaseOpenLongPosition` and `_borrowToLong`
        uint128 borrowerLiquidity = _liquidity / _borrowerLiquidityPorition;
        (uint256 amount0ToBorrow, uint256 amount1ToBorrow) = LiquidityAmounts.getAmountsForLiquidity(
            _sqrtRatioX96,
            _sqrtRatioAX96,
            _sqrtRatioBX96,
            borrowerLiquidity
        );
        (, uint256 requiredEth) = particleInfoReader.getRequiredCollateral(borrowerLiquidity, _tickLower, _tickUpper);
        uint256 amountNeeded = QUOTER.quoteExactOutputSingle(
            address(USDC),
            address(WETH),
            FEE,
            requiredEth - amount1ToBorrow,
            0
        );
        uint256 amountFrom = amountNeeded + amountNeeded / 1e6 - amount0ToBorrow;
        uint256 amountSwap = amountFrom + amount0ToBorrow;
        amountFrom += (amountSwap * FEE_FACTOR) / (BASIS_POINT - FEE_FACTOR);

        // user supplies a large `marginTo` to get a big premium
        uint256 marginTo = requiredEth - amount1ToBorrow + 1;

        vm.startPrank(WHALE);
        USDC.transfer(SWAPPER, amountFrom);
        WETH.transfer(SWAPPER, marginTo);
        vm.stopPrank();

        vm.startPrank(SWAPPER);
        TransferHelper.safeApprove(address(USDC), address(particlePositionManager), amountFrom);
        TransferHelper.safeApprove(address(WETH), address(particlePositionManager), marginTo);

        // impossible as it will revert on underflow
        vm.expectRevert(stdError.arithmeticError);
        particlePositionManager.openPosition(
            DataStruct.OpenPositionParams({
                tokenId: _tokenId,
                marginFrom: amountFrom,
                marginTo: marginTo,
                amountSwap: amountSwap,
                liquidity: borrowerLiquidity,
                tokenFromPremiumPortionMin: 0,
                tokenToPremiumPortionMin: 0,
                marginPremiumRatio: type(uint8).max,
                zeroForOne: true,
                data: new bytes(0) // will not make it to the swap
            })
        );
        vm.stopPrank();
    }

Tools Used

Manual audit

Recommended Mitigation Steps

Consider not counting marginTo towards expected output from the swap. As shown in another issue, there are further problems with this. marginTo is the premium for the "to"-side of the position. Hence should not be part of the expected output of the swap as it is the safety for the position.

Assessed type

Under/Overflow

[c4-judge]

0xleastwood marked the issue as primary issue

[wukong-particle]

Oh this is very interesting and careful point! So instead of the current check, we can do

params.marginTo > collateralTo - cache.amountToBorrowed ? 0 : collateralTo - cache.amountToBorrowed - params.marginTo

we still need marginTo to make the minimum amount for the swap right, happy to go back to the figures in https://excalidraw.com/#json=TcmwLn2W4K9H_UlCExFXa,J_yKjXNaowF0gYL8uvPruA for further discussion

[c4-judge]

0xleastwood marked the issue as selected for report

[wukong-particle]

confirm with the issue but our intended fix is slightly different from suggested.

[c4-sponsor]

wukong-particle (sponsor) confirmed

[romeroadrian]

Good catch 👍