search

sherlock-audit / 2024-05-pooltogether-judging

2 stars 0 forks source link

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

Open sherlock-admin4 opened 1 month ago

sherlock-admin4 commented 1 month ago

elhaj

medium

PUSH0 opcode Is Not Supported on Linea yet

Summary

Vulnerability Detail

Impact

sherlock-admin3 commented 1 month ago

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

nevillehuang commented 1 month ago

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.

10xhash commented 1 month ago

Escalate

push0 is not present in the generated bytecode of any contract

sherlock-admin3 commented 1 month ago

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.

elhajin commented 1 month ago

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 commented 1 month ago

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: ...

  1. 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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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

amankakar commented 1 month ago

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 commented 1 month ago

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 commented 4 weeks ago

Result: Medium Has duplicates

sherlock-admin4 commented 4 weeks ago

Escalations have been resolved successfully!

Escalation status: