Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openPosition() Lack of minimum token0PremiumPortion/token1PremiumPortion limit #27

Open
c4-bot-6 opened this issue Dec 21, 2023 · 4 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working M-11 primary issue Highest quality submission among a set of duplicates selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@c4-bot-6
Copy link
Contributor

Lines of code

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

Vulnerability details

Vulnerability details

In openPosition(), it allows token0PremiumPortion and token1PremiumPortion to be 0 at the same time.

In this case, if tokenId enters out_of_price, for example, UpperOutOfRange, anyone might be able to input:

marginFrom = 0
marginTo = 0
amountSwap = 0
zeroForOne = false
liquidity > 0

Note: amountFromBorrowed + marginFrom == 0 , so fees ==0

to open a new Position, borrow Liquidity, but without paying any fees. It's basically a no-cost loan.

Impact

out_of_price tokenId, due to the no-cost loan, might lead to the following issues:

  1. Malicious occupation of Liquidity
  2. Since both token0Premium/token1Premium are 0, the liquidator will not execute liquidatePosition(), because there is no profit.
  3. Since both token0Premium/token1Premium are 0, LP cannot get fees, but the borrower might still be able to profit.

Proof of Concept

The following test case demonstrates that if it is out_of_price, anyone can borrow at no cost.

add to OpenPosition.t.sol

    function testZeroFees() public {
        _setupUpperOutOfRange();
        uint128 borrowerLiquidity = _liquidity / _borrowerLiquidityPorition;
        console.log("borrowerLiquidity:",borrowerLiquidity);
        address anyone = address(0x123999);
        vm.startPrank(anyone);
        particlePositionManager.openPosition(
            DataStruct.OpenPositionParams({
                tokenId: _tokenId,
                marginFrom: 0,
                marginTo: 0,
                amountSwap: 0,
                liquidity: borrowerLiquidity,
                tokenFromPremiumPortionMin: 0,
                tokenToPremiumPortionMin: 0,
                marginPremiumRatio: type(uint8).max,
                zeroForOne: false,
                data: ""
            })
        );
        vm.stopPrank();
        (
            ,
            uint128 liquidity,
            ,
            uint128 token0Premium,
            uint128 token1Premium,
            ,
            ,            
        ) = particleInfoReader.getLien(anyone, 0);
        console.log("liquidity:",liquidity);
        console.log("token0Premium:",token0Premium);
        console.log("token1Premium:",token1Premium);
    }
Logs:
  borrowerLiquidity: 1739134199054731
  liquidity: 1739134199054731
  token0Premium: 0
  token1Premium: 0

Recommended Mitigation

It is suggested that openPosition() should add a minimum token0PremiumPortion/token1PremiumPortion limit.

    function openPosition(
        DataStruct.OpenPositionParams calldata params
    ) public override nonReentrant returns (uint96 lienId, uint256 collateralTo) {
...

+       require(params.tokenFromPremiumPortionMin>=MIN_FROM_PREMIUM_PORTION,"invalid tokenFromPremiumPortionMin");
+       require(params.tokenToPremiumPortionMin>=MIN_TO_PREMIUM_PORTION,"invalid tokenToPremiumPortionMin");

Assessed type

Other

@c4-bot-6 c4-bot-6 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Dec 21, 2023
c4-bot-6 added a commit that referenced this issue Dec 21, 2023
@c4-judge
Copy link
Contributor

0xleastwood marked the issue as primary issue

@c4-judge c4-judge added the primary issue Highest quality submission among a set of duplicates label Dec 21, 2023
@wukong-particle
Copy link

It's a good suggestion. We will add minimum premium in contract (current frontend enforces a minimum 1-2%).

@c4-judge
Copy link
Contributor

0xleastwood marked the issue as selected for report

@c4-judge c4-judge added the selected for report This submission will be included/highlighted in the audit report label Dec 23, 2023
@c4-sponsor
Copy link

wukong-particle (sponsor) confirmed

@c4-sponsor c4-sponsor added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Dec 24, 2023
@C4-Staff C4-Staff added the M-11 label Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working M-11 primary issue Highest quality submission among a set of duplicates selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

5 participants