Ironsidesec - `_redeemPT` uses 0 slippage

[sherlock-admin2]

Ironsidesec

High

_redeemPT uses 0 slippage

Summary

Notional will burn the shares to mint at a lower share value due to volatility or forced MEV. To protect from these, the slippage used is 0. That is the root cause and redeeming less amounts out than deserving is the impact.

Vulnerability Detail

_redeemPT below calls SY.redeem with 0 as minTokenOut. If you look at SYBase.redeem, there is a slippage check if amountTokenOut < minTokenOut. So, if you input 0 as min out then it is not slippage protected and prone to MEV attacks.

_redeemPT is called in 3 flows,

1. BaseStrategyVault.redeemFromNotional -> BaseStakingVault._redeemFromNotional -> EthenaVault._executeInstantRedemption -> PendlePrincipalToken._redeemPT
2. BaseStakingVault.initiateWithdraw -> EtherFi._initiateWithdrawImpl -> PendlePrincipalToken._redeemPT

Flow 2 has internal overrides with custom implementation at _initiateSYWithdraw between initiateWithdraw -> _initiateWithdrawImpl

Flow 1 is onlyNotional, but flow 2 can be called by any notional account It doesn't matter even if you use private mempool to redeem, the redemption is subjected to slippage loss. There is no guarantee that slippage will be zero or 100%, because its possible if the minimum amount input is 0, which is the case here. Due to high volatility, the share price may go too low, and we end up burning our shares at low prices leading to lower redemption amounts. And in flow 2, if a private mempool is not used, the it is subjected to MEV. Someone will redeem by burning the shares, then backrun Notional account's redemption, by depositing to mint shares at a much lower price.

https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/14d3eaf0445c251c52c86ce88a84a3f5b9dfad94/leveraged-vaults-private/contracts/vaults/staking/protocols/PendlePrincipalToken.sol#L137

    function _redeemPT(uint256 vaultShares) internal returns (uint256 netTokenOut) {
        uint256 netPtIn = getStakingTokensForVaultShare(vaultShares);
...SNIP...

>>>>    netTokenOut = SY.redeem(address(this), netSyOut, TOKEN_OUT_SY, 0, true);
    }

https://github.com/pendle-finance/pendle-core-v2-public/blob/97b1b9708478b389f9540d71816c7894aab6bb77/contracts/core/StandardizedYield/SYBase.sol#L76

    function redeem(
        address receiver,
        uint256 amountSharesToRedeem,
        address tokenOut,
  >>>>  uint256 minTokenOut,
        bool burnFromInternalBalance
    ) external nonReentrant returns (uint256 amountTokenOut) {
...SNIP...

        amountTokenOut = _redeem(receiver, tokenOut, amountSharesToRedeem);
  >>>>  if (amountTokenOut < minTokenOut) revert Errors.SYInsufficientTokenOut(amountTokenOut, minTokenOut);
        emit Redeem(msg.sender, receiver, tokenOut, amountSharesToRedeem, amountTokenOut);
    }

Impact

Notional will burn the shares to mint at a lower share value due to volatility or forced MEV. To protect from these, the slippage used is 0. That is the root cause and redeeming less amounts out than deserving is the impact. High impact due to loss of funds and likelihood is always due to hardcoded 0 slippage amount.

Code Snippet

https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/14d3eaf0445c251c52c86ce88a84a3f5b9dfad94/leveraged-vaults-private/contracts/vaults/staking/protocols/PendlePrincipalToken.sol#L137

https://github.com/pendle-finance/pendle-core-v2-public/blob/97b1b9708478b389f9540d71816c7894aab6bb77/contracts/core/StandardizedYield/SYBase.sol#L76

Tool used

Manual Review

Recommendation

https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/14d3eaf0445c251c52c86ce88a84a3f5b9dfad94/leveraged-vaults-private/contracts/vaults/staking/protocols/PendlePrincipalToken.sol#L137

-   function _redeemPT(uint256 vaultShares) internal returns (uint256 netTokenOut) {
+   function _redeemPT(uint256 vaultShares, uint256 minOut) internal returns (uint256 netTokenOut) {
        uint256 netPtIn = getStakingTokensForVaultShare(vaultShares);
        uint256 netSyOut;

...SNIP...

-       netTokenOut = SY.redeem(address(this), netSyOut, TOKEN_OUT_SY, 0, true);
+       netTokenOut = SY.redeem(address(this), netSyOut, TOKEN_OUT_SY, minOut, true);
    }

Duplicate of #70