sherlock-audit / 2024-06-leveraged-vaults-judging

9 stars 8 forks source link

denzi_ - `minAmountOut` set to 0 in `_redeemPT()` can cause loss of funds through redemption #110

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 2 months ago

denzi_

High

minAmountOut set to 0 in _redeemPT() can cause loss of funds through redemption

Summary

In the contract PendlePrincipalToken.sol, function _redeemPT() is used for PT redemption whether it is expired or not.

This function redeems netSyOut amount of shares for TOKEN_OUT_SY but the function is passing in a hardcoded 0 in the minTokenOut Param

Vulnerability Detail

The function is :

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

        // PT tokens are known to be ERC20 compatible
        if (PT.isExpired()) {
            PT.transfer(address(YT), netPtIn);
            netSyOut = YT.redeemPY(address(SY));
        } else {
            PT.transfer(address(MARKET), netPtIn);
            (netSyOut, ) = MARKET.swapExactPtForSy(address(SY), netPtIn, "");
        }

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

Here the code first gets netSyOut and then redeems it for TOKEN_OUT_SY through this code

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

The 0 here is used for minTokenOut which is a slippage parameter. This causes the trade to execute even on lower than estimated/desired returns which causes loss of funds on the redemption.

minTokenOut should be used so that the function reverts in situations where the price has changed which can be done by external users (changing the reservers through flashloans etc.)

Impact

Loss of funds when redeeming.

Code Snippet

_redeemPT

Tool used

Manual Review

Recommendation

Enforce slippage protection by passing in minTokenOut

Duplicate of #70