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..cc49801 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_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"}, + } + + // 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) + } +}