Open olaoyesalem opened 6 months ago
We have modifiers preventing use of malicious tokens as input variables in syncpool and isValidPool and thus only possible if malicious token is added. But that means centralized party made a mistake and thus its out of scope since we mentioned such things to be out of scope. Therofr its invalid
Description:
The
enterFarm
first calls the internal function _getWiseLendingNFT to obtain a WiseLending NFT ID, which constitutes a state change. Subsequently, the function makes an external call to _safeTransferFrom to transfer tokens from the caller to the contract. The critical issue lies in the fact that the state change (obtaining the NFT ID) occurs before the external call, potentially leaving the contract vulnerable to reentrancy attacks.Impact
If the external call to
_safeTransferFrom
triggers a malicious contract that reenters theenterFarm
function before the state changes are completed, it could exploit this vulnerability to manipulate the contract's state or perform unauthorized actions. For instance, an attacker's contract could recursively callenterFarm
, taking advantage of the incomplete state changes to manipulate the NFT ID or other critical data. This could lead to fund theft, unauthorized NFT manipulation.Recommendation
To mitigate this vulnerability, it is crucial to reorder the sequence of operations within the
enterFarm
function to ensure that all state changes are completed before any external calls are made. Additionally, implementing a reentrancy guard, such as the one provided by OpenZeppelin, is recommended to prevent reentrancy attacks and enhance the overall security of the function. By addressing this specific reentrancy vulnerability and implementing recommended security measures, the smart contract can significantly enhance its resilience against exploitation and mitigate the potential impact outlined above.