Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Due to interest rates update method, Interest-Free Loans are possible and the Cost of DoS are reduced #435

Open
c4-bot-4 opened this issue Mar 15, 2024 · 13 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue M-04 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons sufficient quality report This report is of sufficient quality

Comments

@c4-bot-4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L1150-L1165

Vulnerability details

Impact

Allowing in interest free debt in 1 block could have several unwanted results:

  • Allowing for in same block (not necessarly same transaction) interest-free loans, could be abused by wales for arbitrage operations, resulting in protocol users unable to borrow because of the daily limit.
  • DoS attacks, with the new updates on the main net resulting in way lower transaction fees, a whale account with a big position could borrow a big amount of money and then repay it in the same block, resulting in him not paying any interest, and users unable to borrow in this block. The attack will then be repeated in each block resulting in DoS

=> The attack would result not only in DoS, but also the protocol Liquidity Providers (LPs), and the protocol would lose potential interest payments for their deposits.

  • Note: Please also note, that in L2 blockchains, it is quite common for multiple L2 Blocks to have the same block.timestamp. So borrower could potentially have interest-free debt on the span of multiple blocks.

Proof of Concept

To prove the severance of the problems, we want to first demonstrate the math around the cost of a 24 hours DoS attack,and second we want to demonstrate through a coded PoC, that when borrowing in the same block, there are no fees to be paid.
24 hours DoS costs:
Because the debt is interest free (see next step, for PoC) the cost of a 24 hours DoS attack is the cost of the gas to borrow and repay the loan. Due to the new changes deployed to Ethereum (Ethereum's Dencun upgrade) the transactions fees on L2 like arbitrum, have massively decreased, resulting in a cost of around 0.1$ per transaction. Right now on arbitrum, on average a block is minted each 15 seconds, resulting in 4 blocks per minute, 240 blocks per hour, and 5760 blocks per day. The cost of a single attack in a single block is (0.1 * 2) + 0.2~0.5 usd for the first borrow() to front run the other transactions in the block. The cost of a 24 hours DoS attack is then between 5760 * (0.1 * 2 + 0.2) =2304 usd and 5760 * (0.1 * 2 + 0.5)= 4032 usd. This would allow a whale whith a sufficient LP position that he can use as collateral for the 10% of the Lenders deposits (which is maximum daily borrowing limit). Specially in the early days of the protocol, this doesn't necessary need to be a lot. A realistic scenario would look like this:

  • Total Users Deposits 10 million USDC
  • Attacker has an LP position valued at 1.3 million USDC
  • Daily borrowing limit is 10% of the total deposits, so 1 million USDC
  • Attack Budget for attacker ~ 5000 usd
  • The attacker first borrows 1 million USDC at the start of the block (frontrun), and repay it in the same block (backrun), resulting in no interest to be paid, and users unable to borrow in the same period.
  • The attacker goes on and repeats the attack repeatedly for 24 hours.

Proof that no fees are paid when borrowing and repaying in the same block
It is intended by the protocol developpers to only update interest rates once per block, and not for each transaction.
This design choice could be shown in the _updateGlobalInterest() method

function _updateGlobalInterest()
        internal
        returns (uint256 newDebtExchangeRateX96, uint256 newLendExchangeRateX96)
    {
        // only needs to be updated once per block (when needed)
        if (block.timestamp > lastExchangeRateUpdate) {
            (newDebtExchangeRateX96, newLendExchangeRateX96) = _calculateGlobalInterest();
            lastDebtExchangeRateX96 = newDebtExchangeRateX96;
            lastLendExchangeRateX96 = newLendExchangeRateX96;
            lastExchangeRateUpdate = block.timestamp;
            emit ExchangeRateUpdate(newDebtExchangeRateX96, newLendExchangeRateX96);
        } else {
            newDebtExchangeRateX96 = lastDebtExchangeRateX96;
            newLendExchangeRateX96 = lastLendExchangeRateX96;
        }
    }

As we can see, from the inline comment, it is the intention of the protocol developpers to only update the interest rates once per block, and not for each transaction.
To showcase that an interest free debt is possible in the same block, please add the following test to test/integration/V3Vault.t.sol:

    function testInterestFreeDebt() external {
        // @audit initialize Vault
        vm.startPrank(TEST_NFT_ACCOUNT_2);
        USDC.approve(address(vault), 10000000);
        vault.deposit(1000000, TEST_NFT_ACCOUNT_2);
        vm.stopPrank();
        // @audit PoC 
        vm.startPrank(TEST_NFT_ACCOUNT);
        NPM.approve(address(vault), TEST_NFT);
        vault.create(TEST_NFT,TEST_NFT_ACCOUNT);
        console.log("balance of user before borrow",USDC.balanceOf(TEST_NFT_ACCOUNT));
        vault.borrow(TEST_NFT, 100000);
        console.log("balance of user after borrow",USDC.balanceOf(TEST_NFT_ACCOUNT));
        USDC.approve(address(vault), 100000);
        vault.repay(TEST_NFT, 100000, false);
        console.log("balance of user after repay",USDC.balanceOf(TEST_NFT_ACCOUNT));
        // @audit assert that debt paid in full
        assertTrue(NPM.ownerOf(TEST_NFT)==TEST_NFT_ACCOUNT);
    }

Result:

Running 1 test for test/integration/V3Vault.t.sol:V3VaultIntegrationTest
[PASS] testInterestFreeDebt() (gas: 542028)
Logs:
  balance of user before borrow 0
  balance of user after borrow 100000
  balance of user after repay 0

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 418.62ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests

Tools Used

Foundry, manual review

Recommended Mitigation Steps

The most simple solution to this issue, is to add a small borrow fee (percentagewise for e.g 0.1% of borrowed debt). This way even if arbitrageurs try to do swaps, or attackers try to DoS the system, Liquidity Providers will receive their fair yield (potentially a lot more yield if an attacker tries the DoS Attack described in the this report).

Assessed type

Context

@c4-bot-4 c4-bot-4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Mar 15, 2024
c4-bot-1 added a commit that referenced this issue Mar 15, 2024
@c4-pre-sort
Copy link

0xEVom marked the issue as duplicate of #283

@c4-pre-sort
Copy link

0xEVom marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Mar 21, 2024
@c4-pre-sort c4-pre-sort reopened this Mar 21, 2024
@c4-pre-sort
Copy link

0xEVom marked the issue as not a duplicate

@c4-pre-sort c4-pre-sort added primary issue Highest quality submission among a set of duplicates and removed duplicate-283 labels Mar 21, 2024
@c4-pre-sort
Copy link

0xEVom marked the issue as primary issue

@0xEVom
Copy link

0xEVom commented Mar 25, 2024

Medium severity seems more appropriate.

@kalinbas
Copy link

"Allowing for in same block (not necessarly same transaction) interest-free loans, could be abused by wales for arbitrage operations, resulting in protocol users unable to borrow because of the daily limit." - if the whale borrow is repayed in the same block - the limit is reset. So other users can borrow in the next block.

About the DOS attack: There is a way to disable this attack by increasing the dailyLimitMinValue. The probability of someone attacking like this seems very low, so we are comfortable with this workaround.

@c4-sponsor
Copy link

kalinbas (sponsor) acknowledged

@c4-sponsor c4-sponsor added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Mar 27, 2024
@jhsagd76
Copy link

I do not consider this to be a high issue; the first impact is false. Regarding the second, a DoS, the attacker would suffer huge losses without gaining anything.

But I still believe the issue is valid because MEV bots have enough incentive to hold debt from the vault over multiple blocks(one by one, borrow in the index 0 tx and repay in the last index tx), which could actually lead to a deterioration in the protocol's reliability.

@c4-judge
Copy link

jhsagd76 changed the severity to 2 (Med Risk)

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Mar 31, 2024
@c4-judge
Copy link

jhsagd76 marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Mar 31, 2024
@c4-judge
Copy link

c4-judge commented Apr 1, 2024

jhsagd76 marked the issue as selected for report

@c4-judge c4-judge added the selected for report This submission will be included/highlighted in the audit report label Apr 1, 2024
@lanrebayode
Copy link

Hello @jhsagd76
Thanks form judging. I think the severity of this should be reconsidered due to it impact.

Due to update mode, user can get loans without interest as long as repayment is done in the same transaction, this is the entire basis for Flashloan which also comes at a cost in popular lending platforms like AAVE and UNISWAP.

Since this action can be repeated overtime, protocol will be losing a lot as unclaimed interest fee, which would have made more funds to the LPs and protocol.

Since loss of funds(fee) is evident, it's valid as a high severity.

@jhsagd76
Copy link

jhsagd76 commented Apr 3, 2024

There is no technical disagreement, the attack you mentioned has already been described in my comments above, maintain M for cost and likelihood of attack

@C4-Staff C4-Staff added the M-04 label Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue M-04 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

9 participants