code-423n4 / 2021-12-yetifinance-findings

0 stars 0 forks source link

Reentrancy in contracts/BorrowerOperations.sol #183

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

heiho1

Vulnerability details

Impact

There are several potential re-entrant functions in contracts/BorrowerOperations.sol:

=> Function addColl() on line 346 is potentially re-entrant as it is external but has no re-entrancy guard declared. This function invokes _adjustTrove() which potentially impacts user debt, collateral top-ups or withdrawals.

Same applies to -- withdrawColl() on line 373 -- withdrawYUSD() on line 389 -- repayYUSD() on line 406 -- adjustTrove() on line 420

=> Function openTrove() on line 207 is potentially re-entrant as it is external but has no re-entrancy guard declared. This function invokes _openTroveInternal() which potentially impacts trove creation, YUSD withdrawals and YUSD gas compensation.

=> Function closeTrove() on line 628 is potentially re-entrant as it is external but has no re-entrancy guard declared. This function invokes troveManagerCached.removeStake(msg.sender) and troveManagerCached.closeTrove(msg.sender) impacting outcomes like debt, rewards and trove ownership.

Proof of Concept

https://solidity-by-example.org/hacks/re-entrancy/

https://github.com/code-423n4/2021-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/BorrowerOperations.sol#L346

https://github.com/code-423n4/2021-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/BorrowerOperations.sol#L373

https://github.com/code-423n4/2021-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/BorrowerOperations.sol#L389

https://github.com/code-423n4/2021-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/BorrowerOperations.sol#L420

https://github.com/code-423n4/2021-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/BorrowerOperations.sol#L207

https://github.com/code-423n4/2021-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/BorrowerOperations.sol#L628

Tools Used

Slither

Recommended Mitigation Steps

Potential solution is a re-entrancy guard similar to https://docs.openzeppelin.com/contracts/4.x/api/security#ReentrancyGuard

kingyetifinance commented 2 years ago

@LilYeti: Duplicate with #57

alcueca commented 2 years ago

Taking as main