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

Solidity version 0.8.20 may not work on other chains due to PUSH0 #584

Closed
code423n4 opened this issue Jul 4, 2023 · 2 comments
Closed
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-c QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/ArbitrumBranchBridgeAgent.sol#L2
https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/ArbitrumBranchPort.sol#L3
https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/ArbitrumCoreBranchRouter.sol#L2

Vulnerability details

Impact

The compiler for Solidity 0.8.20 switches the default target EVM version to Shanghai, which includes the new PUSH0 opcode. This opcode is not yet implemented on some L2s including Arbitrum, meaning contracts deployed on those chains will be non-functional (see proof of concept section). Many in scope contracts use a floating pragma (^0.8.0), which may result in compiler version 0.8.20 being used for deployment to Arbitrum.

Although this is fairly unlikely, the impact is high as the contracts would be completely unusable and funds could be sent to those addresses by unaware users, which would be irretrievable.

Proof of Concept

Relevant issue from solidity github:

When compiled using 0.8.19, this works fine.

When compiled using 0.8.20, the contract deploys but is non-functional.

See: https://arbiscan.io/address/0x504ada2360ac822faf7ac703b350fadc8d931211#code

In File 16, you'll see for example: bool public constant isRECustodian = true;, but if you click the Read Contract tab you'll see that it returns false (or rather, it returns nothing at all and Arbiscan just shows false). This is similarly true with all the other values

When reading amountRecovered using a dummy parameter like 0x1234567812345678123456781234567812345678, we see this: Error: Returned error: invalid opcode: PUSH0

I note that using push0 was a new feature in 0.8.20

Perhaps it's "not a bug, it's Arbitrum's problem for not supporting push0". But it would be helpful to know how to disable push0 generation, if that's the case. And it's probably worth some documentation. It's a confusing trap to fall into.

Recommended Mitigation Steps

Ensure pragma does not include Solidity version 0.8.20 for contracts that will be deployed to other chains, for example like so:

pragma solidity >=0.8.0 <0.8.20;

Assessed type

Other

@code423n4 code423n4 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 Jul 4, 2023
code423n4 added a commit that referenced this issue Jul 4, 2023
@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Jul 11, 2023
@c4-judge
Copy link
Contributor

trust1995 changed the severity to QA (Quality Assurance)

@c4-judge
Copy link
Contributor

trust1995 marked the issue as grade-c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-c QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants