hats-finance / Wise-Lending-0xa2ca45d6e249641e595d50d1d9c69c9e3cd22573

0 stars 1 forks source link

Nyxaris (H-2) Potential Re entrancy in _manuallyWithdrawShares function in PendlePowerFarm.sol #57

Open olaoyesalem opened 8 months ago

olaoyesalem commented 8 months ago

Summary

the _manuallyWithdrawShares function in the contract is vulnerable to reentrancy due to performing an external call to withdraw tokens before completing all state changes. This vulnerability arises because the function makes an external call to WISE_LENDING.withdrawExactShares without fully updating the contract's state beforehand. As a result, the contract is left in an incomplete state during the external call, creating an opportunity for attackers to exploit and manipulate the contract's state in unintended ways.

Proof Of Code

function _manuallyWithdrawShares(uint256 _nftId, uint256 _withdrawShares) internal {
    // Effects: Calculate withdrawal amount
    uint256 withdrawAmount = WISE_LENDING.cashoutAmount(PENDLE_CHILD, _withdrawShares);

    // Effects: Perform withdrawal
    withdrawAmount = WISE_LENDING.withdrawExactShares(_nftId, PENDLE_CHILD, _withdrawShares);

    // Effects: Transfer tokens after all state changes
    _safeTransfer(PENDLE_CHILD, msg.sender, withdrawAmount);
}

Recommendations:

To mitigate the reentrancy vulnerability, follow these recommendations:

Use the Checks-Effects-Interactions pattern: Ensure that all state changes are completed before interacting with external contracts to prevent reentrancy attacks.

Perform state changes first: Calculate the withdrawal amount and update contract state before making external calls to avoid leaving the contract in an inconsistent state.

vm06007 commented 8 months ago

reentrancy only can happen if the calling contract recalls the target, in our case it is impossible, WISE_LENDING is not an external contract that malicious actor can modify.

Also similar questions about malicious tokens are out of scope

olaoyesalem commented 8 months ago

https://github.com/hats-finance/Wise-Lending-0xa2ca45d6e249641e595d50d1d9c69c9e3cd22573/issues/59