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

elhaj - PUSH0 opcode Is Not Supported on Linea yet #79

Open
sherlock-admin4 opened this issue Jun 6, 2024 · 15 comments
Open

elhaj - PUSH0 opcode Is Not Supported on Linea yet #79

sherlock-admin4 opened this issue Jun 6, 2024 · 15 comments
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Won't Fix The sponsor confirmed this issue will not be fixed

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Jun 6, 2024

elhaj

medium

PUSH0 opcode Is Not Supported on Linea yet

Summary

Vulnerability Detail

  • The current codebase is compiled with Solidity version 0.8.24, which includes the PUSH0 opcode in the compiled bytecode. According to the README, the protocol will be deployed on the Linea network.

  • This presents an issue because Linea does not yet support the PUSH0 opcode, which can lead to unexpected behavior or outright failures when deploying and running the smart contracts.see here

Impact

  • Deploying the protocol on Linea with the current Solidity version (0.8.24) may result in unexpected behavior or failure due to the unsupported PUSH0 opcode.

Code Snippet

Tool used

Manual Review

Recommendation

  • for Linea you may consider to use version 0.8.19 to compile .
@github-actions github-actions bot added Medium A valid Medium severity issue Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Jun 11, 2024
@sherlock-admin3
Copy link

1 comment(s) were left on this issue during the judging contest.

infect3d commented:

contract will simply not deploy so no risk/loss__ see bullet 24 of invalid findings in sherlock rules

@sherlock-admin3 sherlock-admin3 added the Won't Fix The sponsor confirmed this issue will not be fixed label Jun 14, 2024
@nevillehuang
Copy link
Collaborator

nevillehuang commented Jun 15, 2024

Valid medium, since if current contract is deployed as is, it will have issues and as per noted in contest details, so the contest details will override sherlock rule 24 since it is stated as follows:

The protocol team can use the README (and only the README) to define language that indicates the codebase's restrictions and/or expected functionality. Issues that break these statements, irrespective of whether the impact is low/unknown, will be assigned Medium severity. High severity will be applied only if the issue falls into the High severity category in the judging guidelines.

@sherlock-admin3 sherlock-admin3 added the Sponsor Confirmed The sponsor acknowledged this issue is valid label Jun 18, 2024
@sherlock-admin3 sherlock-admin3 changed the title Bubbly Chocolate Wombat - PUSH0 opcode Is Not Supported on Linea yet elhaj - PUSH0 opcode Is Not Supported on Linea yet Jun 18, 2024
@sherlock-admin3 sherlock-admin3 added the Reward A payout will be made for this issue label Jun 18, 2024
@10xhash
Copy link
Collaborator

10xhash commented Jun 20, 2024

Escalate

push0 is not present in the generated bytecode of any contract

@sherlock-admin3
Copy link

Escalate

push0 is not present in the generated bytecode of any contract

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@sherlock-admin4 sherlock-admin4 added the Escalated This issue contains a pending escalation label Jun 20, 2024
@elhajin
Copy link
Collaborator

elhajin commented Jun 20, 2024

Foundry has a default evm_version set to Paris. If you compile directly with this default setting, you won't get the PUSH0 opcode in the bytecode. However, by setting the environment variable FOUNDRY_EVM_VERSION to Shanghai or Cancun, or by compiling with the flag --evm-version shanghai, for example, the PUSH0 opcode will be introduced.

We don't know how devs will compile the codebase . Given this statement from the README:

We're interested to know if there will be any issues deploying the code as-is to any of these chains, and whether their opcodes fully support our application.

I believe this is at least a medium severity issue according to Sherlock's rules.

@InfectedIsm
Copy link

Sherlock's rules :

V. How to identify a medium issue:

  • Causes a loss of funds but requires certain external conditions or specific states, or a loss is highly constrained. The losses must exceed small, finite amount of funds, and any amount relevant based on the precision or significance of the loss.
  • Breaks core contract functionality, rendering the contract useless or leading to loss of funds.

But here there are no loss of funds, neither core contract functionality broken/contract useless as the contract will simply not be deployable if I'm not mistaken

But not only that, this is considered invalid by Sherlock's rules:

VII. List of Issue categories that are not considered valid:
...
24. Using Solidity versions that support EVM opcodes that don't work on networks on which the protocol is deployed is not a valid issue beacause one can manage compilation flags to compile for past EVM versions on newer Solidity versions.

@elhajin
Copy link
Collaborator

elhajin commented Jun 20, 2024

The protocol team can use the README (and only the README) to define language that indicates the codebase's restrictions and/or expected functionality. Issues that break these statements, irrespective of whether the impact is low/unknown, will be assigned Medium severity. High severity will be applied only if the issue falls into the High severity category in the judging guidelines.

@nevillehuang
Copy link
Collaborator

Agree with comment here and here, by sherlocks hierarchy of truth, escalations should be rejected:

If the protocol team provides specific information in the README or CODE COMMENTS, that information stands above all judging rules.

@WangSecurity
Copy link

Firstly, want to clarify that the hierarchy of truth quoted in the couple of messages above is the new one, while this contest uses an old version of rules. See here. Now, each contest has its rule version (the current rules at the contest start) and you can see the link on the contest's page (below the end date).

Secondly, as I understand the current code can indeed be deployed on Linea, but using the older compiler version, correct? Taking the fact, that we don't know how the contracts will be compiled, into consideration, I believe the Rule 24 about EVM Opcodes still applies here and the issue is low severity. Unless this paragraph is not wrong, planning to accept the escalation and invalidate the issue.

@nevillehuang
Copy link
Collaborator

nevillehuang commented Jun 24, 2024

@WangSecurity

the old rules also point to read me overriding sherlock rules. Also when is the new rules introduced and from which contest is it applied (should make an announcement for important changes like this)

In case of conflict between information in the README vs Sherlock rules, the README overrides Sherlock rules

the contracts will compile correctly because of the foundry configurations mentioned here. But with the given solidity version, push0 will cause deployment reverts given linea does not support this opcode.

It is very likely there will be a zero constant somewhere within the codebase pushed onto the stack, but I agree watsons should verify this.

@elhajin
Copy link
Collaborator

elhajin commented Jun 24, 2024

Sorry didn't know about which rules are used.. but I think my point still hold

SHERLOCK RULES (for this contest):  In case of conflict between information in the README vs Sherlock rules, the README overrides Sherlock rules

CONTEST README : We're interested to know if there will be any issues deploying the code as-is to any of these chains, and whether their opcodes fully support our application

  • the compiling of the code depends on the command you run .. and how you set your environment variables and we don't know how Devs will compile the code

@amankakar
Copy link

amankakar commented Jun 24, 2024

CONTEST README : We're interested to know if there will be any issues deploying the code as-is to any of these chains, and whether their opcodes fully support our application

The contest read me has a high priority over the Sherlock rules and the team is really interested to know if there is any opcode incompatibility, This finding provides the clear explanation of issue which will result in DoS if deployed on Linea.
Hence the sponsor confirmed it which means they did not know about it.

@WangSecurity
Copy link

Excuse me for the confusion about the rules, I just meant to remind you about the commits each contest has and didn't mean to say that it somehow changes the situation.

I believe this issue is indeed medium severity cause the protocol asked about issues with the contracts as-is and their opcodes. Planning to reject the escalation and leave the issue as it is.

@WangSecurity
Copy link

Result:
Medium
Has duplicates

@sherlock-admin2 sherlock-admin2 removed the Escalated This issue contains a pending escalation label Jun 25, 2024
@sherlock-admin3 sherlock-admin3 added the Escalation Resolved This issue's escalations have been approved/rejected label Jun 25, 2024
@sherlock-admin4
Copy link
Contributor Author

Escalations have been resolved successfully!

Escalation status:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Won't Fix The sponsor confirmed this issue will not be fixed
Projects
None yet
Development

No branches or pull requests

9 participants