code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/FixedPrice.sol#L110 https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/OpenEdition.sol#L122

Vulnerability details

Impact

selfdestruct() will not be available after EIP-4758. This EIP will rename the SELFDESTRUCT opcode and replace its functionality. It will no longer destroy code or storage, so, the contract still will be available. In this case it will break the logic of the project because it will not work as aspected:

FixedPrice.sol

After call the cancel() function the contract still will be available. Users will be able to buy a number of NFTs even if the current sale is cancelled.

OpenEdition.sol

After call the cancel() function the contract still will be available. Users will be able to buy a number of NFTs even if the current sale is cancelled.

Proof of Concept

According to EIP-4758:

The SELFDESTRUCT opcode is renamed to SENDALL, and now only immediately moves all ETH in the account to the target; it no longer destroys code or storage or alters the nonce.
All refunds related to SELFDESTRUCT are removed.

Tools Used

Manual Review

Recommended Mitigation Steps

The architecture should be changed to avoid that problem.

c4-judge commented 1 year ago

berndartmueller marked the issue as primary issue

stevennevins commented 1 year ago

If the storage is still in tact, then after this point the start and end time would prevent buys from occurring. But yeah I agree we should move away from selfdestruct all together

c4-sponsor commented 1 year ago

stevennevins marked the issue as disagree with severity

stevennevins commented 1 year ago

Suggesting Low

berndartmueller commented 1 year ago

This finding demonstrates an issue regarding the EIP-4758, which makes it impossible to cancel an active sale. Thus, I consider Medium severity to be appropriate as the functionality of the protocol is impacted.