code-423n4 / 2024-02-spectra-findings

4 stars 2 forks source link

No deadline checks for slippage #55

Closed c4-bot-8 closed 8 months ago

c4-bot-8 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/main/src/tokens/PrincipalToken.sol#L220

Vulnerability details

Impact

deadline is a useful tool to ensure that your tx cannot be “saved for later”.

Due to the removal of the check, it may be more profitable for a validator to deny the transaction from being added until the transaction does not match slippage provided

Proof of Concept

 function depositIBT(
        uint256 ibts,
        address ptReceiver,
        address ytReceiver,
        uint256 minShares //@audit---info slippage here
    ) external override returns (uint256 shares) {
        shares = depositIBT(ibts, ptReceiver, ytReceiver);
        if (shares < minShares) {
            revert ERC5143SlippageProtectionFailed();
        }
    }

the function has a minShares parameter that helps the user protect themselves against slippage. However Without a deadline, the transaction might be left hanging in the mempool and be executed way later than the user wanted.

That could lead to users getting a worse price, because a validator can just hold onto the transaction.

other instances below

https://github.com/code-423n4/2024-02-spectra/blob/main/src/tokens/PrincipalToken.sol#L188

https://github.com/code-423n4/2024-02-spectra/blob/main/src/tokens/PrincipalToken.sol#L244

https://github.com/code-423n4/2024-02-spectra/blob/main/src/tokens/PrincipalToken.sol#L269

Tools Used

manual review

Recommended Mitigation Steps

Let users provide a fixed deadline as param

Assessed type

MEV

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as insufficient quality report

gzeon-c4 commented 8 months ago

invalid

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid