internal _update() in Market.update() does not return updated context.

Summary

Vulnerability Detail

In the Market contract, the external function update(), is meant to update the account's position and collateral by loading the account's context in memory and then updating the accoun't context positions in the internal _update() before saving the updated context for the account. However the _saveContext() is only saving the the previous loaded context in memory and not the updated context. This means that while the account's position gets updated, its collateral in the local state is never updated.

The _update() call in https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/Market.sol#L86 should return the updated context in order for _saveContext() to save the new local state for the account.

Impact

Loss of account's collateral

Code Snippet

_loadContext(account) loads the account's context to memory and calls _update() with the memory variable context as an argument

        Context memory context = _loadContext(account);
        _settle(context, account);
        _update(context, account, newMaker, newLong, newShort, collateral, protect);
        _saveContext(context, account);

context is updated in _update() call but updates collateral for the inputed memory argument and does not store the updated values to storage

// update collateral
        context.local.update(collateral);
        context.currentPosition.local.update(collateral);

Tool used

Manual Review

Recommendation

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

sherlock-audit / 2023-07-perennial-judging

cryptphi - internal _update() in Market.update() does not return updated context. #124

internal _update() in Market.update() does not return updated context.

Summary

Vulnerability Detail

Impact

Code Snippet

Tool used

Recommendation