search

code-423n4 / 2022-09-nouns-builder-findings

10 stars 6 forks source link

ERC721.transferFrom() would always revert #640

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/main/src/lib/token/ERC721.sol#L134

Vulnerability details

Impact

Due to the if statement in line 134 of ERC721 contract, the transferFrom() call would always revert.

The if-statement would only pass when the owner of the token operatorApprovals to themself , however upon the call to _afterTokenTransfer , this would revert due to the check in ERC721Votes._moveDelegateVotes() that ensures there is no delegation to same from and to address.

Proof of Concept

https://github.com/code-423n4/2022-09-nouns-builder/blob/main/src/lib/token/ERC721.sol#L134

Tools Used

Manual review

Recommended Mitigation Steps

It would be quite easier to use a require check similar in the OZ ERC71 library in https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/ERC721.sol#L241

GalloDaSballo commented 2 years ago
Screenshot 2022-09-17 at 03 02 22

If the caller is tranfering, the call will pass per the first check being false, meaning no revert will happen