Separate CSSR historic data update and user facing operations

[code423n4]

Handle

hyh

Vulnerability details

Proof of Concept

MochiVault.withdraw and borrow, which are public user facing functions, do historic CSSR data update as a part of asset price request. It uses lots of gas and, given frequent enough user operations, most of gas spending can be unnecessary.

The need to update the data before getting a price is better served with separate system level script from security standpoint as well: 1) _data can be for any block number, so malicious user can send old data as an update, UniswapV2CSSR.saveReserve doesn't throw being fed with old data, i.e. data update can effectively be made void. https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-cssr/contracts/cssr/UniswapV2CSSR.sol#L78)

2) _data update is for one CSSR source only, i.e. uniswapCSSR or sushiCSSR, so even if valid recent data is being sent with withdraw/borrow for one of them, the other's data for getPrice can be still stalled. I.e. an attacker can update the source that isn't actually used for the asset (one without enough liquidity, for example). https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-cssr/contracts/adapter/UniswapV2TokenAdapter.sol#L77

In other words, separating user operations and system data update looks promising both as major gas saving measure (for example, if 10 users request withdraw/borrow with the same piece of data, its unpacking and transmitting is wasting gas in 90% cases) and from security standpoint (as placing data update functionality that doesn't throw when the real update didn't happened in the hands of a user isn't generally secure).

Recommended Mitigation Steps

In the same time for both functions only current price is actually needed, which can be performed via getPrice, given that historic data is updated otherwise. https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-core/contracts/vault/MochiVault.sol#L189 https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-core/contracts/vault/MochiVault.sol#L219 Both float memory price = engine.cssr().update(address(asset), _data); calls can be replaced with float memory price = engine.cssr().getPrice(address(asset)); while 'bytes memory _data' to be removed from MochiVault.withdraw, borrow, increase, decrease function signatures. Data update to be called via UniswapV2CSSR.saveReserve directly via separate system script with necessary frequency.

[r2moon]

There was enough validation of data.

[ghoul-sol]

per sponsor comment, invalid