Upgraded Q -> M from 131 [1654475029272] #339
Labels
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
duplicate
This issue or pull request already exists
upgraded by judge
Judge has assessed an item in Issue #131 as Medium risk. The relevant finding follows:
Recommend using safeTransferFrom() instead of transferFrom() for NFTs
The EIP-721 standard states:
Cally.sol uses transferFrom() to facillitate the transfer of ERC721 NFTs from the contract to users, as shown below.
In exercise():
Cally.sol:295 ? ERC721(vault.token).transferFrom(address(this), msg.sender, vault.tokenIdOrAmount)
In withdraw():
Cally.sol:344 ? ERC721(vault.token).transferFrom(address(this), msg.sender, vault.tokenIdOrAmount)
To prevent a permanent loss of NFTs, there should be checks in place to ensure msg.sender is capable of receiving NFTs. Otherwise, consider using safeTransferFrom() instead of transferFrom().
The text was updated successfully, but these errors were encountered: