removeFromAllTicks() withdraws all tick assets before deposit and withdraw re-deposit them creates a reentrancy attacks.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/GeVault.sol#L313-L317 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/GeVault.sol#L330-L332

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

Assessed type

Reentrancy

[c4-pre-sort]

raymondfam marked the issue as low quality report

[raymondfam]

OOS. Wrong contest submission.

[c4-judge]

HickupHH3 marked the issue as unsatisfactory: Out of scope