Inaccurate/unfair treasury fee added to the minter's position on mintOsToken

[hats-bug-reporter[bot]]

Github username: @koolexcrypto Submission hash (on-chain): 0xeedef2c48b3ee2d09c092957352ed6acb8875851b45a1c7a4c4414558406d3ef Severity: high

Description:

Description

On mintOsToken, when OsToken position > 0, _syncPositionFee is called but only after minting OsTokenShares. This causes inaccurate treasury fee added to the position and it is unfair to the minter. because minting OsTokenShares increases _totalAssets of OsToken which impact how treasury fee is calculated.

Attack Scenario

There is not an attack here. It is actually a loss for the minter.

1. Bob mints 1 OsToken.

2. Position shares now is greater than 0.

3. Some time passes (e.g. 1 day).

4. Bob mints 1000 OsToken.

5. 1000 osToken shares are minted which increases _totalAssets of OsToken

6. Now, _syncPositionFee is called which calls _osToken.cumulativeFeePerShare.

7. _osToken.cumulativeFeePerShare is calculating the rewards (i.e. profitAccrued) by calling _unclaimedAssets.

8. _unclaimedAssets calculates the rewards based on many variables including _totalAssets as follows:

   return Math.mulDiv(avgRewardPerSecond * _totalAssets, timeElapsed, _wad);

   Code link

9. Since _totalAssets is bigger, the returned rewards will be bigger.

10. because of this, a bigger treasury fee will be added to the minter position which is not fair as the user just minted the 1000 OsToken and it shouldn't be considred in the treasury fee calculation.

Attachments

1. Proof of Concept (PoC)

• mintOsToken method where minting osToken shares occurs before _syncPositionFee

  
  // mint osToken shares to the receiver
  assets = _osToken.mintShares(receiver, osTokenShares);
  
  // fetch user position
  OsTokenPosition memory position = _positions[msg.sender];
  if (position.shares > 0) {
    _syncPositionFee(position);
  } else {

[Code link](https://github.com/stakewise/v3-core/blob/5996ae760a7e4a24d42029e64c56f3df087053cd/contracts/vaults/modules/VaultOsToken.sol#L64-L72)

- `_osToken.cumulativeFeePerShare` where it calls `_unclaimedAssets` to calculate rewards
```solidity
    // calculate rewards
    uint256 profitAccrued = _unclaimedAssets();
    if (profitAccrued == 0) return currCumulativeFeePerShare;

Code link

-_unclaimedAssets which considers _totalAssets for rewards calculation

    return Math.mulDiv(avgRewardPerSecond * _totalAssets, timeElapsed, _wad);

Code link

Recommended Mitigation

Sync the position before minting the shares. So, the user doesn't receive unfair additional treasury fee added to the position.

[tsudmi]

In mintShares we call updateState that syncs the fee before new osToken shares are minted. So when _osToken.cumulativeFeePerShare is called it will return the correct value as timeElapsed will be 0.