Reentrancy Vulnerability in paybackExactAmountETH Function which lead to Loss of Funds

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L1088

Vulnerability details

Impact

Detailed description of the impact of this finding. The paybackExactAmountETH function allows users to pay back a specific amount of ETH borrowed against a given NFT. It wraps the ETH and performs the payback operation. However, it is vulnerable to reentrancy attacks because it performs state changes after sending ETH to an external contract, which can then recursively call back into the vulnerable contract before the state changes are completed.

An attacker could exploit this vulnerability to repeatedly call the paybackExactAmountETH function before the state changes are finalized, effectively draining the contract's ETH balance.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. // Attacker contract contract Attacker { WiseLending lendingContract;

constructor(address _lendingContract) {
    lendingContract = WiseLending(_lendingContract);
}

// Function to perform reentrant attack
function attack(uint256 _nftId) external payable {
    // Send some initial ETH to trigger the paybackExactAmountETH function
    lendingContract.paybackExactAmountETH{value: msg.value}(_nftId);

    // Re-enter the paybackExactAmountETH function while it's still executing
    lendingContract.paybackExactAmountETH{value: msg.value}(_nftId);
}

}

Tools Used

Manual Review

Recommended Mitigation Steps

Implement the checks-effects-interactions pattern to ensure that state changes are performed before interacting with external contracts. Use reentrancy guards to prevent recursive calls from within the same function. Consider using the withdrawal pattern to separate state changes from sending funds. Additionally, consider using the nonReentrant modifier from OpenZeppelin's ReentrancyGuard library to protect vulnerable functions from reentrancy attacks.

Assessed type

Reentrancy

[vm06007]

This is invalid submission.

Whoever submitted this did not pay attention, that the reentrancy check is present on all functions with syncPool modifier if ETH is being sent from the contract then a flag sendingInProgress is set to true and any attempt to call another function from attacker contract once ETH is received would be stopped by _checkReentrancy() function in lendingContract

please make sure you can read code well and look into our implementation of _checkReentrancy() in the code base

Dismissed.

[c4-pre-sort]

GalloDaSballo marked the issue as insufficient quality report

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid