libratus - Possible drift from expected virtual price over time

[github-actions]

libratus

medium

Possible drift from expected virtual price over time

Summary

Virtual price grows slower than every 3 minutes and entirely depends on the interval at which _updateVirtualPrice is being called. As a result, less fees are collected.

Vulnerability Detail

_updateVirtualPrice vault internal function is supposed to update virtual price at a continuous rate over time. However, update rate depends on how often this code is being executed. This can cause price to be updated up to 50% slower than intended.

https://github.com/sherlock-audit/2022-11-isomorph/blob/main/contracts/Isomorph/contracts/Vault_Base_ERC20.sol#L212-L220

For example, if timeDelta / 180 has a remainder of 179, these 179 seconds will not be accounted for. lastUpdateTime is then set to _currentBlockTime which skips 179 seconds.

Example #1:
12:00:00 - loan opened
12:03:00 - collateral increased
12:06:00 - collateral increased
12:10:00 - collateral increased
This will lead to 3 increases in virtual price as expected for 10-minute interval

Example #2:
12:00:00 - loan opened
12:05:00 - collateral increased
12:10:00 - colleteral increased
This will lead to only 2 increases in virtual price during 10 minutes

I created a failing test case for this scenario: https://github.com/sherlock-audit/2022-11-isomorph-kiseln/commit/e30eec663c92f865d7592534af6ab5026c7038fb

Impact

Interest fees will be lower than expected. Down close to 50% in the worst possible scenario.

Code Snippet

Tool used

Manual Review

Recommendation

I think the easiest way to solve this is to pass
threeMinuteDelta * 180 + lastUpdateTime to collateralBook like the following:

collateralBook.vaultUpdateVirtualPriceAndTime(_collateralAddress, virtualPrice, threeMinuteDelta * 180 + lastUpdateTime);

This way lastUpdateTime will always increase by a product of 180 and no seconds will be lost

Duplicate of #158