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

Cheerful Taffy Dolphin - Lack of Slippage Protection in Share Minting Allows MEV Sandwich Attacks and Market Manipulation Which can Extract Value From Depositors #40

Open
sherlock-admin3 opened this issue Jan 31, 2025 · 0 comments

Comments

@sherlock-admin3
Copy link

Cheerful Taffy Dolphin

Medium

Lack of Slippage Protection in Share Minting Allows MEV Sandwich Attacks and Market Manipulation Which can Extract Value From Depositors

Summary

There is no slippage protection in the Vault's deposit mechanism, which means users have no way to specify minimum shares they expect to receive when depositing assets. This omission exposes users to potential value loss through share price manipulation and market movements during the transaction confirmation period. While the contract handles the deposit-to-shares conversion through convertToShares(), it lacks any guardrails to protect users from executing deposits at unexpectedly unfavorable rates.

The vulnerability manifests in the share price calculation mechanics within the vault's deposit flow. When users deposit assets, the conversion to shares occurs through convertToShares():

function convertToShares(UFixed6 assets) external view returns (UFixed6) {
    (UFixed6 _totalAssets, UFixed6 _totalShares) =
        (UFixed6Lib.unsafeFrom(totalAssets()), totalShares());
    return _totalShares.isZero() ? assets : assets.muldiv(_totalShares, _totalAssets);
}

The core issue is that the _update() function accepts the deposit and mints shares without any minimum share output validation:

https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/vault/contracts/Vault.sol#L313

function _update(
    Context memory context,
    address account,
    UFixed6 depositAssets,
    UFixed6 redeemShares,
    UFixed6 claimAssets
) private {
    // No slippage check before executing the deposit
    context.global.update(context.currentId, claimAssets, redeemShares, depositAssets, redeemShares);
    asset.pull(msg.sender, UFixed18Lib.from(depositAssets));
}

The share price can be manipulated between transaction submission and execution through multiple vectors:

  • The vault's market positions can be adjusted via _manage(), affecting totalAssets()
  • Underlying oracle price updates shift position valuations
  • Front-running deposits/withdrawals can alter the share/asset ratio through context.global.update()

This allows MEV bots to sandwich deposit transactions by:

  1. Front-running with actions that devalue shares
  2. Allowing victim's deposit to execute at unfavorable share ratio
  3. Back-running to restore share price and extract value

Impact

The vulnerability creates a systemic risk for depositors since their transactions can be exploited by MEV bots or suffer from adverse price movements with no recourse to revert based on share output. The lack of slippage protection means every deposit transaction is potentially vulnerable to sandwich attacks and market manipulation, leading to direct value extraction from depositors.

Fix

The fix requires adding slippage validation during the deposit flow:

// Add minSharesOut parameter to update function
if (convertToShares(depositAssets).lt(minSharesOut)) revert InsufficientSharesOut();

This enforces user-specified minimum share requirements before proceeding with deposit execution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant