Not having granulated fees leads to loss of funds

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-06-krystal-defi/blob/f65b381b258290653fa638019a5a134c4ef90ba8/src/Common.sol#L103-L104

Vulnerability details

Impact

Users will lose their assets to the fees for the action that is not worth for it.

Proof of Concept

The Krystal DeFi applies only one type of fee for the protocol known as the PROTOCOL_FEE.

Currently, it´s set to max 10% as per the contracts while it can be changed anytime as the contracts have a setter for that.

Contract: Common.sol

103:         _maxFeeX64[FeeType.GAS_FEE] = 1844674407370955264; // 10% 
104:         _maxFeeX64[FeeType.PROTOCOL_FEE] = 1844674407370955264; // 10%

The protocol has one protocol fee - not as granulated as mintfee, swap fee, change liquidity fee, collect fee etc. The lack of projected fees leads to the users losing their funds to the protocol fees from their positions.

Below you can see the logic of deducting fees.

Contract: Common.sol

662:     /**
663:      * @notice calculate fee
664:      * @param emitEvent: whether to emit event or not. Since swap and mint have not had token id yet.
665:      * we need to emit event latter
666:      */ 
667:     function _deductFees(DeductFeesParams memory params, bool emitEvent) internal returns(uint256 amount0Left, uint256 amount1Left, uint256 amount2Left, uint256 feeAmount0, uint256 feeAmount1, uint256 feeAmount2) {
668:         uint256 Q64 = 2 ** 64;// 18 eth
669:         if (params.feeX64 > _maxFeeX64[params.feeType]) { 
670:             revert TooMuchFee();
671:         }
672: 
673:         // to save gas, we always need to check if fee exists before deductFees
674:         if (params.feeX64 == 0) {
675:             revert NoFees();
676:         }
677: 
678:         if (params.amount0 > 0) {
679:             feeAmount0 = FullMath.mulDiv(params.amount0, params.feeX64, Q64); 
680:             amount0Left = params.amount0 - feeAmount0;
681:             SafeERC20.safeTransfer(IERC20(params.token0), FEE_TAKER, feeAmount0);
682:         }
683:         if (params.amount1 > 0) {
684:             feeAmount1 = FullMath.mulDiv(params.amount1, params.feeX64, Q64);
685:             amount1Left = params.amount1 - feeAmount1;
686:             SafeERC20.safeTransfer(IERC20(params.token1), FEE_TAKER, feeAmount1);
687:         }
688:         if (params.amount2 > 0) {
689:             feeAmount2 = FullMath.mulDiv(params.amount2, params.feeX64, Q64);
690:             amount2Left = params.amount2 - feeAmount2;
691:             SafeERC20.safeTransfer(IERC20(params.token2), FEE_TAKER, feeAmount2);
692:         }
693: 
694: 
695:         if (emitEvent) {
696:             emit DeductFees(address(params.nfpm), params.tokenId, params.userAddress, DeductFeesEventData(
697:                 params.token0, params.token1, params.token2, 
698:                 params.amount0, params.amount1, params.amount2, 
699:                 feeAmount0, feeAmount1, feeAmount2,
700:                 params.feeX64,
701:                 params.feeType
702:             ));
703:         }
704: 
705:     }

As can be seen, the fees are taken even if the action is COMPOUND_FEES.

E.g. Imagine the user has 100 ETH in their LP and they have executed the action as COMPOUND_FEES for the LP they have.

The fee deduction will be more than the fees gained in the LP in this case due to having one fee type.

Imagine another user has 1000 ETH and changing their position from tick-x to tick-y Even if the fee is 1%, the protocol charges the user for 10 eth.

And If the user swaps 1000 eth, only 990 eth goes to swap - 10 eth is deducted for protocol.

However, if the swap has experienced slippage - there is always slippage - the user will swap 989 ETH (1 pct slippage) and 1 ETH will be refunded.

So there is also a hidden fee here by deducting the amount beforehand.

Example

For 1000 ETH, the platform had their 10 Eth fee, and the user has done only 989 ETH equivalent swap 1000 / 989 = 1.011 % So the hidden fee is 0.011% in 1% slippage

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a fee tier where the users can actually benefit from using the protocol. E.g.:

Swap Fee: 0.03%

Mint Fee: 0.02%

Collect Fee: 0.01%

Assessed type

Other

[3docSec]

Report of sufficient quality, provisionally marking as satisfactory

[c4-judge]

3docSec marked the issue as satisfactory

[c4-judge]

3docSec marked the issue as selected for report

[jarvisnn]

Disputed, fees are to be communicated transparently on our docs, and could be verified anytime

[3docSec]

Invalid: it's a missing feature rather than a vulnerability

[c4-judge]

3docSec marked the issue as unsatisfactory: Invalid