search

Lodestar-Finance / lodestar-protocol

Houses the code for the Lodestar Finance DeFi protocol.
BSD 3-Clause "New" or "Revised" License
10 stars 7 forks source link

reentrancy can be possible to avoid liquidation #18

Closed rajatbeladiya closed 1 year ago

rajatbeladiya commented 1 year ago

Affected Contracts

Comptroller.sol

Severity

High

Description

https://github.com/LodestarFinance/lodestar-protocol/blob/cfca1ae275d023a02198798bbcb24b2a1f646776/.circleci/contracts/Comptroller.sol#L473-L509

liquidateBorrowAllowed() used to check position is can be liquidated or not.

https://github.com/LodestarFinance/lodestar-protocol/blob/cfca1ae275d023a02198798bbcb24b2a1f646776/contracts/CErc20.sol#L140-L147

Suppose,

Alice has two following borrowed tokens tokenA, tokenB and two following collateral deposits tokenC, tokenD

tokenA is ERC777 contract with callback,

  1. the attacker calls liquidateBorrow() on cA with collateral cC. liquidateBorrowedAllowed() return liquidation is allowed for short time span.
  2. the token transferFrom() of A is triggered and callback to the attacker will executed.
    1. as part of the callback, the attacker calls liquidateBorrow() on cB with collateral cD. liquidateBorrowAllowed will be evaluated as a small shortfall and liquidation is allowed.
    2. the biggest possible amount of B tokens is repaid and cD tokens are received as reward. the position of Alice is now safe again.
  3. despite the position being safe, the original liquidation continues and the biggest possible amount of A tokens is liquidated to received cC as reward.

impact can be large for ERC777 tokens, for reference checkout this, https://twitter.com/peckshield/status/1432249600002478081

Recommendation

Add reentrancy guard

0xAppo commented 1 year ago

liquidateBorrowInternal has the reentrancy guard applied to it.