Skip to content
This repository has been archived by the owner on Jul 21, 2024. It is now read-only.

bughuntoor - Project may be unable to be deployed on Arbitrum due to incompatibility with Shanghai hardfork #32

Closed
sherlock-admin opened this issue Jan 18, 2024 · 19 comments
Labels
Escalation Resolved This issue's escalations have been approved/rejected Non-Reward This issue will not receive a payout Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Jan 18, 2024

bughuntoor

medium

Project may be unable to be deployed on Arbitrum due to incompatibility with Shanghai hardfork

Summary

Project might be unusable upon deployment due to not supporting PUSH0 OPCODE

Vulnerability Detail

The project uses unsafe pragma (^0.8.20) which by default uses PUSH0 OPCODE. However, Arbitrum currently does not support it.

This means that the produced bytecode for the different contracts won't be compatible with Arbitrum as it does not yet support the Shanghai hard fork.

Impact

Unusable contracts, will need redeploy

Code Snippet

https://github.com/sherlock-audit/2023-12-jojo-exchange-update/blob/main/smart-contract-EVM/src/JOJOExternal.sol#L6

Tool used

Manual Review

Recommendation

change pragma to 0.8.19 or change the EVM version

@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 Jan 22, 2024
@sherlock-admin2
Copy link
Contributor

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

takarez commented:

invalid because {invalid}

@JoscelynFarr JoscelynFarr added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Jan 23, 2024
@nevillehuang
Copy link
Collaborator

nevillehuang commented Jan 25, 2024

@JoscelynFarr just to double confirm, in the foundry configuration foundry.toml you guys didn't set the evm_version to paris during the time of contest , so this is valid correct?

@JoscelynFarr
Copy link

yes, we did not set the evm_version to paris, so it is valid, and we will update the pragma to 0.8.19

JoscelynFarr pushed a commit to JOJOexchange/smart-contract-EVM that referenced this issue Jan 26, 2024
@sherlock-admin2 sherlock-admin2 changed the title Original Powder Kookaburra - Project may be unable to be deployed on Arbitrum due to incompatibility with Shanghai hardfork bughuntoor - Project may be unable to be deployed on Arbitrum due to incompatibility with Shanghai hardfork Jan 27, 2024
@sherlock-admin2 sherlock-admin2 added the Reward A payout will be made for this issue label Jan 27, 2024
@0xMR0
Copy link

0xMR0 commented Jan 27, 2024

Escalate

This should be informational or low severity at best. This issue doesn't justifies medium severity at sherlock unless the inscope contracts are affected by this issue. The issue does not mention any such information affecting in scope contracts.

This was similar issue submitted during Symmio audit and it was invalidated by sherlock.

Feel free to correct if i am missing something.

@sherlock-admin
Copy link
Contributor Author

Escalate

This should be informational or low severity at best. This issue doesn't justifies medium severity at sherlock unless the inscope contracts are affected by this issue. The issue does not mention any such information affecting in scope contracts.

This was similar issue submitted during Symmio audit and it was invalidated by sherlock.

Feel free to correct if i am missing something.

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-admin2 sherlock-admin2 added the Escalated This issue contains a pending escalation label Jan 27, 2024
@nevillehuang
Copy link
Collaborator

nevillehuang commented Jan 27, 2024

@0xMR0

  • The difference between this and the issue you pointed out is that there is evidence via foundry.toml that if deployed wrongly, the whole contract will be DoSed, whereas the issue in symmio uses hardhat, which defaults to paris evm version

  • I am borderline on this one too, given the fix can be both in-scope contract and out-of-scope configuration, but I believe the evidence from the github repo suffice to show that it is a valid medium severity issue given an explicit fix is required, with the root cause being a combination of both inscope contracts and out of scope configuration file

@Hash01011122
Copy link

Correct me if I am wrong.
Based on my thorough analysis of the rules of Sherlock, I am confident in asserting that there is no risk of fund loss in this scenario. My understanding is that the issue primarily involves adapting the EVM version in the codebase for deployment on an alternative mainnet, such as Arbitrum. This adjustment appears to be, at most, a matter of low severity, primarily serving an informational purpose. It's crucial to recognize that this change does not compromise the security or integrity of the funds, aligning perfectly with the established protocols of Sherlock(no loss of funds).

@nevillehuang
Copy link
Collaborator

nevillehuang commented Jan 27, 2024

@Hash01011122 As long as it impacts core contract functionality, there is no requirement that there needs to be a definite loss of funds, however, I can agree that it is borderline out of scope, but not completely.

https://discord.com/channels/812037309376495636/881726370370158592/1200387163624308797

@0xMR0
Copy link

0xMR0 commented Jan 27, 2024

How this issue is breaking core contract functionality?

Compiler config is out of scope and the compiler version to be deployed with is decided by protocol team i.e admin. Admin is trusted per readme. Arbitrum does not support PUSH0 is well known issue since May,2023 and since the admin is trusted, it is expected that the admin/developer won't deploy the contract with incompatible compiler with on-chain.

Refer this judgement for similar issue given by sherlock judging head.

@nevillehuang
Copy link
Collaborator

@0xMR0 Thanks for the response, this issue should be invalid

@detectiveking123
Copy link
Collaborator

This is invalid.

@JoscelynFarr
Copy link

Fixed PR: JOJOexchange/smart-contract-EVM@7104f58

@IAm0x52
Copy link
Collaborator

IAm0x52 commented Feb 4, 2024

Fix looks good. Changes solidity version from 8.20 to 8.19.

@Evert0x
Copy link

Evert0x commented Feb 5, 2024

Planning to accept escalation based on similar reasoning as this previous judgment sherlock-audit/2023-10-real-wagmi-judging#84 (comment)

@T1MOH593
Copy link

T1MOH593 commented Feb 6, 2024

@Evert0x Isn't escalation should be accepted with referred reasoning?

@0xMR0
Copy link

0xMR0 commented Feb 6, 2024

@Evert0x I said low severity in escalation which is by default invalid at sherlock then i think the escalation should be accepted.

@Evert0x
Copy link

Evert0x commented Feb 6, 2024

@T1MOH593 @0xMR0 correct, edited original message.

@Czar102 Czar102 removed the Medium A valid Medium severity issue label Feb 8, 2024
@sherlock-admin sherlock-admin added Non-Reward This issue will not receive a payout and removed Reward A payout will be made for this issue labels Feb 8, 2024
@Czar102 Czar102 closed this as completed Feb 8, 2024
@Czar102
Copy link

Czar102 commented Feb 8, 2024

Result:
Invalid
Has duplicates

@sherlock-admin sherlock-admin removed the Escalated This issue contains a pending escalation label Feb 8, 2024
@sherlock-admin
Copy link
Contributor Author

Escalations have been resolved successfully!

Escalation status:

@sherlock-admin2 sherlock-admin2 added Escalation Resolved This issue's escalations have been approved/rejected and removed Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Feb 8, 2024
coin96 pushed a commit to coin96/smart-contract-EVM that referenced this issue Apr 29, 2024
Tvenus added a commit to Tvenus/smart-contract-EVM that referenced this issue May 27, 2024
Omokami added a commit to Omokami/smart-contract-EVM that referenced this issue May 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Escalation Resolved This issue's escalations have been approved/rejected Non-Reward This issue will not receive a payout Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests