Fix zellic finding 5 #584

kunxian-xia · 2023-07-03T09:45:14Z

Description

Fix the finding 5 reported in Zellic/Kalos Audit Report finding 5.

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

roynalnaruto · 2023-07-04T20:36:43Z

zkevm-circuits/src/rlp_circuit_fsm.rs

+                let (lt, eq) = tlength_lte_mlength.expr(meta, Some(Rotation::cur()));
+                cb.require_equal(
+                    "tag_length <= max_length",
+                    sum::expr([


nitpick: I would personally prefer or over sum as its more intuitive in this case

But or has higher degree than sum. How about I leave some comments there?

Okay, it works in this case as both lt and eq are boolean, so sum and or will actually give us the same result. But yes, a comment to explain it would be better as sum by itself shouldn't be used (but it works in this case).

roynalnaruto · 2023-07-04T20:37:34Z

zkevm-circuits/src/witness/rlp_fsm.rs

+// The number of bytes of list can not larger than 2^24 = 2^(8*3).
+pub(crate) const N_BYTES_LIST: usize = 3;


why this change?

Because if we want to add tag_length <= max_length for the case of LongList, then the tag_length here means the number of bytes to represent a LongList(e.g. [0xf9, 0x01, 0xff] has tag_length = 2). Maybe we should change the naming of this const?

Yes, and the comment as well can be more descriptive..

…S auditors (#572) * fix finding 3 (#575) * Fix zellic finding 4 (#576) * fix finding 3 (#575) * fix finding 4 --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * add range check on diffs (#586) * Fix finding 10 (#578) * fix finding 3 (#575) * fix finding 10 * Fix finding 13 (#579) * fix finding 3 (#575) * fix finding 13 * Fix zellic finding 14 (#580) * fix finding 3 (#575) * fix finding 14 * Fix zellic finding 5 (#584) * fix finding 3 (#575) * fix finding 5 * refine comments * fmt * Fix finding 17 (#602) * add q_last * fix * add more diff range check * fix finding 7 (#625) * tx_id = 1 when sm starts * Fix finding 11 : use length for rlc in rlp table (#719) * fix: use tag_bytes_rlc and tag_length to copy tag's bytes around * fix lookup input for Len & RLC & GasCost fields in tx circuit * refactor * fix * refactor * fix col phase issue * refactor bytes_rlc type * Fix the bugs in Tx & PI circuits reported by Zellic & KALOS auditors (#612) * lookup chain_id to RLP table * fix finding 22 (#614) * fix finding 21 (#613) * fix finding 23 (#618) * fix finding 26 (#622) * fix finding 28 (#624) Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * fix finding 29 (#623) Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * enforce is_final is true at the last row and fix RLC related vul (#735) * Fix finding 30 (#733) * enforce all txs in a block are included in the tx table * clippy --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * Fix Zellic / Kalos finding25 (#619) * fix finding 25 * add comment --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * fix conflicts --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com> * use q_first instead * fmt --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>

* add row counting interface for keccak * add class level capacity calculator for keccak * remove f capacity from core * remove capacity calculator in aggregator util * remove unnecessary imports * replace max keccak round in core * replace reference for max keccak * remove unnecessary keccak imports and constants * remove max keccak constant * remove constants in hash cell parsing * remove constant column sanity check * add state column usage log * adjust input bytes column * add long column padding * correct fmt * fix fmt * minor fixes * fix * Fix: allow skipping of L1Msg tx part 2 (calculate num_all_txs in tx circuit) (#778) * calculate num_l1_msgs and num_l2_txs in tx circuit * fix * fmt and clippy * fix: non-last tx requires next is calldata * add NumAllTxs in block table and copy it from pi to block table * add lookup for NumAllTxs in tx circuit * clippy * add block num diff check to avoid two real block have same num * clippy * address comments * Fix the bugs in RLP/Tx/PI circuit which are reported by Zellic & KALOS auditors (#572) * fix finding 3 (#575) * Fix zellic finding 4 (#576) * fix finding 3 (#575) * fix finding 4 --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * add range check on diffs (#586) * Fix finding 10 (#578) * fix finding 3 (#575) * fix finding 10 * Fix finding 13 (#579) * fix finding 3 (#575) * fix finding 13 * Fix zellic finding 14 (#580) * fix finding 3 (#575) * fix finding 14 * Fix zellic finding 5 (#584) * fix finding 3 (#575) * fix finding 5 * refine comments * fmt * Fix finding 17 (#602) * add q_last * fix * add more diff range check * fix finding 7 (#625) * tx_id = 1 when sm starts * Fix finding 11 : use length for rlc in rlp table (#719) * fix: use tag_bytes_rlc and tag_length to copy tag's bytes around * fix lookup input for Len & RLC & GasCost fields in tx circuit * refactor * fix * refactor * fix col phase issue * refactor bytes_rlc type * Fix the bugs in Tx & PI circuits reported by Zellic & KALOS auditors (#612) * lookup chain_id to RLP table * fix finding 22 (#614) * fix finding 21 (#613) * fix finding 23 (#618) * fix finding 26 (#622) * fix finding 28 (#624) Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * fix finding 29 (#623) Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * enforce is_final is true at the last row and fix RLC related vul (#735) * Fix finding 30 (#733) * enforce all txs in a block are included in the tx table * clippy --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * Fix Zellic / Kalos finding25 (#619) * fix finding 25 * add comment --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> * fix conflicts --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com> * use q_first instead * fmt --------- Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com> Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com> * add pi comments * rename preimage col idx * add keccak rows check * rename input bytes col finder fn * modify keccak row env constaint * modify keccak row env constaint * add named constant setup vars * modify keccak row check * clippy advised * add comments on chunk hash * fmt * avoid constant lookup table * avoid repetitive computation of input_bytes_col_idx --------- Co-authored-by: Zhuo Zhang <mycinbrin@gmail.com> Co-authored-by: xkx <xiakunxian130@gmail.com> Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

kunxian-xia added 2 commits June 29, 2023 16:46

fix finding 3 (#575)

ba5f76a

fix finding 5

c524076

github-actions bot added the crate-zkevm-circuits label Jul 3, 2023

kunxian-xia mentioned this pull request Jul 3, 2023

Fix the bugs in RLP/Tx/PI circuit which are reported by Zellic & KALOS auditors #572

Merged

12 tasks

kunxian-xia force-pushed the fix/rlp-audit-wave1 branch from ba5f76a to 8de5a6d Compare July 3, 2023 12:20

roynalnaruto reviewed Jul 4, 2023

View reviewed changes

kunxian-xia added 2 commits July 6, 2023 10:49

refine comments

bfa56ff

fmt

0a9189d

roynalnaruto approved these changes Jul 6, 2023

View reviewed changes

kunxian-xia merged commit 1243ef4 into fix/rlp-audit-wave1 Jul 6, 2023

kunxian-xia deleted the fix/rlp-zellic-finding5 branch July 6, 2023 13:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix zellic finding 5 #584

Fix zellic finding 5 #584

kunxian-xia commented Jul 3, 2023

roynalnaruto Jul 4, 2023

kunxian-xia Jul 5, 2023

roynalnaruto Jul 5, 2023

roynalnaruto Jul 4, 2023

kunxian-xia Jul 5, 2023 •

edited

Loading

roynalnaruto Jul 5, 2023

		// The number of bytes of list can not larger than 2^24 = 2^(8*3).
		pub(crate) const N_BYTES_LIST: usize = 3;

Fix zellic finding 5 #584

Fix zellic finding 5 #584

Conversation

kunxian-xia commented Jul 3, 2023

Description

Type of change

roynalnaruto Jul 4, 2023

Choose a reason for hiding this comment

kunxian-xia Jul 5, 2023

Choose a reason for hiding this comment

roynalnaruto Jul 5, 2023

Choose a reason for hiding this comment

roynalnaruto Jul 4, 2023

Choose a reason for hiding this comment

kunxian-xia Jul 5, 2023 • edited Loading

Choose a reason for hiding this comment

roynalnaruto Jul 5, 2023

Choose a reason for hiding this comment

kunxian-xia Jul 5, 2023 •

edited

Loading