sherlock-audit / 2022-09-knox-judging

0 stars 0 forks source link

0x52 - Users can avoid performance fees by withdrawing before the end of the epoch forcing other users to pay their fees #75

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

0x52

high

Users can avoid performance fees by withdrawing before the end of the epoch forcing other users to pay their fees

Summary

No performance fees are taken when user withdraws early from the vault but their withdrawal value will be used to take fees, which will be taken from other users.

Vulnerability Detail

uint256 adjustedTotalAssets = _totalAssets() + l.totalWithdrawals;

if (adjustedTotalAssets > l.lastTotalAssets) {
    netIncome = adjustedTotalAssets - l.lastTotalAssets;

    feeInCollateral = l.performanceFee64x64.mulu(netIncome);

    ERC20.safeTransfer(l.feeRecipient, feeInCollateral);
}

When taking the performance fees, it factors in both the current assets of the vault as well as the total value of withdrawals that happened during the epoch. Fees are paid from the collateral tokens in the vault, at the end of the epoch. Paying the fees like this reduces the share price of all users, which effectively works as a fee applied to all users. The problem is that withdraws that take place during the epoch are not subject to this fee and the total value of all their withdrawals are added to the adjusted assets of the vault. This means that they don't pay any performance fee but the fee is still taken from the vault collateral. In effect they completely avoid the fee force all there other users of the vault to pay it for them.

Impact

User can avoid performance fees and force other users to pay them

Code Snippet

VaultInternal.sol#L504-L532

Tool used

Manual Review

Recommendation

Fees should be taken on withdrawals that occur before vault is settled

0xCourtney commented 1 year ago

https://github.com/KnoxFinance/knox-contracts/pull/87

rcstanciu commented 1 year ago

Reply from @arbitrary-CodeBeholder


looks good