`getCreditAmountIn` returns lower fees than expected

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/AccountingLibrary.sol#L249

Vulnerability details

Impact

The getCreditAmountIn function calculates the amount of fees inaccurately, causing them to be less than intended by the protocol. As a result, there will be a loss of funds for the protocol as the fee recipient will not receive the full amount of swap fees.

Proof of Concept

The getCreditAmountIn function is used in the SellCreditMarket.sol contract when the caller has set params.exactAmountIn to false. It calculates the amount of swap fees that the fee recipient is going to receive the following way:

fees = Math.mulDivUp(cashAmountOut, swapFeePercent, PERCENT);

As we can see the fee is derived as a percent of cashAmountOut. The issue is that cashAmountOut is the same amount of funds that will be sent to the borrower:

state.data.borrowAToken.transferFrom(params.lender, msg.sender, cashAmountOut);

The problem is that in all other functions, that calculate cash/credit input and output amounts(getCashAmountOut, getCreditAmountIn, getCreditAmountOut, getCashAmountIn) from the AccountingLibrary.sol contract the fees are always derived from an amount greater than the cash that the borrower will actually receive. For example: 1/ In getCashAmountOut:

cashAmountOut = maxCashAmountOut - fees;

Here the fees are derived from maxCashAmountOut, and cashAmountOut is sent to the borrower. Therefore, when this method is used the fees would be greater than if getCreditAmountIn was used.

2/ In getCreditAmountOut:

fees = getSwapFee(state, cashAmountIn, tenor);

However, here the cash sent to the borrower is not cashAmountIn, but:

state.data.borrowAToken.transferFrom(msg.sender, borrower, cashAmountIn - fees);

Therefore, here the fees are again derived from an amount greater than the cash actually sent to the borrower.

3/ In getCashAmountIn:

fees = getSwapFee(state, cashAmountIn, tenor);

This is the same as the example above, as again the cash sent to the borrower is:

cashAmountIn - fees

Tools Used

Manual review

Recommended Mitigation Steps

Consider deriving the fees in getCreditAmountIn as follows:

fees = Math.mulDivUp(cashAmountOut * (PERCENT + swapFeePercent) / PERCENT, swapFeePercent, PERCENT);

Assessed type

Math

[c4-judge]

hansfriese marked the issue as not a duplicate

[hansfriese]

Duplicate of #288. Will apply 75% partial credit as it found 1 of 2 incorrect instances.

[c4-judge]

hansfriese changed the severity to 3 (High Risk)

[c4-judge]

hansfriese marked the issue as duplicate of #288

[c4-judge]

hansfriese marked the issue as partial-75