VaultManager.sol - user can never fully withdraw his collateral, due to portfolio fee inflation

[hats-bug-reporter[bot]]

Github username: @PlamenTSV Twitter username: @p_tsanev Submission hash (on-chain): 0xbef85828e8ad5adb6760bf16ea0a1ddd5c19eb1ca494f6452861ce86cc7f4298 Severity: high

Description: Description\ The vault manager contract manages both deposits and withdraws, with their respective sanity checks, limitations and cooldown periods. One important aspect of the protocol's well-working is the fees charged per operation. These fees are minted as portfolio tokens directly to the velvet treasury address instead of being taken from the caller. Fees are independent of the depositor/withdrawer's respective passed amounts. Instead fees are dependent only on the configurated percentages and the current total supply of the given portfolio token. The above, combined with the fact that fee shares are directly minted without being kept track off, leads to an artifical inflation in the vault's total supply which freezes a part of user's funds due to precision loss, resulting from said inflation.

Attack Scenario\ A step-by-step example would make it the easiest to visualize the issue with a set of parameters:

• let the initial portfolio amount be 1000e18 tokens
• let the total fees charged per operation be 10% of the total supply
• USDC and USDT are the 2 current tokens of our example portfolio vault

Now, let's have the scenario where Alice does several deposits and a withdrawal at the end:

1. Alice deposits 1000e6 of both tokens and since the current supply is 0, she gets the initial 1000e18 portfolio tokens as the first depositor
2. Alice decides she wants a larger deposit, so she deposits 1000e6 more of both tokens. This time since we have a total supply portfolio of 1000, the protocol charges the 10% fees and the new total supply becomes 1100. Due to this, Alice gets minted 1100 more portfolio tokens. This means that now both USDC and USDT balances are 200, Alice's portfolio balance is 2100, the total supply is 2200 since the treasury owns 100 portfolio fees
3. Alice decides to withdraw all her tokens. The protocol first charges the 10% fees, so the new total supply becomes 2420 = 2100 Alice + 320 treasury fees. When we reach the calculation of how much Alice should receive of both USDC and USDT, the calculation looks like: (2100 * 2000)/2420 = 1735.

As a result 265 of Alice's tokens are left stuck and she cannot claim them, since the total supply was inflated by the fees. She burned all her portfolio shares

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

Recommendation\ The problem arises from the fact that fees are untracked when regarding them in terms of total portfolio tokens and are also minted in the beginning of each operation, tampering with the total supply value later on in the call chain. The solutions I can propose are 2:

1. Track the minted portfolio tokens internally and use that variable to determine withdraw amounts
2. Track the minted fees and whenever withdrawing, first deduct those fees from the total supply to get the actual amount that's backing the contract's collateral, since the fees aren't back by any tokens

[langnavina97]

Our protocol mints fees to ensure users are appropriately charged for the service, thereby inflating the token amount. This inflation is fair to new users who deposit the same amount of tokens already in the vault because they receive a proportionally higher token amount, representing their actual value. Additionally, fees are charged as streaming fees per second over a year, not always at a flat rate of 10%. This approach ensures that users pay their fair share without causing unfair token inflation. @PlamenTSV