code-423n4 / 2024-06-size-findings

0 stars 0 forks source link

Double-Counting of Fragmentation Fee in getCashAmountIn Function Leads to Incorrect Calculations #408

Closed howlbot-integration[bot] closed 2 months ago

howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/AccountingLibrary.sol#L326-L327 https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/AccountingLibrary.sol#L329 https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/AccountingLibrary.sol#L303-L336

Vulnerability details

Impact

The calculation of cashAmountIn when there is credit fractionalization. In the case of credit fractionalization (creditAmountOut < maxCredit), the code calculates cashAmountIn as follows: getCashAmountIn#L326-L327

uint256 netCashAmountIn = Math.mulDivUp(creditAmountOut, PERCENT, PERCENT + ratePerTenor);
cashAmountIn = netCashAmountIn + state.feeConfig.fragmentationFee;

However, this calculation needs to be corrected. The fragmentationFee should not be added to cashAmountIn because it is already included in the calculation of the fees in the next line: getCashAmountIn#L329

fees = getSwapFee(state, netCashAmountIn, tenor) + state.feeConfig.fragmentationFee;

By adding fragmentationFee to both cashAmountIn and fees, the function is double-counting the fragmentation fee, which may lead to incorrect results.

Proof of Concept

Link to the getCashAmountIn function in the AccountingLibrary.solhttps://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/AccountingLibrary.sol#L303-L336

In the case of credit fractionalization (creditAmountOut < maxCredit), the fragmentationFee is added to both cashAmountIn and fees. This leads to double-counting of the fragmentation fee, resulting in an incorrect calculation of cashAmountIn.

The correct calculation should only include the fragmentationFee in the fees calculation, not in cashAmountIn.

Let's say creditAmountOut is 800, maxCredit is 1000, ratePerTenor is 10%, and state.feeConfig.fragmentationFee is 5.

With the current implementation

However, the correct calculation should be

Tools Used

Vs

Recommended Mitigation Steps

The calculation of cashAmountIn should be updated to

uint256 netCashAmountIn = Math.mulDivUp(creditAmountOut, PERCENT, PERCENT + ratePerTenor);
cashAmountIn = netCashAmountIn;

This way, the fragmentationFee is only included in the fees calculation, and cashAmountIn represents the correct amount of cash required to obtain the desired credit amount.

Assessed type

Math

c4-judge commented 2 months ago

hansfriese marked the issue as unsatisfactory: Invalid