c4-bot-6 commented 9 months ago

Lines of code

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

Vulnerability details

Description

ParticlePositionManager::addPremium:

File: protocol/ParticlePositionManager.sol

523:        liens.updatePremium(
524:            lienKey,
525:            uint24(((token0Premium + premium0) * Base.BASIS_POINT) / collateral0),
526:            uint24(((token1Premium + premium1) * Base.BASIS_POINT) / collateral1)
527:        );

If a user adds a lot of tokens to have a big premium it is possible that this cast to uint24 can overflow. If the premium is >16x (type(uint24).max/1_000_000). These tokens would be lost as the overflow would make the premium portion very low.

Although, a 16x premium is a lot hence it is unlikely this will happen.

Impact

Instead of getting a lot of premium a user would lock their tokens in the contract.

Proof of Concept

PoC test in LiquidatePosition.t.sol:

    function testAddPremiumOverflow() public {
        _openLongPosition();

        // add so there is some premium to be less than
        _addPremium(PREMIUM_0, 0);

        (uint128 token0PremiumBefore, ) = particleInfoReader.getPremium(SWAPPER, 0);
        (uint256 collateral0, ) = particleInfoReader.getRequiredCollateral(
            _liquidity / BORROWER_LIQUIDITY_PORTION,
            _tickLower,
            _tickUpper
        );

        // add to overflow
        _addPremium(
            uint128((type(uint24).max*collateral0)/1_000_000),
            0
        );

        (uint128 token0PremiumAfter, ) = particleInfoReader.getPremium(SWAPPER, 0);

        // more premium before adding premium than after
        assertGt(token0PremiumBefore,token0PremiumAfter);
    }

Tools Used

Manual audit

Recommended Mitigation Steps

Consider reverting if the premium portion is >type(uint24).max, similar to Base.uint256ToUint24.

Assessed type

Under/Overflow

c4-judge commented 9 months ago

0xleastwood marked the issue as primary issue

c4-judge commented 9 months ago

0xleastwood marked the issue as duplicate of #17

wukong-particle commented 9 months ago

Agree to use uint256ToUint24. Thanks for pointing out!

c4-judge commented 8 months ago

0xleastwood marked the issue as satisfactory