Submitted by kirk-baird, also found by reassor and sseefried
https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L324
https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L338
https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L344
https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L360
https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L436
https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L601
Both ERC20.transferFrom(address to, address from, uint256 amount) and ERC721.transferFrom(address to, address from, uint256 id) have the same function signature 0x23b872dd. The impact of this is its possible for baseAsset or erc20Assets to be ERC721 addresses rather than ERC20.
These functions will successfully transfer the NFT into the protocol however they will fail to transfer the NFT out of the contract. That is because the outgoing transfer is ERC20.safeTransfer() which calls transfer(to, amount) which does not match up with any valid function signatures on ERC721.
Therefore any ERC721 tokens transferred into the contract in this manner via fillOrder() will be permanently stuck in the contract as neither exercise() nor withdraw() will successfully transfer the tokens out of the contract.
ERC721.transferFrom()
ERC20.transferFrom()
Consider whitelisting approved ERC721 and ERC20 token contracts. Furthermore, separate these two contracts into different whitelists for ERC20s and ERC721s then ensure each contract is in the right category.
outdoteth (Putty Finance) acknowledged and commented:
hyh (warden) reviewed mitigation:
