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.

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

proveBatches() will revert on EVM mainnet #543

Closed
c4-submissions opened this issue Oct 22, 2023 · 5 comments
Closed

proveBatches() will revert on EVM mainnet #543

c4-submissions opened this issue Oct 22, 2023 · 5 comments
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 low quality report This report is of especially low quality sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-submissions
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/contracts/ethereum/contracts/zksync/facets/Executor.sol#L351

Vulnerability details

Impact

Function proveBatches() won't correctly work on EVM mainnet, due to assertion at line 351.
This would prevent functioning upon deployment of system causing the need of redeployment, thus it should qualify as an issue with Medium severity since its a sort of DoS.

Code4rena Severity Categorization

Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

In the current scenario, proveBatches() will revert on EVM mainnet. The protocol will be broken down, since proveBatches() won't be functioning on the EVM mainnet.

Proof of Concept

On Ethereum Mainnet, the black.chainid returns 1.
This implies, that below line of code will revert, making the whole proveBatches() not working as intendant.

File: ethereum/contracts/zksync/facets/Executor.sol

 assert(block.chainid != 1);

Whenever contract will be deployed on Ethereum Mainnet - proveBatches() will revert.

Tools Used

Manual code review

Recommended Mitigation Steps

Get rid of any code which was created for testing/development environment, before deploying contract to the EVM Mainnet.
Line assert(block.chainid != 1); should be removed.

Assessed type

DoS

@c4-submissions c4-submissions 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 Oct 22, 2023
c4-submissions added a commit that referenced this issue Oct 22, 2023
@c4-pre-sort c4-pre-sort added the low quality report This report is of especially low quality label Nov 1, 2023
@c4-pre-sort
Copy link

bytes032 marked the issue as low quality report

@miladpiri
Copy link

Invalid. The contract should be preprocessed before using it in production.

@c4-sponsor c4-sponsor added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Nov 8, 2023
@c4-sponsor
Copy link

miladpiri (sponsor) disputed

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Nov 15, 2023
@c4-judge
Copy link
Contributor

GalloDaSballo marked the issue as unsatisfactory:
Invalid

@GalloDaSballo
Copy link

As discussed in other issues, the Preprocessor has to be taken into account

@lsaudit lsaudit mentioned this issue Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to 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 low quality report This report is of especially low quality sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

6 participants