Skip to content

Commit d3b1daf

Browse files
committed
fskit: fix offset overflow on writes
1 parent 04895a7 commit d3b1daf

File tree

2 files changed

+221
-12
lines changed

2 files changed

+221
-12
lines changed

fs/fskit/node.go

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package fskit
22

33
import (
4-
"bytes"
54
"context"
65
"io"
76
"log/slog"
@@ -329,7 +328,15 @@ func (f *nodeFile) Read(b []byte) (n int, err error) {
329328
if f.offset < 0 {
330329
return 0, &fs.PathError{Op: "read", Path: f.inode.Path(), Err: fs.ErrInvalid}
331330
}
332-
n = copy(b, f.data[f.offset:])
331+
332+
// Check if offset can be represented as int (required for slice operations)
333+
// This prevents overflow on 32-bit systems and WASM
334+
const maxInt = int(^uint(0) >> 1)
335+
if f.offset > int64(maxInt) {
336+
return 0, &fs.PathError{Op: "read", Path: f.inode.Path(), Err: fs.ErrInvalid}
337+
}
338+
339+
n = copy(b, f.data[int(f.offset):])
333340
f.offset += int64(n)
334341
return n, nil
335342
}
@@ -380,7 +387,15 @@ func (f *nodeFile) ReadAt(b []byte, offset int64) (n int, err error) {
380387
if offset < 0 || offset > int64(len(f.data)) {
381388
return 0, &fs.PathError{Op: "read", Path: f.inode.Path(), Err: fs.ErrInvalid}
382389
}
383-
n = copy(b, f.data[offset:])
390+
391+
// Check if offset can be represented as int (required for slice operations)
392+
// This prevents overflow on 32-bit systems and WASM
393+
const maxInt = int(^uint(0) >> 1)
394+
if offset > int64(maxInt) {
395+
return 0, &fs.PathError{Op: "read", Path: f.inode.Path(), Err: fs.ErrInvalid}
396+
}
397+
398+
n = copy(b, f.data[int(offset):])
384399
if n < len(b) {
385400
return n, io.EOF
386401
}
@@ -406,18 +421,50 @@ func (f *nodeFile) Write(b []byte) (n int, err error) {
406421

407422
n = len(b)
408423
cur := f.offset
409-
diff := cur - int64(len(f.data))
410-
var tail []byte
411-
if n+int(cur) < len(f.data) {
412-
tail = f.data[n+int(cur):]
424+
425+
// Validate offset is within valid range for slice operations
426+
if cur < 0 {
427+
return 0, &fs.PathError{Op: "write", Path: f.inode.Path(), Err: fs.ErrInvalid}
413428
}
414-
if diff > 0 {
415-
f.data = append(f.data, append(bytes.Repeat([]byte{00}, int(diff)), b...)...)
416-
f.data = append(f.data, tail...)
429+
430+
// Check if offset can be represented as int (required for slice operations)
431+
// This prevents overflow on 32-bit systems and WASM
432+
const maxInt = int(^uint(0) >> 1)
433+
if cur > int64(maxInt) {
434+
return 0, &fs.PathError{Op: "write", Path: f.inode.Path(), Err: fs.ErrInvalid}
435+
}
436+
437+
dataLen := int64(len(f.data))
438+
endPos := cur + int64(n)
439+
440+
// Check if end position would overflow
441+
if endPos > int64(maxInt) {
442+
return 0, &fs.PathError{Op: "write", Path: f.inode.Path(), Err: fs.ErrInvalid}
443+
}
444+
445+
curInt := int(cur)
446+
endPosInt := int(endPos)
447+
448+
// Calculate if we need to grow the data slice
449+
if cur > dataLen {
450+
// Need to pad with zeros between current data end and write position
451+
// Grow data to accommodate padding + new data
452+
newData := make([]byte, endPosInt)
453+
copy(newData, f.data)
454+
// zeros are already in place from make()
455+
copy(newData[curInt:], b)
456+
f.data = newData
457+
} else if endPos > dataLen {
458+
// Write starts within or at the end of existing data but extends beyond
459+
newData := make([]byte, endPosInt)
460+
copy(newData, f.data)
461+
copy(newData[curInt:], b)
462+
f.data = newData
417463
} else {
418-
f.data = append(f.data[:cur], b...)
419-
f.data = append(f.data, tail...)
464+
// Write is entirely within existing data
465+
copy(f.data[curInt:], b)
420466
}
467+
421468
f.modTime = time.Now()
422469
f.dirty = true
423470
f.offset += int64(n)

fs/fskit/node_test.go

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,3 +592,165 @@ func TestNodeModTimeTracking(t *testing.T) {
592592
t.Errorf("expected 'hello world', got %q", string(n.Data()))
593593
}
594594
}
595+
596+
// TestNodeFileOffsetOverflow is a regression test for integer overflow issues
597+
// when converting int64 offsets to int for slice operations.
598+
// Before the fix, large offsets would overflow on 32-bit systems (including WASM),
599+
// causing incorrect behavior like returning MaxInt32 (2147483647) as bytes written.
600+
func TestNodeFileOffsetOverflow(t *testing.T) {
601+
node := Entry("test.txt", 0644, []byte("hello"))
602+
603+
f, err := node.Open(".")
604+
if err != nil {
605+
t.Fatalf("failed to open: %v", err)
606+
}
607+
defer f.Close()
608+
609+
// Calculate maxInt for this platform
610+
const maxInt = int(^uint(0) >> 1)
611+
612+
// Test 1: Seek beyond maxInt should fail or succeed based on platform
613+
sf := f.(interface {
614+
Seek(int64, int) (int64, error)
615+
})
616+
617+
// Attempt to seek to a large offset
618+
// Avoid overflow by checking if we're on a 64-bit platform
619+
var largeOffset int64
620+
if maxInt == 9223372036854775807 {
621+
// 64-bit platform, use a large but valid offset
622+
largeOffset = int64(maxInt) - 1000
623+
} else {
624+
// 32-bit platform, use maxInt + 1 to test overflow handling
625+
// Compute at runtime to avoid compile-time overflow on 64-bit
626+
maxIntAsInt64 := int64(maxInt)
627+
largeOffset = maxIntAsInt64 + 1
628+
}
629+
630+
_, _ = sf.Seek(largeOffset, 0)
631+
// On platforms where int64 > int, this should fail with ErrInvalid
632+
// On 64-bit platforms where int == int64, the seek might succeed but
633+
// subsequent write should still validate
634+
635+
// Test 2: Try to write at an offset that would overflow
636+
wf := f.(interface{ Write([]byte) (int, error) })
637+
nBytes, err := wf.Write([]byte("test"))
638+
639+
// The write should either:
640+
// - Fail because seek failed (offset validation in Seek)
641+
// - Fail because write validates offset is > maxInt
642+
// - Succeed on 64-bit if within memory limits
643+
// But it should NEVER return an overflowed value like MaxInt32
644+
if err == nil {
645+
// If write succeeded, verify it returned correct count
646+
if nBytes != 4 {
647+
t.Errorf("write should return correct count (4), got %d", nBytes)
648+
}
649+
} else {
650+
// Error is expected on 32-bit systems
651+
if err != fs.ErrInvalid && err != fs.ErrClosed {
652+
t.Logf("write failed with error (may be expected): %v", err)
653+
}
654+
}
655+
656+
// Test 3: WriteAt with large offset
657+
waf := f.(interface {
658+
WriteAt([]byte, int64) (int, error)
659+
})
660+
661+
nBytes, err = waf.WriteAt([]byte("test"), largeOffset)
662+
if err == nil {
663+
// Write succeeded - verify correct count
664+
if nBytes != 4 {
665+
t.Errorf("WriteAt should return correct count (4), got %d", nBytes)
666+
}
667+
} else {
668+
// Error expected on platforms where offset > maxInt
669+
if err != fs.ErrInvalid && err != fs.ErrClosed {
670+
t.Logf("WriteAt failed with error (may be expected): %v", err)
671+
}
672+
}
673+
674+
// Test 4: ReadAt with large offset
675+
f.Close()
676+
f, _ = node.Open(".")
677+
defer f.Close()
678+
679+
raf := f.(interface {
680+
ReadAt([]byte, int64) (int, error)
681+
})
682+
683+
buf := make([]byte, 10)
684+
nBytes, err = raf.ReadAt(buf, largeOffset)
685+
if err == nil || err == io.EOF {
686+
// Read succeeded (shouldn't happen with empty data at that offset)
687+
// but if it does, nBytes should be sane
688+
if nBytes < 0 || nBytes > len(buf) {
689+
t.Errorf("ReadAt returned invalid count: %d", nBytes)
690+
}
691+
} else {
692+
// Error expected
693+
if err != fs.ErrInvalid && err != fs.ErrClosed {
694+
t.Logf("ReadAt failed with error (may be expected): %v", err)
695+
}
696+
}
697+
}
698+
699+
// TestNodeFileOffsetOverflowSpecific tests the specific bug scenario
700+
// where a write of 175 bytes returned 2147483647 (MaxInt32).
701+
func TestNodeFileOffsetOverflowSpecific(t *testing.T) {
702+
node := Entry("test.txt", 0644, []byte{})
703+
704+
f, err := node.Open(".")
705+
if err != nil {
706+
t.Fatalf("failed to open: %v", err)
707+
}
708+
defer f.Close()
709+
710+
sf := f.(interface {
711+
Seek(int64, int) (int64, error)
712+
})
713+
wf := f.(interface{ Write([]byte) (int, error) })
714+
715+
// Scenario: offset that would cause int overflow on 32-bit
716+
// const maxInt32 = 2147483647
717+
const maxInt = int(^uint(0) >> 1)
718+
719+
// If we're on a 32-bit system (or WASM), try to reproduce the overflow
720+
if maxInt <= 2147483647 {
721+
// Try to seek near the overflow boundary
722+
testOffset := int64(maxInt) - 100
723+
_, err = sf.Seek(testOffset, 0)
724+
if err != nil {
725+
// Expected on some systems
726+
t.Logf("Seek to large offset failed (expected): %v", err)
727+
return
728+
}
729+
730+
// Try to write 175 bytes (the original failing case)
731+
data := make([]byte, 175)
732+
for i := range data {
733+
data[i] = byte(i)
734+
}
735+
736+
nBytes, err := wf.Write(data)
737+
738+
// The bug was: n would be 2147483647 instead of 175
739+
// After fix: either write succeeds with n=175, or fails with proper error
740+
if err != nil {
741+
if err != fs.ErrInvalid && err != fs.ErrClosed {
742+
t.Logf("write failed with error (may be expected): %v", err)
743+
}
744+
} else {
745+
// Write succeeded
746+
if nBytes != 175 {
747+
t.Errorf("REGRESSION: write of 175 bytes returned %d (expected 175)", nBytes)
748+
}
749+
if nBytes == 2147483647 {
750+
t.Error("REGRESSION: write returned MaxInt32, indicating integer overflow bug")
751+
}
752+
}
753+
} else {
754+
t.Logf("Skipping 32-bit specific test on 64-bit platform")
755+
}
756+
}

0 commit comments

Comments
 (0)