Skip to content

index: guard posting iterator against overflowing varints - #1107

Open
youdie006 wants to merge 1 commit into
sourcegraph:mainfrom
youdie006:fix/posting-iterator-uvarint-guard
Open

index: guard posting iterator against overflowing varints#1107
youdie006 wants to merge 1 commit into
sourcegraph:mainfrom
youdie006:fix/posting-iterator-uvarint-guard

Conversation

@youdie006

Copy link
Copy Markdown

Addresses finding #2 of #1106 (the compressed posting-iterator panic).

Problem

newCompressedPostingIterator and (*compressedPostingIterator).next in index/hititer.go slice blob[sz:] using the length returned by binary.Uvarint without checking it. binary.Uvarint returns a non-positive length when the varint is empty or overflows 64 bits (per its contract), so a malformed posting list makes the slice bound go negative and panics:

panic: runtime error: slice bounds out of range [-11:]
  index/hititer.go:192

Repro from the issue:

blob := []byte{0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01}
newCompressedPostingIterator(blob, ...) // panics

Fix

Guard both binary.Uvarint call sites: on a non-positive length, treat the posting list as exhausted (construction) / stop advancing (next) instead of slicing with a bad length.

Test

Added TestCompressedPostingIterator_overflowVarint, covering both the constructor and the next paths with the overflowing-varint input. Verified red before the fix (it panics with slice bounds out of range [-11:]) and green after. go test ./index/, gofmt, and go vet all pass locally.

Scope

Scoped to finding #2 only (the posting-iterator negative-length guard). The other two findings in #1106 (re-opening a panicked shard, and the corrupt-shard hang) are larger and left for separate changes.


Disclosure: developed with the assistance of Claude Code (AI); reviewed and verified by me.

newCompressedPostingIterator and (*compressedPostingIterator).next
sliced blob[sz:] using the length returned by binary.Uvarint without
checking it. binary.Uvarint returns a non-positive length when the
varint is empty or overflows 64 bits, so a malformed posting list
made the slice bound go negative and panic (index out of range
[-11:]).

Guard both call sites: on a non-positive length, treat the posting
list as exhausted instead of slicing. Add a regression test using the
overflowing-varint input from the report.
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