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

Audit Fixes EIP2930/1559 RLP Decoding Table Correspondence #1181

Merged
merged 29 commits into from
Apr 24, 2024

Conversation

darth-cy
Copy link

Description

This PR resolves a major issue in RLP audit that raises concern for malicious RLP stack op injection into the decoding table. The fix includes lookups from the sorted decoding table into rlp_table and RLP state machine to ensure row correspondence.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

@darth-cy darth-cy marked this pull request as draft April 1, 2024 19:01
@darth-cy darth-cy changed the base branch from develop to audit/1559-rlp April 1, 2024 20:55
@darth-cy darth-cy marked this pull request as ready for review April 3, 2024 21:15
@darth-cy darth-cy self-assigned this Apr 4, 2024
zkevm-circuits/src/rlp_circuit_fsm.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/rlp_circuit_fsm.rs Outdated Show resolved Hide resolved
@lispc lispc merged commit e65f258 into audit/1559-rlp Apr 24, 2024
15 checks passed
@lispc lispc deleted the audit/1559-decoding-correspondence branch April 24, 2024 02:23
lispc added a commit that referenced this pull request Apr 24, 2024
* Correct typo

* Correct RLC

* Change tag column status

* Add tx_nonce consistency

* Remove vestige gadget

* Add v constraint for 1559/2930

* Constrain depth consistency for fsm state transition

* Correct q_first query for initial num_all_txs_acc condition

* Ensure continuity of sectional flag is_calldata and is_access_list

* Remove tx unchanged constraint for dynamic sectional transition

* Add tx_id constraints in dynamic section

* Constrain access list tx table fields according to AccessListAddressLen

* Constrain is_padding_tx

* Add constraint for id calculation in rlp_decoding_table

* Remove redundant boolean requirements

* Add init condition for access_list_idx and storage_key_idx

* Turn on value_is_zero for access list len

* Remove whitespace

* Add byte_idx to PUSH/POP lookup correspondence

* fmt

* Update cargo

* Resolve build issues

* fmt and clippy

* Recover tests

* Correct gate name

* Remove debug flags

* fmt

* cargo

* Revert ethers-core branch

* cargo

* Restrict booleans

* fmt

* Audit Fixes EIP2930/1559 RLP Decoding Table Correspondence (#1181)

* Remove debug flags

* Add decoding table PUSH lookup correspondence

* Complete Stack Op Correspondence

* fmt

* Correct lookup constraint

* Remove unused macro

* fmt

* Correct lookup constraint for pre 2930 txs

* fmt

* Adjust comments

* fmt

* Add lookup indicator column for state machine

* Add correct condition for stack op lookups

* Complete PUSH op lookup

* Correct POP op lookup

* Correct PUSH lookup

* fmt

* Add backward compatibility for eip155 and pre155

* Add al_idx to input expression

* Add access list idx to PUSH correspondence

---------

Co-authored-by: DreamWuGit <[email protected]>

* Audit Fixes EIP2930/1559 TxCircuit Dynamic Section Transitions (#1176)

* Refactor tx circuit dynamic section after introducing access list

* Adjust comments

* fmt

* Column naming

---------

Co-authored-by: DreamWuGit <[email protected]>

---------

Co-authored-by: DreamWuGit <[email protected]>
Co-authored-by: Zhang Zhuo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants