security: fix silent data corruption in three edge cases#3
Merged
Conversation
- index_writer: reject user metadata > math.MaxUint32 bytes in newIndexWriter; previously the length was silently truncated via uint32() cast, writing a wrong length field to disk with no error. - bijection: change encodeEFWithCheckpoints to return ([]byte, error) and return an error when the EF bit-position checkpoint overflows uint16, matching the existing error-return pattern already used for seedBitPos overflow on line 440. Previously the value was clamped to 65535 and the build succeeded silently, producing wrong query results for affected blocks. - encoding: add init() assertion that the running architecture is little-endian; WriteEntry/WriteEntryGeneric use native-width pointer stores that produce corrupt index files on big-endian hardware with no failure signal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses three security-audit findings where certain edge cases previously produced silently corrupted index output, by turning those cases into explicit failures (error or early panic) instead of writing invalid data.
Changes:
- Add a startup endianness assertion in
internal/encodingto fail fast on big-endian architectures where unsafe native stores would corrupt output. - Change bijection EF checkpoint encoding to return an error on
uint16overflow instead of clamping. - Add a bounds check for user metadata length before writing the on-disk
uint32length field.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/encoding/encoding.go | Adds runtime little-endian enforcement to prevent silently corrupt writes on big-endian systems. |
| internal/bijection/builder.go | Makes EF checkpoint encoding overflow explicit by returning an error instead of clamping. |
| index_writer.go | Adds an explicit upper bound check for user metadata length before encoding it into a uint32 field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The size guard compared an int length against math.MaxUint32, an untyped int constant. On 32-bit platforms (e.g. GOOS=linux GOARCH=386) MaxUint32 overflows int, so the comparison and the fmt.Errorf argument both failed to compile, regressing a build that previously worked. Cast the length to uint64 rather than forcing the constant into an int; behaviour on 64-bit is unchanged and the package now builds on every architecture.
Tighten the three explanatory comments to state the why in plain terms: - index_writer: length is stored as uint32; uint64 compare keeps 32-bit builds compiling. - encoding: defer the rationale to the package doc; just note the fail-fast and how the endianness probe works. - bijection: explain why the EF checkpoint errors instead of clamping, and that it mirrors the seedBitPos check.
tamirms
enabled auto-merge (squash)
June 4, 2026 13:45
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three small fixes from a security/correctness audit. Each closes a case where bad input or an unsupported platform could silently produce a wrong index instead of a clear error. None affects normal use on 64-bit little-endian machines — these are edge-case and portability guards.
Fix 1 — Reject oversized user metadata (
index_writer.go)The metadata length is stored on disk as a
uint32. If the metadata was larger than ~4 GiB, that length silently truncated while the file offsets still used the real size, soOpen()would later read garbage — with no error at build time.newIndexWriternow returns an error when the metadata exceedsmath.MaxUint32. The check compares viauint64so it also compiles on 32-bit builds, where theMaxUint32constant doesn't fit in anint.Fix 2 — Error instead of silently clamping an EF checkpoint (
internal/bijection/builder.go)EF checkpoints are stored as
uint16and used by the decoder as the exact bit offset to start reading from. The old code clamped an overflow toMaxUint16, which would point the decoder at the wrong bit and return wrong results with no warning. The neighboringseedBitPoscheckpoint already errored on the same condition.Now it errors too, for consistency. In practice this can't trigger at the current block size (the offset tops out around 2048, well under 65535), so it's a safety guard rather than a live bug — and output is unchanged for all real inputs.
Fix 3 — Refuse to run on big-endian (
internal/encoding/encoding.go)This package reads and writes entries with native-endian pointer access, so it only produces correct files on little-endian CPUs (amd64, arm64). On a big-endian platform like s390x it would silently write incompatible index files.
An
init()check now panics at startup on big-endian, turning silent corruption into an immediate, obvious failure.Testing
amd64,linux/386, andlinux/s390x;gofmt,go vet, andgovulncheckpass.go teston the affected packages and the root package pass, including-race.