frontrunning vulnerability in the code between calling update() and _update().

Summary

The issue is that between calling update() and _update(), there is a window where the state is settled but the user's deposit/redeem has not been processed yet. An attacker could watch for update() calls with large deposit/redeem amounts. After the state is settled but before update() is called, the attacker could make their own call to update() to deposit/redeem first and frontrun the user's intended deposit/redeem.

Vulnerability Detail

The issue occurs between the _settle() and _update() calls in update().

_settle() settles the global and local state of the vault up to the latest checkpoint. This updates the global shares, assets, etc.

Then _update() actually processes the user's deposit, redeem and claim.

So after _settle(), the global state reflects the new settled amounts, but before _update(), the user's intended deposit/redeem has not yet been processed.

This creates an opening for a frontrunning attack:

User calls update() with a large deposit amount
_settle() is called, settling global state, so vault now shows increased assets
Attacker watches the blockchain, sees the update() call and the global state change
Attacker quickly calls update() to deposit and redeem a large amount
Attacker's deposit and redeem is processed by _update()
Finally, the user's original _update() is called to process their deposit
This results in the attacker being able to frontrun and deposit/redeem first before the user's intended operation is applied.

Impact

The attacker can drain substantial assets from the vault by redeeming right after a large deposit. Or the attacker could deposit right before a large redeem, diluting the redeeming user's shares.

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol#L204-L213 https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol#L213 https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol#L215

Tool used

Manual Review

Recommendation

the _update() call logic should be moved into the update() function directly, removing the vulnerable window between _settle() and _update()

sherlock-audit / 2023-07-perennial-judging

feelereth - frontrunning vulnerability in the code between calling update() and _update(). #89