Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improper function behaviour in ERC721Enumerable #69

Closed
code423n4 opened this issue Aug 25, 2022 · 1 comment
Closed

Improper function behaviour in ERC721Enumerable #69

code423n4 opened this issue Aug 25, 2022 · 1 comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working invalid This doesn't seem right sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-08-nounsdao/blob/452695d4764ba9d5e1d3eef0d5ecca3d004f215a/contracts/base/ERC721Enumerable.sol#L54-L56

Vulnerability details

Impact

One of the view functions will revert instead of returning false.

Proof of Concept

In package.json, the following lines show which version of openzeppelin is being used:

    "@openzeppelin/contracts": "^4.1.0",
    "@openzeppelin/contracts-upgradeable": "^4.1.0",

The function supportsInterface() in ERC721Enumerable.sol calls OZ's ERC165 supportsInterface(), which has a known vulnerability that causes the function to revert instead of returning false due to an abi.decode behaviour introduced in solidity's version 0.8

More details in the links below:
https://security.snyk.io/vuln/SNYK-JS-OPENZEPPELINCONTRACTS-2958047
OpenZeppelin/openzeppelin-contracts#3552

Tools Used

Manual code reading

Recommended Mitigation Steps

Update OpenZeppelin to version 4.7.1 or more recent.
@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Aug 25, 2022
code423n4 added a commit that referenced this issue Aug 25, 2022
@code423n4
rokinot issue #69
fac9792
@eladmallel
Copy link
Collaborator

the issue is incorrect; ERC721 calls ERC165.supportsInterface which performs a simple equality check, whereas the snyk issue linked refers to a different contract ERC165Checker.

@eladmallel eladmallel added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Aug 29, 2022
@gzeoneth gzeoneth added the invalid This doesn't seem right label Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working invalid This doesn't seem right sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@eladmallel @code423n4 @gzeoneth