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

Slippage issues in redeem()/addLiquiditySingleSy()/removeLiquiditySingleSy() in PendlePowerFarmLeverageLogic.sol #95

Closed
c4-bot-2 opened this issue Mar 8, 2024 · 7 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-130 edited-by-warden sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-bot-2
Copy link
Contributor

c4-bot-2 commented Mar 8, 2024

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarm/PendlePowerFarmLeverageLogic.sol#L170
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarm/PendlePowerFarmLeverageLogic.sol#L183
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarm/PendlePowerFarmLeverageLogic.sol#L472

Vulnerability details

Impact

Pendle pools can be very volatile especially in extreme market scenarios, when performing leverage tactics in PendlePowerFarmLeverageLogic.sol it makes interactions with the Pendle pools the issue here is that on the call to:

  • addLiquiditySingleSy()
  • redeem()
  • removeLiquiditySingleSy()

The contract accepts that the minimum amount of tokens it receives to a minimum of zero.
Sample :

        uint256 tokenOutAmount = PENDLE_SY.redeem(
            {
                _receiver: address(this),
                _amountSharesToRedeem: netSyOut,
                _tokenOut: tokenOut,
                _minTokenOut: 0,
                _burnFromInternalBalance: false
            }
        );

Even this is specified in Pendle SDK.
https://api-v2.pendle.finance/sdk/#/

This is bad as an attacker can front-run the calls to cause the contracts to incur losses and backrun the tx to gain.
They require a slippage check to ensure the transactions do not end in loss to the caller.
This wouldn't revert the transaction and there loss of yield to the leverage strategy so I am labeling a high.

Tools Used

Manual review.

Recommended Mitigation Steps

As per the Docs use the underlying preview function to determine the supposed minimum amount out before performing the calls.

Assessed type

Other

@c4-bot-2 c4-bot-2 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 Mar 8, 2024
c4-bot-2 added a commit that referenced this issue Mar 8, 2024
@c4-bot-4 c4-bot-4 removed the 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value label Mar 8, 2024
@code4rena-admin code4rena-admin added the 3 (High Risk) Assets can be stolen/lost/compromised directly label Mar 8, 2024
@c4-bot-4 c4-bot-4 changed the title Slippage issues when redeem()/addLiquiditySingleSy() SY tokens. Slippage issues when redeem()/addLiquiditySingleSy()/removeLiquiditySingleSy()SY tokens. Mar 8, 2024
@c4-bot-3 c4-bot-3 changed the title Slippage issues when redeem()/addLiquiditySingleSy()/removeLiquiditySingleSy()SY tokens. Slippage issues in redeem()/addLiquiditySingleSy()/removeLiquiditySingleSy()SY & curve::exchange in PendlePowerFarmLeverageLogic.sol Mar 11, 2024
@c4-bot-5 c4-bot-5 changed the title Slippage issues in redeem()/addLiquiditySingleSy()/removeLiquiditySingleSy()SY & curve::exchange in PendlePowerFarmLeverageLogic.sol Slippage issues in redeem()/addLiquiditySingleSy()/removeLiquiditySingleSy() in PendlePowerFarmLeverageLogic.sol Mar 11, 2024
@c4-pre-sort
Copy link

GalloDaSballo marked the issue as duplicate of #130

@c4-pre-sort
Copy link

GalloDaSballo marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Mar 18, 2024
@c4-judge
Copy link
Contributor

trust1995 marked the issue as unsatisfactory:
Invalid

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Mar 26, 2024
@shealtielanz
Copy link

Hi @trust1995 this issue isn't a duplicate of #130 please look into this.

@trust1995
Copy link

There is an overall before / after value check, which is a fair way for the project to deal with slippage.

@shealtielanz
Copy link

Hi @trust1995 remember that this is the PendlePowerFarmLeverageLogic, my point is that they wouldn't achieve the supposed yield they were initially supposed to receive from the strategy medium looks appropriate at least

@trust1995
Copy link

I respect your position but disagree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-130 edited-by-warden sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

7 participants