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

0 stars 0 forks source link

Lack of validation for `cashAmountIn` in `BuyCreditMarket` when `exactAmountIn == false` allows creating a loan with only `1 AToken` fee #222

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/actions/BuyCreditMarket.sol#L170-L176

Vulnerability details

Impact

There is lack of validation for cashAmountIn in BuyCreditMarket when exactAmountIn == false. Since the fee is charged on the amount of swapped cash, the fee will be 1 AToken if the amount of swapped cash is 1 AToken. An attacker can exploit this to create an arbitrary loan with only 1 AToken fee by setting a high APR sell credit limit order, and then buying a desired amount of credit from himself via buy credit market order with exactAmountIn == false. This behavior could be escalated to two large impacts.

First impact

An attacker can DoS on market orders that using his credit.

  1. Bob (the attacker) sells credit to Alice, Alice has a credit position (id: A, amount: K)
  2. Bob creates an exact credit position to the credit position that he sold to Alice (same credit amount and same due date). Now, Bob has a credit position (id: B, amount: K)
  3. Alice uses sell credit market order to sell the credit position of id A to Candy
  4. Bob front-run Alice's sell credit market order with compensating his debt with the credit position of id B
  5. Since the credit position of id A has been repaid, Alice's sell credit market order will revert

Using the above exploit, Bob pays only 1 AToken fee in step 2. By compensating in step 4, Bob's debt to Alice was reduced. After the attack, Bob only owes Alice K credit from step 2.

Same idea can be applied to DoS buy credit market order.

Second impact

Users can trade credit with each other with only 1 AToken fee

First they need to setup a contract Market that have three functions:

Although the functionality of Market is limited when comparing to the protocol's market, because this market only supports the lender as a maker and the borrower as a taker, but the term of the loan could be negotiated by private messages.

Note that, the whole process is trustless. The lender can always cancel the buy credit limit to claim back USDC and WETH. The borrower is not exposed to any risk.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.23;

import {BaseTest} from "@test/BaseTest.sol";
import {YieldCurveHelper} from "@test/helpers/libraries/YieldCurveHelper.sol";
import {RESERVED_ID} from "@src/libraries/LoanLibrary.sol";
import {Vars} from "@test/BaseTest.sol";
import {console} from "forge-std/Test.sol";

contract POC is BaseTest {
    function test() public {
        _deposit(alice, weth, 200e18);
        _deposit(alice, usdc, 100e6);

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

        Vars memory _before = _state();

        _sellCreditLimit(alice, YieldCurveHelper.pointCurve(tenor, 1e60));
        _buyCreditMarket(alice, alice, RESERVED_ID, amount, tenor, false);

        Vars memory _after = _state();

        console.log("AToken before: %d", _before.alice.borrowATokenBalance);
        console.log("AToken after: %d", _after.alice.borrowATokenBalance);
        console.log("Credit after: %d", _after.alice.debtBalance);
    }
}

Logs:

AToken before: 100000000
AToken after: 99999999
Credit after: 20000000

Tools Used

Manual Review.

Recommended Mitigation Steps

Validate cashAmountIn in BuyCreditMarket when exactAmountIn == false.

if (params.exactAmountIn) {
    ...
} else {
    creditAmountOut = params.amount;
    (cashAmountIn, fees) = state.getCashAmountIn({
        creditAmountOut: creditAmountOut,
        maxCredit: params.creditPositionId == RESERVED_ID ? creditAmountOut : creditPosition.credit,
        ratePerTenor: ratePerTenor,
        tenor: tenor
    });
+   if (cashAmountIn < state.riskConfig.minimumCreditBorrowAToken) {
+       revert Errors.CREDIT_LOWER_THAN_MINIMUM_CREDIT(params.amount, state.riskConfig.minimumCreditBorrowAToken);
+   }
}

Assessed type

Invalid Validation

aviggiano commented 2 months ago

Invalid for the same reason as https://github.com/code-423n4/2024-06-size-findings/issues/223

hansfriese commented 2 months ago

Duplicate of #223 by the same warden.

c4-judge commented 2 months ago

hansfriese marked the issue as unsatisfactory: Invalid