kroma-network / go-ethereum

go-ethereum for Kroma
GNU Lesser General Public License v3.0
44 stars 23 forks source link

[ZKTRIE-001] merkleTreeIterator.seek() can panic due to key vs. path confusion #71

Closed this-is-chainlight closed 6 months ago

this-is-chainlight commented 6 months ago

Summary

merkleTreeIterator.seek() treats the input key as a path, leading to a possible slice bounds out of range error.

Description

merkleTreeIterators are created when the NodeIterator() method is called on a ZkMerkleTrie. As in the standard go-ethereum Trie, this method accepts a starting key for iteration. In the normal trie iterator (nodeIterator), this input key is transformed into nibbles (the equivalent of zkTrie paths):

func (it *nodeIterator) seek(prefix []byte) error {
    // The path we're looking for is the hex encoded key without terminator.
    key := keybytesToHex(prefix)
    key = key[:len(key)-1]
    ...
}

However in merkleTreeIterator, seek() assumes the input byte array is already in path form, leading to incorrect behavior and a possible panic.

func (it *merkleTreeIterator) seek(path []byte) {
    if len(path) == 0 {
        return
    }

    for _, p := range path {
        if parent, ok := it.stack[len(it.stack)-1].(*merkleTreeIteratorParentNode); ok {
            // AUDIT: this path is not validated to be valid, can cause OOB access crash
            if child := it.resolveNode(parent.children[p]); child != nil {
                it.stack = append(it.stack, child)
                it.path = append(it.path, p)
                continue
            }
            ...
        }
        ...
    }
    ...
}

In most cases, the start key values are nil, so this issue is avoided. However, a non-nil start key can be passed via a go-ethereum dump command or by the debug_accountRange RPC method.

Impact

Low

Although the code is reachable by an RPC endpoint, the panic is caught and handled by the RPC handler.

Recommendation

Transform the start key into a path before usage.

diff --git a/trie/iterator.go b/trie/iterator.go
index c5198b741..921298ef9 100644
--- a/trie/iterator.go
+++ b/trie/iterator.go
@@ -929,13 +929,16 @@ func newMerkleTreeIterator(
        return it
 }

-func (it *merkleTreeIterator) seek(path []byte) {
+func (it *merkleTreeIterator) seek(start []byte) {
+       path := zk.NewTreePathFromBytes(start)
+
        if len(path) == 0 {
                return
        }

Reference

N/A

Patch

Status

Applied actions and commit hash