Malicious lender can manipulate the fee to force borrower pay high premium

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/libraries/Base.sol#L132
https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/protocol/ParticlePositionManager.sol#L247
https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/protocol/ParticlePositionManager.sol#L118

Vulnerability details

Impact

Malicious lender can manipulate the fee to force borrower pay high premium

Proof of Concept

When user create a position, the fee is snapshot is queryed from
uniswap v3 position manager when preparing leverage

    function prepareLeverage(
        uint256 tokenId,
        uint128 liquidity,
        bool zeroForOne
    )
        internal
        view
        returns (
            address tokenFrom,
            address tokenTo,
            uint256 feeGrowthInside0LastX128,
            uint256 feeGrowthInside1LastX128,
            uint256 collateralFrom,
            uint256 collateralTo
        )
    {
        int24 tickLower;
        int24 tickUpper;
        (
            ,
            ,
            tokenFrom,
            tokenTo,
            ,
            tickLower,
            tickUpper,
            ,
            feeGrowthInside0LastX128,
            feeGrowthInside1LastX128,
            ,

        ) = UNI_POSITION_MANAGER.positions(tokenId);

and stored in the lien struct

// prepare data for swap
(
	cache.tokenFrom,
	cache.tokenTo,
	cache.feeGrowthInside0LastX128,
	cache.feeGrowthInside1LastX128,
	cache.collateralFrom,
	collateralTo
) = Base.prepareLeverage(params.tokenId, params.liquidity, params.zeroForOne);

then stored in the liens struct

// create a new lien
liens[keccak256(abi.encodePacked(msg.sender, lienId = _nextRecordId++))] = Lien.Info({
	tokenId: uint40(params.tokenId), // @audit
	liquidity: params.liquidity,
	token0PremiumPortion: cache.token0PremiumPortion,
	token1PremiumPortion: cache.token1PremiumPortion,
	startTime: uint32(block.timestamp),
	feeGrowthInside0LastX128: cache.feeGrowthInside0LastX128,
	feeGrowthInside1LastX128: cache.feeGrowthInside1LastX128,
	zeroForOne: params.zeroForOne
});

then when the position is closed, the premium interested paid depends on the spot value of the fee

// obtain the position's latest FeeGrowthInside after increaseLiquidity
(, , , , , , , , cache.feeGrowthInside0LastX128, cache.feeGrowthInside1LastX128, , ) = Base
	.UNI_POSITION_MANAGER
	.positions(lien.tokenId);

// caculate the amounts owed since last fee collection during the borrowing period
(cache.token0Owed, cache.token1Owed) = Base.getOwedFee(
	cache.feeGrowthInside0LastX128,
	cache.feeGrowthInside1LastX128,
	lien.feeGrowthInside0LastX128,
	lien.feeGrowthInside1LastX128,
	lien.liquidity
);

if the fee increased during the position opening time, the premium is used to cover the fee to make sure there are incentive for lenders to deposit V3 NFT as lender

as the comments points out

// calculate the the amounts owed to LP up to the premium in the lien
// must ensure enough amount is left to pay for interest first, then send gains and fund left to borrower

However, because the fee amount is queried from position manager in spot value,

malicious lender increase the liquidity to that ticker range and then can swap back and forth between ticker range to inflate the fee amount to force borrower pay high fee / high premium

while the lender has to pay the gas cost + uniswap trading fee

but if the lender add more liquidity to nft's ticker ragne, he can collect the majority of the liqudity fee + collect high premium from borrower (or forcefully liquidated user to take premium)

Tools Used

Manual Review

Recommended Mitigation Steps

it is recommend to cap the premium interest payment instead of query spot fee amount to avoid swap fee manipulation

Assessed type

Token-Transfer

[c4-judge]

0xleastwood marked the issue as primary issue

[romeroadrian]

The attack is quite impractical, the attacker would need to spend fees between all other LPs in range of the pool, so to get a small portion of it back.

[JeffCX]

Thanks for reviewing my submission @romeroadrian

spend fees between all other LPs in range of the pool

as the original report point out, the lender can always the lender add more liquidity to nft's ticker range to get more share of the fee

if he provider majority of the liquidity, he get majority of the fee

so as long as the premium added by borrower > gas cost + uniswap trading fee

borrower forced to lose money and lender lose nothing.

[wukong-particle]

I tend to agree with @romeroadrian on the general direction here. But let's play out the scenario @JeffCX outlined --

So for the attacker to be profitable, the amount they earn from liquidating a position should outweigh the swapping fees they pay to all other LPs.

Basically, Alice the attacker would need the fee generated in her borrowed liquidity to be more than the fee generated by all other LPs combined that cover her borrowed liquidity's tick range. Note that it should be other liquidity that "cover" the borrowed liquidity range (this even including full range).

This basically requires Alice to be the absolute dominating LP on the entire pool. If that kind of whale is swimming in our protocol, well, I think smart traders will be cautious and that LP won't earn as much in the first place. Basically the incentive will be very low based on typical investment-return ratio.

However, I do worry that some flash loan might have some attack angle -- flash loan enormous and outweigh other LPs. But that require the borrowed amount to outweigh other LPs, this doesn't seem to be doable with one tx flash loan.

Along this line though, if there's other novel attack pattern, happy to discuss any time!

[JeffCX]

So for the attacker to be profitable, the amount they earn from liquidating a position should outweigh the swapping fees they pay to all other LPs.

Yes, premium collected > gas cost + uniswap trading fee + fee paid to all other LPs

lender cannot control how much premium borrowers add.

but if lender see borrower's more premium is valuable

This basically requires Alice to be the absolute dominating LP on the entire pool. If that kind of whale is swimming in our protocol, well, I think smart traders will be cautious and that LP won't earn as much in the first place.

even if does not dominate the LP range in the beginning, he can always increase liquidity to dominate the LP range.

[c4-sponsor]

wukong-particle (sponsor) acknowledged

[c4-sponsor]

wukong-particle marked the issue as disagree with severity

[wukong-particle]

Acknowledge the issue because it's indeed a possible manipulation angle. But as the discussion goes as above, this attack is very impractical, unless the LP is the absolute dominance.

Also disagree with severity because such attack is not practical in economic terms.

Will let the judge join the discussion and ultimately decide. Thanks!

[c4-judge]

0xleastwood changed the severity to 2 (Med Risk)

[c4-judge]

0xleastwood marked the issue as selected for report

[romeroadrian]

@0xleastwood I'll reiterate my previous comment that this is impractical, there's no reason to perform this attack. Profitable conditions to trigger this attack are impossible in practice. Seems more on the QA side to me.

[0xleastwood]

While the attack is impractical in most cases, it is not infeasible. The attacker stands to profit under certain conditions so keeping this as is.