Lack of support for ERC1155 NFT #127
Labels
bug
Something isn't working
downgraded by judge
Judge downgraded the risk level of this issue
duplicate-77
grade-a
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Lines of code
https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/ClearingHouse.sol#L217
https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/CollateralToken.sol#L578
Vulnerability details
Impact
Lack of support for ERC1155 NFT so a vast amount of popular ERC1155 NFT cannot be used as collateral to borrow fund.
Proof of Concept
According to the documentation:
https://docs.astaria.xyz/docs/faq#what-nfts-will-i-be-able-to-borrow-against
The codebase use ERC721 safeTransferFrom extensively and assume that the underlyling NFT contract conforms to ERC721 standard.
In ClearingHouse.sol, the function below is used when flashAction is used
which calls the code below that use ERC721(tokenContract).safeTransferFrom
and in CollateralToken.sol the function onERC721Received hook, the code needs to transfer the NFT from collateralToken to clearning house.
However, there are popular NFT, that conform to ERC1155 standard,
https://etherscan.io/tokens-nft1155
which use safeTransferFrom in ERC1155 implementation and does not match ERC721 safeTransferFrom method call
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/d59306bd06a241083841c2e4a39db08e1f3722cc/contracts/token/ERC1155/ERC1155.sol#L114
Tools Used
Manual Review
Recommended Mitigation Steps
We recommend the protocol support ERC1155 transfer as well given the vast popular ERC1155 NFT in NFT community and NFT marketplace.
The text was updated successfully, but these errors were encountered: