Skip to content

Comments

Add deployed bytecode retrieval mitigation#8282

Merged
Spacesai1or merged 1 commit intodevelopfrom
wyatt/bindgen/bytecode-retrieval-mitigation
Dec 16, 2023
Merged

Add deployed bytecode retrieval mitigation#8282
Spacesai1or merged 1 commit intodevelopfrom
wyatt/bindgen/bytecode-retrieval-mitigation

Conversation

@Spacesai1or
Copy link
Contributor

@Spacesai1or Spacesai1or commented Nov 27, 2023

This PR will query OP mainnet for the contract's deployed bytecode and compare it to the bytecode sourced from ETH mainnet. Because the added contracts from #8281 are all deterministically deployed, the address for the contracts are the same across networks. The thinking of the this mitigation is: if the deployed bytecode on chain A differ from B, then there might be a chain specific consideration that's not accounted for and may affect the validity of using ETH mainnet's deployed bytecode on any OP chain

@Spacesai1or
Copy link
Contributor Author

Spacesai1or commented Nov 27, 2023

@semgrep-app
Copy link
Contributor

semgrep-app bot commented Nov 27, 2023

Semgrep found 1 import-text-template finding:

  • op-bindings/bindgen/generator_local.go: L12

When working with web applications that involve rendering user-generated content, it's important to properly escape any HTML content to prevent Cross-Site Scripting (XSS) attacks. In Go, the text/template package does not automatically escape HTML content, which can leave your application vulnerable to these types of attacks. To mitigate this risk, it's recommended to use the html/template package instead, which provides built-in functionality for HTML escaping. By using html/template to render your HTML content, you can help to ensure that your web application is more secure and less susceptible to XSS vulnerabilities.

Ignore this finding from import-text-template.

@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch from d2dfe25 to ca9e5cd Compare November 27, 2023 20:28
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/init-remote-contract-gen branch from e7b892d to d8b1bfd Compare November 27, 2023 20:29
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch 2 times, most recently from 6e9ff00 to 5b45b09 Compare November 27, 2023 21:02
@Spacesai1or Spacesai1or marked this pull request as draft November 27, 2023 21:33
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/init-remote-contract-gen branch from 5441f06 to 713b79e Compare November 27, 2023 22:28
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch from 5b45b09 to 1c5bc07 Compare November 27, 2023 22:29
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 27, 2023

Walkthrough

Walkthrough

The recent changes involve integrating Ethereum and Optimism RPC clients into the binding generation process for smart contracts. New global variables for RPC URLs have been added, and the codebase now includes the functionality to compare deployed bytecode with RPC data. This ensures that the generated bindings match the actual deployed contracts on Ethereum and Optimism networks.

Changes

File(s) Summary
op-bindings/Makefile Introduced RPC_URL_ETH and RPC_URL_OP variables and updated targets to pass these as command line arguments.
op-bindings/bindgen/generator_remote.go, op-bindings/bindgen/main.go Added ethclient import and initialized RPC clients for Ethereum and Optimism, including error handling.
op-bindings/bindgen/remote_handlers.go Included new imports and added a method to compare bytecode from RPC clients with stored bytecode, along with updates to existing handler methods.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ?


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • You can reply to a review comment made by CodeRabbit.
  • You can tag CodeRabbit on specific lines of code or files in the PR by tagging @coderabbitai in a comment.
  • You can tag @coderabbitai in a PR comment and ask one-off questions about the PR and the codebase. Use quoted replies to pass the context for follow-up questions.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/init-remote-contract-gen branch from 713b79e to 1df8707 Compare November 27, 2023 22:40
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch from 1c5bc07 to 296a828 Compare November 27, 2023 22:40
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/init-remote-contract-gen branch from 1df8707 to 45375b2 Compare November 27, 2023 22:44
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch from 296a828 to c0b5253 Compare November 27, 2023 22:44
@Spacesai1or Spacesai1or marked this pull request as ready for review November 28, 2023 21:10
Copy link
Contributor

@hamdiallam hamdiallam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good as well. Same feedback as the previous PR with regards to Crit usage within the application

@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/init-remote-contract-gen branch from 45375b2 to 08c5c83 Compare December 1, 2023 04:15
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch from c0b5253 to b1a3dd0 Compare December 1, 2023 04:15
@semgrep-app
Copy link
Contributor

semgrep-app bot commented Dec 1, 2023

Semgrep found 6 sol-style-return-arg-fmt findings:

Named return arguments to functions must be appended with an underscore (_)

Ignore this finding from sol-style-return-arg-fmt.

Semgrep found 1 sol-style-input-arg-fmt finding:

  • packages/contracts-bedrock/scripts/Deployer.sol: L373

Inputs to functions must be prepended with an underscore (_)

Ignore this finding from sol-style-input-arg-fmt.

@Spacesai1or Spacesai1or marked this pull request as draft December 1, 2023 19:29
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/init-remote-contract-gen branch from 08c5c83 to 6b3ec28 Compare December 2, 2023 01:29
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch from b1a3dd0 to 6901d27 Compare December 2, 2023 01:29
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/init-remote-contract-gen branch from 6b3ec28 to d46de62 Compare December 2, 2023 02:20
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/init-remote-contract-gen branch from b3c7bfb to 5a9e6a4 Compare December 14, 2023 07:03
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch from 6b0fd49 to 443ce2f Compare December 14, 2023 07:03
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/init-remote-contract-gen branch from 5a9e6a4 to 387d992 Compare December 14, 2023 07:11
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch from 443ce2f to 63c78f5 Compare December 14, 2023 07:11
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/init-remote-contract-gen branch from 387d992 to 68a0655 Compare December 14, 2023 21:16
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch from 63c78f5 to c09b84a Compare December 14, 2023 21:16
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/init-remote-contract-gen branch from 68a0655 to c549d3d Compare December 14, 2023 21:55
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch from c09b84a to 9bba7b9 Compare December 14, 2023 21:55
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/init-remote-contract-gen branch from c549d3d to dba53cc Compare December 14, 2023 22:20
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch from 9bba7b9 to 44b836a Compare December 14, 2023 22:20
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/init-remote-contract-gen branch from dba53cc to 725ad1a Compare December 14, 2023 23:38
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch from 44b836a to bc3062d Compare December 14, 2023 23:38
Base automatically changed from wyatt/bindgen/init-remote-contract-gen to develop December 15, 2023 19:38
@Spacesai1or Spacesai1or force-pushed the wyatt/bindgen/bytecode-retrieval-mitigation branch from bc3062d to 81951e2 Compare December 15, 2023 19:46
@codecov
Copy link

codecov bot commented Dec 15, 2023

Codecov Report

Merging #8282 (5441f06) into develop (816e425) will increase coverage by 4.90%.
The diff coverage is n/a.

❗ Current head 5441f06 differs from pull request most recent head 81951e2. Consider uploading reports for the commit 81951e2 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #8282      +/-   ##
===========================================
+ Coverage    34.61%   39.51%   +4.90%     
===========================================
  Files          167      102      -65     
  Lines         7162     3358    -3804     
  Branches      1212      438     -774     
===========================================
- Hits          2479     1327    -1152     
+ Misses        4532     1957    -2575     
+ Partials       151       74      -77     
Flag Coverage Δ
cannon-go-tests 63.48% <ø> (ø)
chain-mon-tests ?
common-ts-tests ?
contracts-bedrock-tests 21.80% <ø> (+1.47%) ⬆️
contracts-ts-tests ?
core-utils-tests ?
sdk-next-tests ?
sdk-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 81 files with indirect coverage changes

@Spacesai1or Spacesai1or requested a review from mds1 December 15, 2023 20:22
@Spacesai1or Spacesai1or added this pull request to the merge queue Dec 16, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 16, 2023
@Spacesai1or Spacesai1or added this pull request to the merge queue Dec 16, 2023
Merged via the queue into develop with commit 4abef20 Dec 16, 2023
@Spacesai1or Spacesai1or deleted the wyatt/bindgen/bytecode-retrieval-mitigation branch December 16, 2023 03:12
This was referenced Dec 17, 2023
roberto-bayardo pushed a commit to roberto-bayardo/optimism that referenced this pull request Dec 19, 2023
roberto-bayardo pushed a commit to roberto-bayardo/optimism that referenced this pull request Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants