sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

cryptphi - Vault.update() does not save updated context #133

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

cryptphi

high

Vault.update() does not save updated context

Summary

_update() in Vault.update() does not return updated context, hence _saveContext() only saves the previous loaded context in memory and not the updated context

Vulnerability Detail

In the Vault contract, the external function update(), is meant to update the account's position anddepositing depositAssets assets, redeeming redeemShares shares, and claiming claimAssets assets by calling the internal function _update() with the local context variable as part of the arguments supplied. However the _update() function does not return the updated context thus making the call to _saveContext() only save the previous loaded context in memory and not the updated context. This means that the account's position updates are not saved.

Impact

Loss of funds

Code Snippet

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

_loadContext(account) sets context values to a local Context variable

        Context memory context = _loadContext(account);

        _settle(context);
        _checkpoint(context);
        _update(context, account, depositAssets, redeemShares, claimAssets);
        _saveContext(context, account);

Tool used

Manual Review

Recommendation

Vault._update() function should return Context memory context values for _saveContext to store the new context values.

sherlock-admin commented 1 year ago

2 comment(s) were left on this issue during the judging contest.

141345 commented:

x

panprog commented:

invalid because context doesn't need to be returned to be updated in memory