M-01. The protocol will mint unnecessary fees if the vault is paused and reopened later.

Submitted by rvierdiiev, NeverGonnaGiveYulUp, 0xanmol, pontifex, hash. Selected submission by: 0xanmol.

Relevant GitHub Links

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXReader.sol#L38

Summary

Unnecessary fees will be minted to the treasury if the vault is paused and reopened later.

Vulnerability Details

Based on the test results, the protocol mints 5(this can be more) wei(gvToken) for each gvToken every second since the last fee collection. For example, if the totalSupply of gvToken is 1000000e18 and the time difference between the current block and the last fee collection is 10 seconds, the amount of lp tokens minted as a fee will be 50000000 wei in terms of gvToken. This is acceptable when the protocol is functioning properly.

javascript
function pendingFee(GMXTypes.Store storage self) public view returns (uint256) {
uint256 totalSupply_ = IERC20(address(self.vault)).totalSupply();
uint256 _secondsFromLastCollection = block.timestamp - self.lastFeeCollected;
return (totalSupply_ * self.feePerSecond * _secondsFromLastCollection) / SAFE_MULTIPLIER;
}

However, if the protocol needs to be paused due to a hack or other issues, and then the vault is reopened, let's say after 1 month of being paused, the time difference from block.timestamp - _secondsFromLastCollection will be = 2630000s

If the first user tries to deposit after the vault reopens, the fees charged will be 1000000e18 _ 5 _ 2630000 / 1e18 = 1315000000000

This is an unnecessary fee generated for the treasury because the vault was paused for a long time, but the fee is still generated without taking that into account. This can result in the treasury consuming a portion of the user shares.

Impact

This will lead to a loss of user shares for the duration when the vault was not active. The severity of the impact depends on the fee the protocol charges per second, the totalSupply of vault tokens, and the duration of the vault being paused.

Tools Used

manual review

Recommendations

If the vault is being reopened, there should be a function to override the _store.lastFeeCollected = block.timestamp; with block.timestamp again.