sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

levi - `BalancedVault` `sync()` can be gamed to gain an advantage #176

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

levi

medium

BalancedVault sync() can be gamed to gain an advantage

Summary

BalancedVault sync() can be gamed to gain an advantage

Vulnerability Detail

When BalancedVault::sync() is called the global context is updated while the account context is not. Users can call this function just before price movements to have global variables updated.

    function sync() external {
        syncAccount(address(0));
    }

They will then gain an advantage by calling BalancedVault::syncAccount() to process their delayed deposits or redemptions to either enter a position or exit a position based on the price movements that they are aware of and have timed.

    function syncAccount(address account) public {
        (EpochContext memory context, ) = _settle(account);
        _rebalance(context, UFixed18Lib.ZERO);
    }

By being able to time price movements and the sync of global variables and account variables, they gain an advantage by syncing their account just in time to either avoid losses or to gain profit.

Impact

Sophisticated users gain an unfair advantage in avoiding losses and earning profits.

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-vaults/contracts/balanced/BalancedVault.sol#L137-L149

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-vaults/contracts/balanced/BalancedVault.sol#L375-L423

Tool used

Manual Review

Recommendation

Updating the global variables with pending amounts should coincide with the update of accounts' pending amounts.