Lines of code
<https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/GeVault.sol#L330-L332&gt;

Vulnerability details

Impact

reentrancy attacks can result to stolen funds

Proof of Concept

The key issue is that removeFromAllTicks() calls removeFromTick(index) in a loop, which calls lendingPool.withdraw() and tr.withdraw(). These external calls could trigger a reentrant call back into the contract before the loop finishes.

The lendingPool.withdraw() or tr.withdraw() calls could trigger a reentrant call back into deposit() or withdraw() before the loop finishes withdrawing from all ticks. This could lead to inconsistent state or enabling reentrancy attacks.

Vulnerability arises because removeFromAllTicks() withdraws all tick assets before deposit and withdraw re-deposit them. This creates a reentrancy vulnerability - if deposit or withdraw is re-entered before re-depositing assets, the funds could be stolen:

• Attacker calls deposit or withdraw
• removeFromAllTicks() executes, draining all funds from the contract
• Before re-depositing funds, attacker makes a second call to deposit or withdraw
• Second call executes removeFromAllTicks() again before first call finishes re-depositing funds
• All funds are withdrawn a second time and sent to the attacker

Tools Used

Manual

Recommended Mitigation Steps

update state before making external calls.

Assessed type

Reentrancy

The text was updated successfully, but these errors were encountered:

All reactions