From a81f8388c8e4c0f248b7a93000611f2a9698c4ee Mon Sep 17 00:00:00 2001 From: Simon Chow Date: Wed, 3 Jun 2026 12:29:40 -0700 Subject: [PATCH 1/3] security: fix silent data corruption in three edge cases - 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> --- index_writer.go | 4 ++++ internal/bijection/builder.go | 14 ++++++++++---- internal/encoding/encoding.go | 11 +++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/index_writer.go b/index_writer.go index 636013f..9edc1aa 100644 --- a/index_writer.go +++ b/index_writer.go @@ -4,6 +4,7 @@ import ( "encoding/binary" "errors" "fmt" + "math" "os" "github.com/cespare/xxhash/v2" @@ -54,6 +55,9 @@ type indexWriter struct { func newIndexWriter(path string, cfg *buildConfig, numBlocks uint32, algo blockBuilder) (*indexWriter, error) { // Compute variable section sizes userMetadataLen := len(cfg.userMetadata) + if userMetadataLen > math.MaxUint32 { + return nil, fmt.Errorf("user metadata size %d exceeds maximum of %d bytes", userMetadataLen, math.MaxUint32) + } algoConfigLen := algo.GlobalConfigSize() // Estimate total file size for pre-allocation diff --git a/internal/bijection/builder.go b/internal/bijection/builder.go index a43d48e..b8a1c27 100644 --- a/internal/bijection/builder.go +++ b/internal/bijection/builder.go @@ -246,7 +246,10 @@ func (bb *Builder) MaxMetadataSizeForCurrentBlock() int { func (bb *Builder) encodeSeparatedMetadataInto(dst []byte) (int, error) { // Step 1: Encode Elias-Fano cumulative counts and compute EF checkpoints var cp checkpoints - efData := bb.encodeEFWithCheckpoints(&cp, bucketsPerBlock) + efData, err := bb.encodeEFWithCheckpoints(&cp, bucketsPerBlock) + if err != nil { + return 0, err + } // Step 2: Encode SeedStream and compute Seed checkpoints bb.seedEncoder.resetEncoder() @@ -406,7 +409,7 @@ func (bb *Builder) resolveFallbackSeed(bucketIdx int, subBucket uint8) uint32 { } // encodeEFWithCheckpoints encodes Elias-Fano and computes EF checkpoints. -func (bb *Builder) encodeEFWithCheckpoints(cp *checkpoints, numBuckets int) []byte { +func (bb *Builder) encodeEFWithCheckpoints(cp *checkpoints, numBuckets int) ([]byte, error) { efData := encodeEliasFanoInto(bb.cumulative, uint32(bb.keysInBlock), &bb.efBuffer) lowBits := computeEFLowBits(numBuckets, bb.keysInBlock) @@ -423,11 +426,14 @@ func (bb *Builder) encodeEFWithCheckpoints(cp *checkpoints, numBuckets int) []by highPart = int(prevCumulative) >> lowBits } - bitPos := min(bucketIdx+highPart, math.MaxUint16) + bitPos := bucketIdx + highPart + if bitPos > math.MaxUint16 { + return nil, fmt.Errorf("bijection: EF bit position overflow: %d exceeds uint16 max", bitPos) + } cp.efBitPos[checkpointIdx] = uint16(bitPos) } - return efData + return efData, nil } // encodeSeedsWithCheckpoints encodes seeds and computes checkpoints. diff --git a/internal/encoding/encoding.go b/internal/encoding/encoding.go index f28e0ab..ac1282e 100644 --- a/internal/encoding/encoding.go +++ b/internal/encoding/encoding.go @@ -6,6 +6,17 @@ package encoding import "unsafe" +func init() { + // Verify little-endian byte order at startup. WriteEntry and WriteEntryGeneric + // use native-width pointer stores that are only correct on little-endian + // architectures; running on a big-endian machine would produce silently corrupt + // index files. + var probe uint16 = 0x0102 + if *(*byte)(unsafe.Pointer(&probe)) != 0x02 { + panic("encoding: package requires a little-endian architecture") + } +} + // WriteEntry packs a fingerprint and payload into a little-endian entry of // entrySize bytes at position pos in the buffer starting at basePtr. // The entry is stored as: fp | (payload << fpShift), where fpShift is From 1a77b4a2352c55db4971e586a99b7590699a1640 Mon Sep 17 00:00:00 2001 From: Simon Chow Date: Wed, 3 Jun 2026 14:17:03 -0700 Subject: [PATCH 2/3] fix: make user-metadata size guard compile on 32-bit platforms 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. --- index_writer.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/index_writer.go b/index_writer.go index 9edc1aa..ceda83a 100644 --- a/index_writer.go +++ b/index_writer.go @@ -55,8 +55,12 @@ type indexWriter struct { func newIndexWriter(path string, cfg *buildConfig, numBlocks uint32, algo blockBuilder) (*indexWriter, error) { // Compute variable section sizes userMetadataLen := len(cfg.userMetadata) - if userMetadataLen > math.MaxUint32 { - return nil, fmt.Errorf("user metadata size %d exceeds maximum of %d bytes", userMetadataLen, math.MaxUint32) + // Cast the length (not the constant) to uint64: math.MaxUint32 is an untyped + // int constant, so comparing it against an int operand overflows int and fails + // to compile on 32-bit platforms. uint64 holds MaxUint32 on every architecture + // and the comparison is identical on 64-bit. + if uint64(userMetadataLen) > math.MaxUint32 { + return nil, fmt.Errorf("user metadata size %d exceeds maximum of %d bytes", userMetadataLen, uint64(math.MaxUint32)) } algoConfigLen := algo.GlobalConfigSize() From 2b2c086e0cc57508754c27df8aa802bbc035d400 Mon Sep 17 00:00:00 2001 From: Simon Chow Date: Wed, 3 Jun 2026 14:27:08 -0700 Subject: [PATCH 3/3] docs: simplify inline comments for the audit fixes 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. --- index_writer.go | 7 +++---- internal/bijection/builder.go | 4 ++++ internal/encoding/encoding.go | 7 +++---- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/index_writer.go b/index_writer.go index ceda83a..44e036c 100644 --- a/index_writer.go +++ b/index_writer.go @@ -55,10 +55,9 @@ type indexWriter struct { func newIndexWriter(path string, cfg *buildConfig, numBlocks uint32, algo blockBuilder) (*indexWriter, error) { // Compute variable section sizes userMetadataLen := len(cfg.userMetadata) - // Cast the length (not the constant) to uint64: math.MaxUint32 is an untyped - // int constant, so comparing it against an int operand overflows int and fails - // to compile on 32-bit platforms. uint64 holds MaxUint32 on every architecture - // and the comparison is identical on 64-bit. + // The length is written to disk as a uint32, so reject anything larger. + // Comparing via uint64 keeps this compiling on 32-bit builds, where the + // math.MaxUint32 constant doesn't fit in an int. if uint64(userMetadataLen) > math.MaxUint32 { return nil, fmt.Errorf("user metadata size %d exceeds maximum of %d bytes", userMetadataLen, uint64(math.MaxUint32)) } diff --git a/internal/bijection/builder.go b/internal/bijection/builder.go index b8a1c27..103b117 100644 --- a/internal/bijection/builder.go +++ b/internal/bijection/builder.go @@ -426,6 +426,10 @@ func (bb *Builder) encodeEFWithCheckpoints(cp *checkpoints, numBuckets int) ([]b highPart = int(prevCumulative) >> lowBits } + // Checkpoints are stored as uint16. This shouldn't overflow for valid + // blocks, but if it ever does, error out instead of clamping to a wrong + // offset (which would silently corrupt query results). Mirrors the + // seedBitPos check in encodeSeedsWithCheckpoints. bitPos := bucketIdx + highPart if bitPos > math.MaxUint16 { return nil, fmt.Errorf("bijection: EF bit position overflow: %d exceeds uint16 max", bitPos) diff --git a/internal/encoding/encoding.go b/internal/encoding/encoding.go index ac1282e..e02ed82 100644 --- a/internal/encoding/encoding.go +++ b/internal/encoding/encoding.go @@ -7,10 +7,9 @@ package encoding import "unsafe" func init() { - // Verify little-endian byte order at startup. WriteEntry and WriteEntryGeneric - // use native-width pointer stores that are only correct on little-endian - // architectures; running on a big-endian machine would produce silently corrupt - // index files. + // Fail loudly on big-endian CPUs rather than writing silently corrupt files + // (see the package doc above). The probe is 0x0102; on little-endian its first + // byte in memory is 0x02. var probe uint16 = 0x0102 if *(*byte)(unsafe.Pointer(&probe)) != 0x02 { panic("encoding: package requires a little-endian architecture")