Accounting for Fixed_Fee and Base_Fee twice leading to less amount or token recieved

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-shell/blob/c61cf0e01bada04c3d6055acb81f61955ed600aa/src/proteus/EvolvingProteus.sol#L548 https://github.com/code-423n4/2023-08-shell/blob/c61cf0e01bada04c3d6055acb81f61955ed600aa/src/proteus/EvolvingProteus.sol#L594 https://github.com/code-423n4/2023-08-shell/blob/c61cf0e01bada04c3d6055acb81f61955ed600aa/src/proteus/EvolvingProteus.sol#L641 https://github.com/code-423n4/2023-08-shell/blob/c61cf0e01bada04c3d6055acb81f61955ed600aa/src/proteus/EvolvingProteus.sol#L551 https://github.com/code-423n4/2023-08-shell/blob/c61cf0e01bada04c3d6055acb81f61955ed600aa/src/proteus/EvolvingProteus.sol#L644

Vulnerability details

Impact

There are multiple instances where fees is deducted twice. I will explain this with one function similar logic follows in other functions as well.So whenever the swapGivenInputAmount function is called by the user with inputAmount provided by the user then inside that function _swap function is called with FEE_DOWN and the input amount then the _swap function calls the _applyFeeByRounding function which reduces the amount of inputAmount and returns roundedSpecifiedAmount which is added to the initial xi(xi is xbalance),after that (xf,yf) is calculated using _findFinalPoint function which calls the _getPointGivenXandUtility function if x is specified input token(for y token similarly other function is called) which gives the final points (xf,yf) after that if specified token is x then the computed amount of y tokens the user will be calculated (i.e outputAmount) by again calling the _applyFeeByRounding function with yf - yi(outputTokens the user should receive ), feeDirection(in this case this is fee_down thus false) this _applyFeeByRounding function again reduces the amount by reducing it by fees. So the issue is here because the fees should only be applied once whereas here it is applied twice resulting in loss for the user who will recieve less tokens as output without even realizing.

Similar issue is with the following functions 1.swapGivenOutputAmount with the same _swap function

2. depositGivenInputAmount function with _reserveTokenSpecified function which reduces the xf value by using _applyFeeByRounding function which in return reduces the final utility thus decreasing the amount of output LP tokens that are being recieved by the user(sf-si) but again the _applyFeeByRounding function is applied thus again applying the fees and reducing the output LP tokens. 3.depositGivenOutputAmount function with _lpTokenSpecified function with similar logic as mentioned in the previous function(2.) 4.withdrawGivenOutputAmount with the _reserveTokenSpecified function 5.withdrawGivenInputAmount with the _lpTokenSpecified function

Proof of Concept

I will be explaining this with smaller values similar logic can be applied to the larger values For swapGivenInputAmount let's assume inputAmount = 10 and xBalance = 100 and yBalance = 100 Specified input token = x Now it will call _swap(FEE_DOWN(false),10,100,100,x) then it will call roundedSpecifiedAmount = _applyFeeByRounding( specifiedAmount = 10, feeDirection = FEE_Down(false) ); _applyFeeByRounding will calculate the roundedAbsoluteAmount = absoluteValue - (absoluteValue / BASE_FEE) - FIXED_FEE; as There is FEE_Down(false)
Then it returns the value with int256(roundedAbsoluteAmount) which is equal to roundedSpecifiedAmount So now roundedSpecificAmount will be less than specifiedAMount(10) i.e <10 Now back to the _swap function after calculating the roundedSpecificAmount it calculates the utility bu calling the _getUtility function then as we have specified the token as x it will perform the following calculations and call the appropriate functions as follows int256 fixedPoint = xi + roundedSpecifiedAmount; (xf, yf) = _findFinalPoint( fixedPoint, utility, _getPointGivenXandUtility ); Key point to understand here is that _findFinalPoint calls the _getPointGivenXandItility function which calculates the final y value by the formula y = (k^2 u^2)/(a k u + x) - b k u now note here that as x = xi + roundedSpecifiedAmount so yf<yi so now had there been x = xi + inputAmount(original amount specified by user) then this x would be larger than x calculated after applyingFee(as roundedSpecificamount < inputAMount) and thus the yf in this case would be smaller than the case when fees was applied. Now computedAmount is calculated by computedAmount = _applyFeeByRounding(yf - yi, feeDirection) here yf-yi is negative,_applyFeeByRounding applies fees on the absolute amount and then returns the negative of that value. Now this computedAmount which is negative is returned to the swapInputAMount function and is stored in the result variable int256 result = _swap( FEE_DOWN, int256(inputAmount), int256(xBalance), int256(yBalance), inputToken ); // amount cannot be less than 0 require(result < 0); it is then transformed into positive value outputAmount = uint256(-result); So now the main point here is that had we not applied fees on the input amount then x(o)= xi(100)+inputAmount(10)=110 then we would have calculated the yf by this formula y = (k^2 u^2)/(a k u + x) - b k u but we have applied fees so x(1)=xi(100)+roundedAmount(<10)<110 so after putting these two values of x in the equation we see that yf from x(0)<yf from x(1) ,naming yf from x(0) as y(0) and from x(1) as y(1) for instance y(0)=95 and y(1)= 98 so y(0)-yi = 95-100(initially taken as 100)=-5 y(1)-yi = 98-100 = -2 Now computedAmount = _applyFeeByRounding(yf - yi, feeDirection) is applied on both the above values which will again reduce these values for instance like for y(0)-yi = -5 it might return the value = -4 and for the other case it might return the value as -1. In the end both these negative values will be turned into positive values as 4,1 so we can see the recived output tokens have decreased becasue had we not applied fees it would have returned 4 as the final value but as we apply fees so 1 will be returned.But main issue here is in the las t step when it applies fees on yf-yi as we can see from the above y(0)-yi = 95-100(initially taken as 100)=-5 y(1)-yi = 98-100 = -2 in this case the returned outputAmount would have been 5,2 and as we have applied the fees the user will receive 2 as the outputAmount not 5(fees not applied)therefore successfully reducing the output amount.So applying fees twice reduces the amount of output tokens by larger amount affecting the recievable tokens by the user. Similar logic follows in other functions that i have mentioned above

Tools Used

Manual Review

Recommended Mitigation Steps

remove _applyFeeByRounding in the last step of the following functions _swap function ,_reserveTokenSpecified,_lpTokenSpecified, there are also checks introduced after every swap or deposit or withdrawal so that the xBalance and yBalance is not less than the min balance i.e the _checkBalances function.So there is no need for deducting fees twice.

Assessed type

Math

[c4-pre-sort]

0xRobocop marked the issue as primary issue

[c4-pre-sort]

0xRobocop marked the issue as low quality report

[0xRobocop]

Invalid.

Appling fee twice is the design choice. The first is to underestimate the perceived inputAmounts or overestimate the perceivedoutputAmounts. The second is to reduce observed outputAamounts or increase inputAmounts.

[c4-judge]

JustDravee marked the issue as unsatisfactory: Invalid