sherlock-audit / 2023-07-blueberry-judging

2 stars 1 forks source link

feelereth - Front-running vulnerability in the liquidate function #81

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

feelereth

high

Front-running vulnerability in the liquidate function

Summary

As liquidate involves multiple transactions, an attacker could watch for pending liquidations and front run to steal some collateral

Vulnerability Detail

The key issue is that liquidate makes multiple state-changing transactions in sequence:

  1. It calls _repay to repay part of the debt and update the shares.
  2. It transfers the liquidated collateral to the msg.sender.
  3. It transfers the liquidated underlying vault shares to the msg.sender. An attacker could watch for pending liquidate calls, and quickly call takeCollateral after step 1 and before steps 2-3 to steal some collateral. For example:
  4. Alice calls liquidate on Bob's undercollateralized position. This is broadcast to the pending transaction pool.
  5. Eve sees Alice's liquidate transaction in the pending tx pool.
  6. Eve quickly calls takeCollateral to remove some of Bob's collateral before Alice's liquidate completes.
  7. When Alice's liquidate finishes steps 2-3, some collateral has already been taken.

Impact

An attacker can steal collateral rewards

Code Snippet

https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/BlueBerryBank.sol#L544-L628

Tool used

Manual Review

Recommendation

Use a "state" variable to gate access to sensitive logic

sherlock-admin2 commented 1 year ago

3 comment(s) were left on this issue during the judging contest.

shogoki commented:

Transactions are atomic

0xyPhilic commented:

invalid because takeCollateral can't be called by external third person

Kral01 commented:

this is not the issue