[rearden-grok]

Summary

Found by bobberb and muse, 2026-10-18.

• Fuse8 open returns corrupt when the declared body length does not fit the file, and when the segment-count length is not a multiple of the segment length (102, 103).
• A Class A body read longer than 64 MiB is refused before the buffer is reserved or zero-filled. Index writebehind uses that same cap, and a wrapped start-plus-length is corrupt (104).
• Scripthash page walks stop after 1,000,000 next links. The free walk also remembers each page it has already freed, so overwriting the page header with the freelist link cannot make a cycle look like a new page (105).
• Historical bodies outside the durable tip window are not revalidated, and store.secret is not a MAC of record bodies. A torn spent.body page below spend_durable can read as unspent; replay does not heal it. Documented only (106, 107). No checksums and no spend write-path change.
• Index: 108.

Findings rows 102–108 here and rows 092–101 on #1069 both append to the same README table. Whichever pull request merges second has to keep both ranges.

Test plan

• cargo test -p rbitcoin-store --lib scripthash_page_free_ and index_read_above_the_cap_is_refused, plus the earlier fuse8 and body-read pins (workspace suite not run locally; the VM was busy)
• cargo fmt -p rbitcoin-store and cargo clippy -p rbitcoin-store --all-targets -- -D warnings
• GitHub qc, test, windows, macos, coverage

[rearden-grok]

Summary

Open refuses a wrapping fuse8 body length and a segment count that is not a multiple of the segment length. A Class A body read above 64 MiB is rejected before the buffer grows. The linked-page reader stops at one million pages and returns corrupt on a cycle. The extent free walk is a separate loop and still has no stop.

The findings index rows in this PR and in #1069 both append to the same README table, so the second merge needs both ranges kept.

Issue counts by severity

• bugs: 1
• suggestions: 0
• nits: 0

Issues outside the diff

These findings reference lines that are not present in the diff and could not be posted as inline comments:

• [bug] crates/rbitcoin-store/src/scripthash.rs:2257 -- free_if_paged still follows next with while off != 0 and no page cap. A two-page cycle, the case collect_linked_pages now rejects, does not return here. Replacing or freeing that extent holds the writer in the loop. The changelog says a cycle returns corrupt.
  • Suggestion: Stop at SH_PAGE_CHAIN_MAX and return StoreError::Corrupt, the same way collect_linked_pages does.

[rearden-grok]

Freeing a paged scripthash extent now stops at the same one million page cap as the reader. A two-page cycle returns corrupt instead of spinning the writer. The test uses a cap of four on a side thread with a two second timeout, so a missing stop fails quickly and coverage does not walk a million links.

The findings README note in the review is now in both pull request descriptions. Rows 092-101 and 102-108 append to the same table, so the second merge has to keep both ranges.

[rearden-grok]

Summary

Verdict: Changes requested

The free walk and the linked read walk both stop at SH_PAGE_CHAIN_MAX, and fuse8 open now rejects a wrapping body length and a non-multiple segment count before use. The free walk is not fail-closed: free_slab overwrites the page header, and a later iteration of a cycle parses that freelist word as next, so a two-page cycle can return success and, once the freelist head is large, install a freelist cycle or free an unrelated page. The 64 MiB Class A cap is also only on VarTable. Index writebehind still zero-fills a create-loc stride.

Issue counts by severity

• bugs: 2
• suggestions: 0
• nits: 0

[rearden-grok]

[bug] follow_page_chain captures next and then visits. free_if_paged visits with free_slab, which write_ats the old freelist head over the first 8 bytes of the page. The packed next lives at bytes 3..8, so that write destroys it. The walk does not remember which offsets it already freed. When a cycle comes back it read_ats the freed page and sh_page_next treats the freelist word as a link. sh_page_next does not error on that word. If the page-class freelist head is 0 or below 32 MiB, the forged next is 0 and the walk returns Ok after freeing each distinct page once. The cycle is not reported. If the head is at least 32 MiB and bit 24 is clear, the forged next is a low page offset ((head >> 25) * 4096). A two-page cycle can then free the first page a second time and link the two slabs into a freelist cycle (A -> B -> A), or follow that forged offset into a live page and free it. The million-page cap never runs. scripthash_page_free_cycle_is_corrupt stays green because its visit is |_, _| Ok(()) and does not write the header.

Suggestion: Before read_at, if off was already visited, return StoreError::Corrupt("scripthash page chain cycle") and do not visit again. Drive the regression through free_if_paged with a non-zero freelist head at or above 32 MiB (bit 24 clear), and assert corrupt plus no duplicate freelist node.

[rearden-grok]

The free walk now remembers each page before it reads the next link. If that page shows up again, the walk returns a corrupt-store error and does not free it a second time. The new test builds a two-page cycle with the freelist head at 32 MiB, so overwriting the page header would otherwise look like a real link, and checks that the free stops on the cycle and the freelist does not hold the same page twice.

[rearden-grok]

[bug] checked_body_read refuses len > 64 MiB before reserve or vec![0u8; n], but that is not every Class A read of a create-loc stride. A stride is a u32 times 8, about 32 GiB (create_loc.rs extract_pairs_ovf). Index writebehind calls read_index_window (index_build_uring.rs). read_parents only checks txout.0 + txout.1 against the body high-water mark, then ReadJob::new does vec![0u8; len as usize]. A full node's tx.body is far larger than 32 GiB, so a corrupt stride whose end still sits inside the high-water mark passes the check and zero-fills about 32 GiB. read_blocks is the same shape for hi - lo, and the input-body length is allocated with no high-water check and no cap. VarTable::with_bytes_at is fixed. This path never calls it. The new test only hits with_bytes_at and pread_span.

Suggestion: Refuse len > 64 MiB inside ReadJob::new, and on the hi - lo and input-body lengths, before the vec allocation, with the same Rejected("body read too large") as checked_body_read. Use checked_add for txout.0 + txout.1 in read_blocks.

[rearden-grok]

Index rebuild reads now use the same 64 MiB cap as other body reads. A length above that cap is refused before any buffer is allocated, including a create-loc stride whose end still sits inside the body file. The block reader also rejects a start plus length that wraps. The new test asks for one byte past the cap and checks that the read is refused.

[rearden-grok]

Summary

Verdict: Approve

The closed fixes still hold. Fuse8 open rejects a wrapped body length and a segment-count length that is not a multiple of the segment length, and the index math stays inside the fingerprint slice. Scripthash follow_page_chain records the offset before read_at and before free_slab, and sh_page_next only yields 4 KiB-aligned offsets, so an 8-byte freelist poke cannot forge a next the seen set misses. collect_linked_pages and the extent tail still stop at 1,000,000 links on a non-cycle. Every ReadJob::new and VarTable body allocation refuses len > 64 MiB before vec / reserve; a u32 stride whose end sits inside the body HWM no longer allocates ~32 GiB. saturating_add remains only the HWM compare on the seqsigwit and parent paths. The historical bit-rot and torn-spend notes match the write and replay paths.

Issue counts by severity

• bugs: 0
• suggestions: 0
• nits: 0