search

code-423n4 / 2022-09-y2k-finance-findings

3 stars 1 forks source link

Fees are taken on risk collateral #44

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-y2k-finance/blob/ac3e86f07bc2f1f51148d2265cc897e8b494adf7/src/Vault.sol#L203-L234

Vulnerability details

Impact

Fees are taken on funds deposited as collateral

Proof of Concept

uint256 feeValue = calculateWithdrawalFeeValue(entitledShares, id);

In L226 of Vault.sol#withdraw the fee is taken on the entire collateral deposited by the risk users. This is problematic for two reasons. The first is that the collateral provided by the risk users will likely be many many times higher than the premium being paid by the hedge users. This will create a strong disincentive to use the protocol because it is likely a large portion of the profits will be taking by fees and the risk user may unexpectedly lose funds overall if premiums are too low.

The second issue is that this method of fees directly contradicts project documents which clearly indicate that fees are only taken on the premium and insurance payouts, not when risk users are receiving their collateral back.

Tools Used

Recommended Mitigation Steps

Fee calculations should be restructured to only take fees on premiums and insurance payouts

HickupHH3 commented 1 year ago

Agree with the warden. If the withdrawal fee exceeds the premium paid, risk users are disincentivised to provide collateral.

The second issue is that this method of fees directly contradicts project documents which clearly indicate that fees are only taken on the premium and insurance payouts, not when risk users are receiving their collateral back.

Not sure if the warden is referring to the fee as the trading fee c in the article, but I would agree if it's the case. Implementation isn't according to spec. The actual fees charged might be more than expected.