Skip to content

Fix a TrieRef slice underflow when its key crosses a node boundary - #71

Open
imlvts wants to merge 1 commit into
Adam-Vandervorst:masterfrom
imlvts:bugfix/trie-ref-key-crossing-node-boundary
Open

Fix a TrieRef slice underflow when its key crosses a node boundary#71
imlvts wants to merge 1 commit into
Adam-Vandervorst:masterfrom
imlvts:bugfix/trie-ref-key-crossing-node-boundary

Conversation

@imlvts

@imlvts imlvts commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

TrieRef::new_with_key_and_path_in descends one step before handing the rest of the walk to node_along_path, and assumed the child it found was reached at or beyond the end of node_key. In debug that assumption was a debug_assert!(consumed_byte_cnt >= node_key_len); in release it was &path[consumed_byte_cnt - node_key_len..], an unchecked subtraction on usize.

The assumption does not hold. create_path materialises an empty node part-way along a key, so node_get_child finds a child inside node_key rather than past it, and the release build computed 2 - 6 and indexed a slice at 18446744073709551612. From the public API:

map.create_path(&[2, 2]);
let mut wz = map.write_zipper_at_path(&[2, 2]);
wz.descend_to(&[2, 1, 3, 1]);
wz.val_at(&[1, 1]);          // panic

When the child is reached inside node_key, what remains to walk is node_key[consumed..] ++ path, which is a slice of neither. It does not need to be: node_along_path below already walks a whole key from a node and crosses boundaries itself, so that case now falls through to it, exactly as the "no child at all" case already did. Both call sites had the same code and both are fixed. This was the single largest panic site the differential fuzzer found, 37% of a sample.

Regression test: trie_ref_key_crossing_a_node_boundary, with nothing below the empty node and with values below it, through a write zipper and through trie_ref_at_path. Fails before this change.

`TrieRef::new_with_key_and_path_in` descends one step before handing the
rest of the walk to `node_along_path`, and assumed the child it found
was reached at or beyond the end of `node_key`.  In debug that
assumption was a `debug_assert!(consumed_byte_cnt >= node_key_len)`; in
release it was `&path[consumed_byte_cnt - node_key_len..]`, an unchecked
subtraction on `usize`.

The assumption does not hold.  `create_path` materialises an empty node
part-way along a key, so `node_get_child` finds a child *inside*
`node_key` rather than past it, and the release build computed `2 - 6`
and indexed a slice at 18446744073709551612.  From the public API:

    map.create_path(&[2, 2]);
    let mut wz = map.write_zipper_at_path(&[2, 2]);
    wz.descend_to(&[2, 1, 3, 1]);
    wz.val_at(&[1, 1]);          // panic

When the child is reached inside `node_key`, what remains to walk is
`node_key[consumed..] ++ path`, which is a slice of neither.  It does not
need to be: `node_along_path` below already walks a whole key from a
node and crosses boundaries itself, so that case now falls through to
it, exactly as the "no child at all" case already did.  Both call sites
had the same code and both are fixed.  This was the single largest panic
site the differential fuzzer found, 37% of a sample.

Regression test: `trie_ref_key_crossing_a_node_boundary`, with nothing
below the empty node and with values below it, through a write zipper
and through `trie_ref_at_path`.  Fails before this change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BZmoASqM5FUuzvJeJaYQjR
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.

1 participant