howlbot-integration[bot] commented 2 months ago

Lines of code

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

Vulnerability details

Given below a simple use case of SellCreditMarket.

Alice deposits 10,000 USDC cash and submits a BuyCreditLimit offer for lending 10 percent yearly
Bob deposits Collateral of 2ETH and picks up Alice's offer with SellCreditMarket(Alice, RESERVED_ID, 1000e6, 365 days) to borrow 1,000 USDC for 1 year. Bob now owns a debt position of 1,100 future value due 365 days from now, and his debtToken should be incremented by the same amount. The cashAmountOut to Bob is 1,000 in BorrowATokens, and with a SwapFeeAPR of 0.5 percent, the SwapFee payable to the protocol is 5

In the current implementation, the SwapFee of 5 is transferred from Alice/Lender's account immediately, So assuming that this is also payable by dueDate with same interest rate, the updated future value of debt position SHOULD be 1105.5 calculated from the formula

  Math.mulDivUp((cashAmountOut + fees), (PERCENT + ratePerTenor), PERCENT)

But as per the current implementation, the future value is 1105.527639, which is higher due to below formula being used in the code.

  Math.mulDivUp(cashAmountOut, (PERCENT + ratePerTenor), (PERCENT - state.getSwapFeePercent(tenor)))

The behaviour of wrong calculation is also present if exactAmountIn value is True in SellCreditMarket.

Impact

The borrower is actually paying more to the lender, in absolute terms, due to the wrong formula used to calculate. The difference may be a low dust amount for a single transaction as shown above, but over long term and aggregating all the borrow transactions it will be significant enough not to ignore.

Proof of Concept

Given below the foundry test case with logs showing the values observed.

TESTFILE : test/local/actions/SellCreditMarketBug.t.sol
// SPDX-License-Identifier: MIT
pragma solidity 0.8.23;

import {RESERVED_ID} from "@src/libraries/LoanLibrary.sol";
import {BaseTest} from "@test/BaseTest.sol";
import {Vars} from "@test/BaseTest.sol";
import {CreditPosition, DebtPosition} from "@src/libraries/LoanLibrary.sol";
import {PERCENT, YEAR} from "@src/libraries/Math.sol";
import {YieldCurveHelper} from "@test/helpers/libraries/YieldCurveHelper.sol";
import {console2 as console} from "forge-std/console2.sol";
import {Math} from "@src/libraries/Math.sol";

contract SellCreditMarketBugTest is BaseTest {

    function test_SellCreditMarket_sellCreditMarket_debtPosition_futureValue() public {
        _deposit(alice, usdc, 10000e6);
        _deposit(bob, weth, 2e18);
        _buyCreditLimit(alice, block.timestamp + 365 days, YieldCurveHelper.pointCurve(365 days, 0.1e18));

        Vars memory _before = _state();

        uint256 amount = 1000e6;
        uint256 tenor = 365 days;

        uint256 debtPositionId = _sellCreditMarket(bob, alice, RESERVED_ID, amount, tenor, false);

        uint256 swapFee = size.getSwapFee(amount, tenor);

        uint256 futureValue = Math.mulDivUp((amount + swapFee), (PERCENT + 0.1e18), PERCENT);

        Vars memory _after = _state();

        console.log("Alice's borrowAToken Reduced by    : ", _before.alice.borrowATokenBalance-_after.alice.borrowATokenBalance);
        console.log("Bob's borrowAToken Balance         : ", _after.bob.borrowATokenBalance);
        console.log("Fee Recipient borrowAToken Balance :    ", _after.feeRecipient.borrowATokenBalance);
        console.log("Swap Fee                           :    ", swapFee);
        console.log("Bob's debtPosition.futureValue     : ", size.getDebtPosition(debtPositionId).futureValue);
        console.log("Correct futureValue                : ", futureValue);

        assertEq(_after.alice.borrowATokenBalance, _before.alice.borrowATokenBalance - amount - swapFee);
        assertEq(_after.bob.debtBalance, size.getDebtPosition(debtPositionId).futureValue);
    }

}

Output from the above test

[PASS] test_SellCreditMarket_sellCreditMarket_debtPosition_futureValue() (gas: 1441752)
Logs:
  Alice's borrowAToken Reduced by    :  1005000000
  Bob's borrowAToken Balance         :  1000000000
  Fee Recipient borrowAToken Balance :     5000000
  Swap Fee                           :     5000000
  Bob's debtPosition.futureValue     :  1105527639
  Correct futureValue                :  1105500000

Tools Used

Manual review , Foundry

Recommended Mitigation Steps

There are multiple ways to resolve this calculation issue. One of the way is to change in function getCreditAmountIn() of AccountingLibrary.sol Assign the correct formula to creditAmountIn instead of maxCredit,

        if (cashAmountOut == maxCashAmountOut) {
            // no credit fractionalization

 ---           creditAmountIn = maxCredit;
            fees = Math.mulDivUp(cashAmountOut, swapFeePercent, PERCENT);
 +++           creditAmountIn = Math.mulDivUp((cashAmountOut + fees), (PERCENT + ratePerTenor), PERCENT)
        }

Assessed type

Other

c4-judge commented 2 months ago

hansfriese changed the severity to 3 (High Risk)