ERC-721: Check ownership in transfer_token_from#2093
Merged
ascjones merged 3 commits intouse-ink:masterfrom Feb 5, 2024
Merged
ERC-721: Check ownership in transfer_token_from#2093ascjones merged 3 commits intouse-ink:masterfrom
transfer_token_from#2093ascjones merged 3 commits intouse-ink:masterfrom
Conversation
transfer_fromtransfer_token_from
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cargo-contractorpallet-contracts?The ERC-721 Specification states that
transferFromshould throw iffromis not the owner of the token. The current implementation doesn't check that. As a result, it's possible for a callerAwho is allowed to transferB's tokenT, can calltransferFrom(C, A, T)without any problem. A side effect is that the balance ofCwill be decremented instead ofB.The
transfer()function also has a similar issue: it seems the intention is to transfer the token from the caller, but the ownership is never checked explicitly. If the caller uses the id of an approved (not owned) token, it will incorrectly decrement the caller's balance.This PR addresses these issues by adding an ownership check in
transfer_token_from. Tests fortransfer_fromandtransferhave been added.Checklist before requesting a review
CHANGELOG.md