Submission Details
Severity:
Missing `safeTransferFrom` in `HorseStore.huff` allows transferring to non-compatible contracts
Summary
Missing safeTransferFrom in HorseStore.huff will lead to loss of nft if accidently sent to a address that does not support ERC721
Vulnerability Details
HorseStore.sol has safeTransferFrom function which make sures that receiver of nft is valid that support ERC721. As it's helpful to prevent sending NFT
to a contract, from it can't be taken out. HorseStore.huff should follow the same methods as well, but it's not there, This will allow user to send nft to any address, irrespective of it's support for ERC721.
Impact
Nft sent to a contract / address which is not compatible with ERC721, Will lead to loss of that nft.
Tools Used
Manulal Review
Recommendations
Add following code in existing HorseStore.huff to mitigate this issue
/* Imports */
#include "../lib/huffmate/src/data-structures/Hashmap.huff"
#include "../lib/huffmate/src/utils/CommonErrors.huff"
........
+ #define function safeTransferFrom(address,address,uint256) nonpayable returns ()
+ #define function safeTransferFrom(address,address,uint256,bytes) nonpayable returns ()
//// Existing code
+ /// @notice Safe Transfer From
+ #define macro SAFE_TRANSFER_FROM() = takes (0) returns (0) {
+ // Setup the stack for the transfer function.
+ 0x44 calldataload // [tokenId]
+ 0x24 calldataload // [to, tokenId]
+ 0x04 calldataload // [from, to, tokenId]
+ TRANSFER_TAKE_FROM() // [from, to, tokenId]
+ TRANSFER_GIVE_TO() // [from, to, tokenId]
+ // Emit the transfer event
+ __EVENT_HASH(Transfer) // [sig, from, to, tokenId]
+ 0x00 0x00 log4 // []
+ // Make sure we can transfer to the recipient
+ 0x24 calldataload // [to]
+ dup1 extcodesize // [to.code.length, to]
+ iszero safe jumpi // [to]
+ // onERC721Received Selector
+ 0x150b7a02 dup1 // [onERC721Received, onERC721Received, to]
+ 0xE0 shl // [onERC721Received_shifted, onERC721Received, to]
+ // Store the left-shifted selector for call
+ 0x20 mstore // [onERC721Received, to]
+ // Store the msg.sender as the first arg
+ caller 0x24 mstore // [onERC721Received, to]
+ // Store from as the second arg
+ 0x04 calldataload // [from, onERC721Received, to]
+ 0x44 mstore // [onERC721Received, to]
+ // Id is the third arg
+ 0x44 calldataload // [tokenId, onERC721Received, to]
+ 0x64 mstore // [onERC721Received, to]
+ // Blank bytes array as 4th arg (no data)
+ 0x80 0x84 mstore
+ 0x00 0xA4 mstore
+ // Call address(to).onERC721Received(msg.sender, from, tokenId, "")
+ 0x20 // [retSize, onERC721Received, to]
+ 0x00 // [retOffset, retSize, onERC721Received, to]
+ 0xA4 // [argSize, retOffset, retSize, onERC721Received, to]
+ dup3 // [argOffset, argSize, retOffset, retSize,
+ onERC721Received, to]
+ dup3 // [value, argOffset, argSize, retOffset, retSize,
+ onERC721Received, to]
+ dup7 // [to, value, argOffset, argSize, retOffset, retSize,
+ onERC721Received, to]
+ gas // [gas, to, value, argOffset, argSize, retOffset, retSize,
+ onERC721Received, to]
+ call // [success, onERC721Received, to]
+ // Revert if call isn't successful
+ cont jumpi // [onERC721Received, to]
+ 0x00 dup1 revert
+ cont:
+ // Compare the return data to the onERC721Received selector
+ 0x00 mload 0xE0 shr // [response, onERC721Received, to]
+ eq safe jumpi // [to]
+ // Revert if the return data is not accepted
+ UNSAFE_RECIPIENT(0x00)
+ // Stop execution if safe
+ safe:
+ stop
+ }
+ #define macro SAFE_TRANSFER_FROM_WITH_DATA() = takes (0) returns (0) {
+ // Setup the stack for the transfer function.
+ 0x44 calldataload // [tokenId]
+ 0x24 calldataload // [to, tokenId]
+ 0x04 calldataload // [from, to, tokenId]
+ TRANSFER_TAKE_FROM() // [from, to, tokenId]
+ TRANSFER_GIVE_TO() // [from, to, tokenId]
+ // Emit the transfer event.
+ __EVENT_HASH(Transfer) // [sig, from, to, tokenId]
+ 0x00 0x00 log4 // []
+ // Make sure we can transfer to the recipient
+ 0x24 calldataload // [to]
+ dup1 extcodesize // [to.code.length, to]
+ iszero safe jumpi // [to]
+ // onERC721Received Selector
+ 0x150b7a02 dup1 // [onERC721Received, onERC721Received, to]
+ 0xE0 shl // [onERC721Received_shifted, onERC721Received, to]
+ // Store the left-shifted selector for call
+ 0x20 mstore // [onERC721Received, to]
+ // Store the msg.sender as the first arg
+ caller 0x24 mstore // [onERC721Received, to]
+ // Store from as the second arg
+ 0x04 calldataload // [from, onERC721Received, to]
+ 0x44 mstore // [onERC721Received, to]
+ // Id is the third arg
+ 0x44 calldataload // [tokenId, onERC721Received, to]
+ 0x64 mstore // [onERC721Received, to]
+ 0x84 calldataload // [len(data), onERC721Received, to]
+ 0x05 shl // [len(data) * 0x20, onERC721Received, to]
+ 0x40 add // [len(data) * 0x20 + 0x40, onERC721Received, to]
+ dup1 // [len(data) * 0x20 + 0x40, len(data) * 0x20 + 0x40,
+ onERC721received, to]
+ 0x64 // [0x64, len(data) * 0x20 + 0x40, len(data) * 0x20 + 0x40,
+ onERC721received, to]
+ 0x84 // [0x20, 0x64, len(data) * 0x20 + 0x40, len(data) * 0x20 +
+ 0x40, onERC721received, to]
+ calldatacopy // [len(bytes), onERC721received, to]
+ // Call address(to).onERC721Received(msg.sender, from, tokenId, bytes)
+ 0x20 // [retSize, len(bytes), onERC721Received, to]
+ 0x00 // [retOffset, retSize, len(bytes), onERC721Received, to]
+ swap1 swap2 // [len(bytes), retOffset, retSize, onERC721Received, to]
+ 0x64 add // [argSize, retOffset, retSize, onERC721Received, to]
+ dup3 // [argOffset, argSize, retOffset, retSize, len(bytes),
+ onERC721Received, to]
+ dup3 // [value, argOffset, argSize, retOffset, retSize, len(bytes),
+ onERC721Received, to]
+ dup7 // [to, value, argOffset, argSize, retOffset, retSize,
+ len(bytes), onERC721Received, to]
+ gas // [gas, to, value, argOffset, argSize, retOffset, retSize,
+ len(bytes), onERC721Received, to]
+ call // [success, len(bytes), onERC721Received, to]
+ // Revert if call isn't successful
+ cont jumpi // [len(bytes), onERC721Received, to]
+ 0x00 dup1 revert
+ cont:
+ // Compare the return data to the onERC721Received selector
+ 0x00 mload 0xE0 shr // [response, onERC721Received, to]
+ eq safe jumpi // [to]
+ // Revert if the return data is not accepted
+ UNSAFE_RECIPIENT(0x00)
+ // Stop execution if safe
+ safe:
+ stop
+ }
/// >>>>>>>>>>>>>>>>>>>>> INTERNAL FUNCTIONS <<<<<<<<<<<<<<<<<<<<<< ///
.........
////Existing code
Updates
Lead Judging Commences
inallhonesty over 1 year ago
Submission Judgement Published Assigned finding tags:
Components of ERC721 not properly (or at all) implemented in HUFF
Rewards breakdown
nSLOC
460This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.