From 3d8d57fcdd38418bc52c6539ef49b2470595f0c3 Mon Sep 17 00:00:00 2001 From: Andrew Hu Date: Fri, 6 Feb 2026 16:12:27 -0500 Subject: [PATCH 1/2] =?UTF-8?q?fix(volume):=20=F0=9F=90=9B=20use=20overflo?= =?UTF-8?q?w-safe=20arithmetic=20in=20bounds=20checks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrite all `offset + length > limit` patterns as `length > limit - offset` to prevent silent int64 overflow when offset is near math.MaxInt64. Affects 5 locations: StageWriteAt, Commit, ReadAt, validateNoOverlaps, and validateVolumeManifest. Add 7 tests exercising near-MaxInt64 values across all affected code paths. Co-Authored-By: Claude Opus 4.6 --- lode/volume.go | 13 +++--- lode/volume_test.go | 101 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 6 deletions(-) diff --git a/lode/volume.go b/lode/volume.go index f8443c4..4ae5dfc 100644 --- a/lode/volume.go +++ b/lode/volume.go @@ -99,7 +99,7 @@ func (v *volume) StageWriteAt(ctx context.Context, offset int64, r io.Reader) (B if length == 0 { return BlockRef{}, fmt.Errorf("lode: block data must not be empty") } - if offset+length > v.totalLength { + if length > v.totalLength-offset { return BlockRef{}, fmt.Errorf("lode: block exceeds volume address space (offset=%d, length=%d, totalLength=%d)", offset, length, v.totalLength) } @@ -155,7 +155,7 @@ func (v *volume) Commit(ctx context.Context, blocks []BlockRef, metadata Metadat if b.Length <= 0 { return nil, fmt.Errorf("lode: block length must be positive (length=%d)", b.Length) } - if b.Offset+b.Length > v.totalLength { + if b.Length > v.totalLength-b.Offset { return nil, fmt.Errorf("lode: block exceeds volume address space (offset=%d, length=%d, totalLength=%d)", b.Offset, b.Length, v.totalLength) } expectedPath := volumeBlockPath(v.id, b.Offset, b.Length) @@ -242,8 +242,9 @@ func validateNoOverlaps(blocks []BlockRef) error { }) for i := 1; i < len(sorted); i++ { - prevEnd := sorted[i-1].Offset + sorted[i-1].Length - if prevEnd > sorted[i].Offset { + // Overflow-safe: equivalent to sorted[i-1].Offset + sorted[i-1].Length > sorted[i].Offset. + // Safe because sorted is in ascending Offset order, so the subtraction is non-negative. + if sorted[i-1].Length > sorted[i].Offset-sorted[i-1].Offset { return ErrOverlappingBlocks } } @@ -263,7 +264,7 @@ func (v *volume) ReadAt(ctx context.Context, snapshotID VolumeSnapshotID, offset if length <= 0 { return nil, fmt.Errorf("lode: length must be positive") } - if offset+length > v.totalLength { + if length > v.totalLength-offset { return nil, fmt.Errorf("lode: read exceeds volume address space (offset=%d, length=%d, totalLength=%d)", offset, length, v.totalLength) } @@ -486,7 +487,7 @@ func validateVolumeManifest(m *VolumeManifest) error { Message: "is required", } } - if b.Offset+b.Length > m.TotalLength { + if b.Length > m.TotalLength-b.Offset { return &manifestValidationError{ Field: fmt.Sprintf("blocks[%d]", i), Message: fmt.Sprintf("exceeds total_length (offset=%d, length=%d, total_length=%d)", b.Offset, b.Length, m.TotalLength), diff --git a/lode/volume_test.go b/lode/volume_test.go index fc27f48..8105bcf 100644 --- a/lode/volume_test.go +++ b/lode/volume_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "errors" + "math" "testing" "time" ) @@ -1199,3 +1200,103 @@ func TestVolume_StageWriteAt_PutError_ReturnsError(t *testing.T) { t.Fatal("expected error when store Put fails") } } + +// ============================================================================= +// Integer overflow safety tests +// ============================================================================= + +func TestVolume_StageWriteAt_OverflowOffset_ReturnsError(t *testing.T) { + vol := newTestVolume(t, "test-vol", NewMemoryFactory(), 1024) + + // offset + length would overflow int64 if computed naively. + _, err := vol.StageWriteAt(t.Context(), math.MaxInt64, bytes.NewReader([]byte("A"))) + if err == nil { + t.Fatal("expected error for offset that would overflow, got nil") + } +} + +func TestVolume_ReadAt_OverflowOffset_ReturnsError(t *testing.T) { + vol := newTestVolume(t, "test-vol", NewMemoryFactory(), 1024) + ctx := t.Context() + + blk := stageBlock(t, vol, 0, bytes.Repeat([]byte("A"), 1024)) + snap := commitBlocks(t, vol, []BlockRef{blk}, Metadata{}) + + // offset + length would overflow int64. + _, err := vol.ReadAt(ctx, snap.ID, math.MaxInt64-5, 10) + if err == nil { + t.Fatal("expected error for read offset that would overflow, got nil") + } +} + +func TestVolume_Commit_OverflowBlockRef_ReturnsError(t *testing.T) { + vol := newTestVolume(t, "test-vol", NewMemoryFactory(), 1024) + + // Craft a BlockRef where Offset + Length overflows int64. + overflowBlock := BlockRef{ + Offset: math.MaxInt64 - 5, + Length: 10, + Path: volumeBlockPath("test-vol", math.MaxInt64-5, 10), + } + + _, err := vol.Commit(t.Context(), []BlockRef{overflowBlock}, Metadata{}) + if err == nil { + t.Fatal("expected error for block ref that would overflow, got nil") + } +} + +func TestVolume_ValidateManifest_OverflowBlock_ReturnsError(t *testing.T) { + m := validVolumeManifest("test-vol", 1024) + m.Blocks = []BlockRef{ + {Offset: math.MaxInt64 - 5, Length: 10, Path: "some/path.bin"}, + } + + err := validateVolumeManifest(m) + if err == nil { + t.Fatal("expected manifest validation error for block that would overflow, got nil") + } +} + +func TestVolume_ValidateNoOverlaps_OverflowPrevEnd_Detected(t *testing.T) { + // Two blocks where prevEnd = Offset + Length would overflow. + // Block 1 starts near MaxInt64; block 2 starts at offset 0. + // Naive prevEnd would wrap to a small number and miss the overlap. + blocks := []BlockRef{ + {Offset: 0, Length: 100, Path: "a.bin"}, + {Offset: math.MaxInt64 - 10, Length: 20, Path: "b.bin"}, + } + + // These blocks don't actually overlap (they're far apart), + // so this should pass. The key is that it doesn't panic or give + // wrong results due to overflow in the prevEnd calculation. + err := validateNoOverlaps(blocks) + if err != nil { + t.Fatalf("expected no overlap for non-overlapping blocks, got: %v", err) + } +} + +func TestVolume_ValidateNoOverlaps_AdjacentAtHighOffset(t *testing.T) { + // Adjacent blocks at high offsets where Offset + Length approaches MaxInt64. + blocks := []BlockRef{ + {Offset: math.MaxInt64 - 20, Length: 10, Path: "a.bin"}, + {Offset: math.MaxInt64 - 10, Length: 10, Path: "b.bin"}, + } + + err := validateNoOverlaps(blocks) + if err != nil { + t.Fatalf("expected no overlap for adjacent blocks at high offset, got: %v", err) + } +} + +func TestVolume_ValidateNoOverlaps_OverlapAtHighOffset(t *testing.T) { + // Overlapping blocks at high offsets where naive arithmetic overflows. + blocks := []BlockRef{ + {Offset: math.MaxInt64 - 20, Length: 15, Path: "a.bin"}, + {Offset: math.MaxInt64 - 10, Length: 10, Path: "b.bin"}, + } + + err := validateNoOverlaps(blocks) + if !errors.Is(err, ErrOverlappingBlocks) { + t.Fatalf("expected ErrOverlappingBlocks for overlapping blocks at high offset, got: %v", err) + } +} From 42b5a51e8596511f6a8381050477da94442539f3 Mon Sep 17 00:00:00 2001 From: Andrew Hu Date: Fri, 6 Feb 2026 19:05:22 -0500 Subject: [PATCH 2/2] =?UTF-8?q?fix(volume):=20=E2=9C=8F=EF=B8=8F=20correct?= =?UTF-8?q?=20test=20name=20and=20comments=20for=20overflow=20overlap=20ch?= =?UTF-8?q?eck?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test asserts no overlap (non-overlapping blocks), so the name "Detected" was misleading. Comments also described block order backwards and mischaracterized the overflow risk as missed detection rather than false-positive detection. Co-Authored-By: Claude Opus 4.6 --- lode/volume_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lode/volume_test.go b/lode/volume_test.go index 8105bcf..cc49801 100644 --- a/lode/volume_test.go +++ b/lode/volume_test.go @@ -1257,10 +1257,10 @@ func TestVolume_ValidateManifest_OverflowBlock_ReturnsError(t *testing.T) { } } -func TestVolume_ValidateNoOverlaps_OverflowPrevEnd_Detected(t *testing.T) { - // Two blocks where prevEnd = Offset + Length would overflow. - // Block 1 starts near MaxInt64; block 2 starts at offset 0. - // Naive prevEnd would wrap to a small number and miss the overlap. +func TestVolume_ValidateNoOverlaps_OverflowPrevEnd_NoFalsePositive(t *testing.T) { + // Two non-overlapping blocks where prevEnd = Offset + Length would overflow. + // Block 1 starts at offset 0; block 2 starts near MaxInt64. + // Naive prevEnd on block 2 would wrap to a small number and falsely report overlap. blocks := []BlockRef{ {Offset: 0, Length: 100, Path: "a.bin"}, {Offset: math.MaxInt64 - 10, Length: 20, Path: "b.bin"},