sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

ak1 - BalancedVault.sol#L137 : `sync()` can be called by anyone which would hurt the user when the market has negative fluctuations. #225

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ak1

medium

BalancedVault.sol#L137 : sync() can be called by anyone which would hurt the user when the market has negative fluctuations.

Summary

function sync() is public and can be called by anyone.

This call will do the following actions, rebalance the maker and taker positions and settle the account.

Since the account would be zero address when the sync function is called, it will not cause any issue for account related operations.

But, since the _rebalance is called during the sync process, the _rebalance do the rebalance for collateral as well as for position.

This would be updating the positions and collateral values unnecessarily.

Vulnerability Detail

sync- can be called by anyone.

function sync() external {
    syncAccount(address(0)); -------- called.
}

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

when we look at the _rebalance function , it will update the collateral and posittion.

function _rebalance(EpochContext memory context, UFixed18 claimAmount) private {
    _rebalanceCollateral(claimAmount);
    _rebalancePosition(context, claimAmount);

When the market is negative fluctuations, the rate of token would be less. Syncing suring this time would lead to settling the collateral and positions with less values

Impact

This would hurt the user balance when the market is in negative fluctuations.

Code Snippet

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

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

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

Tool used

Manual Review

Recommendation

Add modifier so that only the keeper would call this function or check valid account is calling.