search

crytic / properties

Pre-built security properties for common Ethereum operations
GNU Affero General Public License v3.0
276 stars 42 forks source link

[Bug-Candidate]: Difference in ERC721BurnableProperties for external vs. internal #58

Open engn33r opened 4 months ago

engn33r commented 4 months ago

Describe the issue:

CryticERC721ExternalBurnableProperties contains 5 tests while CryticERC721BurnableProperties contains 6 tests. This means that the list of 19 property tests for ERC721 is actually only 18 property tests for the external tests. Either a test is forgotten in the external tests or the list of property tests should differ for external vs internal.

Steps to reproduce the issue:

The external tests only have one test test_ERC721_external_burnRevertOnTransfer while the internal tests have test_ERC721_burnRevertOnTransferFromPreviousOwner and test_ERC721_burnRevertOnTransferFromZeroAddress.

If additional code is needed for reproducing, please copy it here, or drop us a link to the repository:

No response

Echidna version:

2.2.3

Additional information:

No response

tuturu-tech commented 4 months ago

Hey @engn33r, good catch! I believe test_ERC721_external_burnRevertOnTransfer and test_ERC721_burnRevertOnTransferFromPreviousOwner are essentially testing for the same thing so this is probably just an issue of inconsistent naming, but test_ERC721_burnRevertOnTransferFromZeroAddress is definitely missing from the external properties and should be added.