Repository: Eyevinn/mp4ff (go)

Issue:
SencBox.Size() can panic with non-zero SampleCount and empty rawData
I discovered this while fuzzing in #397 but didn't know how to fix it.

```
--- FAIL: FuzzDecodeBox (0.04s)
    --- FAIL: FuzzDecodeBox/75565444c6c2f1dd (0.00s)
panic: runtime error: index out of range [0] with length 0 [recovered]
	panic: runtime error: index out of range [0] with length 0

goroutine 359 [running]:
testing.tRunner.func1.2({0x1046cfd80, 0x140000261c8})
	/opt/homebrew/Cellar/go/1.23.1/libexec/src/testing/testing.go:1632 +0x1bc
testing.tRunner.func1()
	/opt/homebrew/Cellar/go/1.23.1/libexec/src/testing/testing.go:1635 +0x334
panic({0x1046cfd80?, 0x140000261c8?})
	/opt/homebrew/Cellar/go/1.23.1/libexec/src/runtime/panic.go:785 +0x124
github.com/Eyevinn/mp4ff/mp4.(*SencBox).Size(0x0?)
	/Users/eric/src/mp4ff/mp4/senc.go:266 +0xb0
github.com/Eyevinn/mp4ff/mp4.FuzzDecodeBox.func1(0x1400012e820, {0x140004ca610, 0x10, 0x10})
	/Users/eric/src/mp4ff/mp4/fuzz_test.go:73 +0xc4
```

There is currently a skipped test in that pull request that can replicate the issue with commenting out the `Skip()` and then running:

```
go test -run=FuzzDecodeBox/75565444c6c2f1dd ./mp4
```

Pull request description:
fix: make senc parsing more robust
Added checks that values are OK in initial decode of SencBox.

Fixes #400, and makes the mentioned fuzz test obsolete.

Patch:
diff --git a/mp4/senc.go b/mp4/senc.go
index 78de6f7..dc01da2 100644
--- a/mp4/senc.go
+++ b/mp4/senc.go
@@ -36,6 +36,7 @@ type SencBox struct {
 	rawData          []byte                 // intermediate storage when reading
 	IVs              []InitializationVector // 8 or 16 bytes if present
 	SubSamples       [][]SubSamplePattern
+	readBoxSize      uint64 // As read from box header
 }
 
 // CreateSencBox - create an empty SencBox
@@ -86,12 +87,20 @@ func (s *SencBox) AddSample(sample SencSample) error {
 
 // DecodeSenc - box-specific decode
 func DecodeSenc(hdr BoxHeader, startPos uint64, r io.Reader) (Box, error) {
+	if hdr.Size < 16 {
+		return nil, fmt.Errorf("box size %d less than min size 16", hdr.Size)
+	}
 	data, err := readBoxBody(r, hdr)
 	if err != nil {
 		return nil, err
 	}
 
 	versionAndFlags := binary.BigEndian.Uint32(data[0:4])
+	version := byte(versionAndFlags >> 24)
+	flags := versionAndFlags & flagsMask
+	if version > 0 {
+		return nil, fmt.Errorf("version %d not supported", version)
+	}
 	sampleCount := binary.BigEndian.Uint32(data[4:8])
 
 	if len(data) < 8 {
@@ -99,12 +108,18 @@ func DecodeSenc(hdr BoxHeader, startPos uint64, r io.Reader) (Box, error) {
 	}
 
 	senc := SencBox{
-		Version:          byte(versionAndFlags >> 24),
+		Version:          version,
 		rawData:          data[8:], // After the first 8 bytes of box content
-		Flags:            versionAndFlags & flagsMask,
+		Flags:            flags,
 		StartPos:         startPos,
 		SampleCount:      sampleCount,
 		readButNotParsed: true,
+		readBoxSize:      hdr.Size,
+	}
+
+	if flags&UseSubSampleEncryption != 0 && (len(senc.rawData) < 2*int(sampleCount)) {
+		return nil, fmt.Errorf("box size %d too small for %d samples and subSampleEncryption",
+			hdr.Size, sampleCount)
 	}
 
 	if senc.SampleCount == 0 || len(senc.rawData) == 0 {
@@ -116,15 +131,31 @@ func DecodeSenc(hdr BoxHeader, startPos uint64, r io.Reader) (Box, error) {
 
 // DecodeSencSR - box-specific decode
 func DecodeSencSR(hdr BoxHeader, startPos uint64, sr bits.SliceReader) (Box, error) {
+	if hdr.Size < 16 {
+		return nil, fmt.Errorf("box size %d less than min size 16", hdr.Size)
+	}
+
 	versionAndFlags := sr.ReadUint32()
+	version := byte(versionAndFlags >> 24)
+	if version > 0 {
+		return nil, fmt.Errorf("version %d not supported", version)
+	}
+	flags := versionAndFlags & flagsMask
 	sampleCount := sr.ReadUint32()
+
+	if flags&UseSubSampleEncryption != 0 && ((hdr.Size - 16) < 2*uint64(sampleCount)) {
+		return nil, fmt.Errorf("box size %d too small for %d samples and subSampleEncryption",
+			hdr.Size, sampleCount)
+	}
+
 	senc := SencBox{
-		Version:          byte(versionAndFlags >> 24),
+		Version:          version,
 		rawData:          sr.ReadBytes(hdr.payloadLen() - 8), // After the first 8 bytes of box content
-		Flags:            versionAndFlags & flagsMask,
+		Flags:            flags,
 		StartPos:         startPos,
 		SampleCount:      sampleCount,
 		readButNotParsed: true,
+		readBoxSize:      hdr.Size,
 	}
 
 	if senc.SampleCount == 0 || len(senc.rawData) == 0 {
@@ -254,11 +285,14 @@ func (s *SencBox) setSubSamplesUsedFlag() {
 
 // Size - box-specific type
 func (s *SencBox) Size() uint64 {
-	if s.readButNotParsed {
-		return boxHeaderSize + 8 + uint64(len(s.rawData)) // read 8 bytes after header
+	if s.readBoxSize > 0 {
+		return s.readBoxSize
 	}
-	totalSize := uint64(boxHeaderSize + 8)
+	return s.calcSize()
+}
 
+func (s *SencBox) calcSize() uint64 {
+	totalSize := uint64(boxHeaderSize + 8)
 	perSampleIVSize := uint64(s.GetPerSampleIVSize())
 	for i := uint32(0); i < s.SampleCount; i++ {
 		totalSize += perSampleIVSize


Test patch:
diff --git a/mp4/fuzz_test.go b/mp4/fuzz_test.go
index a894f8f..ed4326e 100644
--- a/mp4/fuzz_test.go
+++ b/mp4/fuzz_test.go
@@ -54,10 +54,6 @@ func FuzzDecodeBox(f *testing.F) {
 	}
 
 	f.Fuzz(func(t *testing.T, b []byte) {
-		if t.Name() == "FuzzDecodeBox/75565444c6c2f1dd" {
-			t.Skip("There is a bug in SencBox.Size() that needs to be fixed for " + t.Name())
-		}
-
 		ctx, cancel := context.WithCancel(context.Background())
 		defer cancel()
 		monitorMemory(ctx, t, 500*1024*1024) // 500MB
diff --git a/mp4/senc_test.go b/mp4/senc_test.go
index be227b2..f4335fa 100644
--- a/mp4/senc_test.go
+++ b/mp4/senc_test.go
@@ -4,48 +4,66 @@ import (
 	"bytes"
 	"testing"
 
+	"github.com/Eyevinn/mp4ff/bits"
 	"github.com/go-test/deep"
 )
 
 func TestSencDirectValues(t *testing.T) {
 	iv8 := InitializationVector("12345678")
 	iv16 := InitializationVector("0123456789abcdef")
-	sencBoxes := []*SencBox{
+	cases := []struct {
+		desc string
+		senc *SencBox
+	}{
 		{
-			Version:         0,
-			Flags:           0,
-			SampleCount:     431, // No perSampleIVs
-			perSampleIVSize: 0,
+			desc: "No perSampleIVs",
+			senc: &SencBox{
+				Version:         0,
+				Flags:           0,
+				SampleCount:     431, // No perSampleIVs
+				perSampleIVSize: 0,
+			},
 		},
 		{
-			Version:         0,
-			Flags:           0,
-			SampleCount:     1,
-			perSampleIVSize: 8,
-			IVs:             []InitializationVector{iv8},
-			SubSamples:      [][]SubSamplePattern{{{10, 1000}}},
+			desc: "perSampleIVSize 8",
+			senc: &SencBox{
+				Version:         0,
+				Flags:           0,
+				SampleCount:     1,
+				perSampleIVSize: 8,
+				IVs:             []InitializationVector{iv8},
+				SubSamples:      [][]SubSamplePattern{{{10, 1000}}},
+			},
 		},
 		{
-			Version:         0,
-			Flags:           0,
-			SampleCount:     1,
-			perSampleIVSize: 16,
-			IVs:             []InitializationVector{iv16},
-			SubSamples:      [][]SubSamplePattern{{{10, 1000}, {20, 2000}}},
+			desc: "perSampleIVSize 16",
+			senc: &SencBox{
+				Version:         0,
+				Flags:           0,
+				SampleCount:     1,
+				perSampleIVSize: 16,
+				IVs:             []InitializationVector{iv16},
+				SubSamples:      [][]SubSamplePattern{{{10, 1000}, {20, 2000}}},
+			},
 		},
 		{
-			Version:         0,
-			Flags:           0,
-			SampleCount:     2,
-			perSampleIVSize: 16,
-			IVs:             []InitializationVector{iv16, iv16},
-			SubSamples:      [][]SubSamplePattern{{{10, 1000}}, {{20, 2000}}},
+			desc: "perSampleIVSize 16, 2 subsamples",
+			senc: &SencBox{
+				Version:         0,
+				Flags:           0,
+				SampleCount:     2,
+				perSampleIVSize: 16,
+				IVs:             []InitializationVector{iv16, iv16},
+				SubSamples:      [][]SubSamplePattern{{{10, 1000}}, {{20, 2000}}},
+			},
 		},
 	}
 
-	for _, senc := range sencBoxes {
-		sencDiffAfterEncodeAndDecode(t, senc, 0)
-		sencDiffAfterEncodeAndDecode(t, senc, senc.perSampleIVSize)
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			sencDiffAfterEncodeAndDecode(t, c.senc, 0)
+			sencDiffAfterEncodeAndDecode(t, c.senc, c.senc.perSampleIVSize)
+		})
 	}
 }
 
@@ -147,3 +165,50 @@ func TestImplicitIVSize(t *testing.T) {
 		}
 	}
 }
+
+func TestBadSencData(t *testing.T) {
+	// raw senc box with version > 2 */
+	cases := []struct {
+		desc string
+		raw  []byte
+		err  string
+	}{
+		{
+			desc: "too short",
+			raw:  []byte{0x00, 0x00, 0x00, 0x0f, 's', 'e', 'n', 'c', 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+			err:  "decode senc pos 0: box size 15 less than min size 16",
+		},
+		{
+			desc: "v1 not supported",
+			raw:  []byte{0x00, 0x00, 0x00, 0x10, 's', 'e', 'n', 'c', 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+			err:  "decode senc pos 0: version 1 not supported",
+		},
+		{
+			desc: "too short for subsample encryption",
+			raw:  []byte{0x00, 0x00, 0x00, 0x10, 's', 'e', 'n', 'c', 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0xff},
+			err:  "decode senc pos 0: box size 16 too small for 255 samples and subSampleEncryption",
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			buf := bytes.NewBuffer(c.raw)
+			_, err := DecodeBox(0, buf)
+			if err == nil {
+				t.Errorf("expected error %q, but got nil", c.err)
+			}
+			if err.Error() != c.err {
+				t.Errorf("expected error %q, got %q", c.err, err.Error())
+			}
+
+			sr := bits.NewFixedSliceReader(c.raw)
+			_, err = DecodeBoxSR(0, sr)
+			if err == nil {
+				t.Errorf("expected error %q, but got nil", c.err)
+			}
+			if err.Error() != c.err {
+				t.Errorf("expected error %q, got %q", c.err, err.Error())
+			}
+		})
+	}
+}
diff --git a/mp4/testdata/fuzz/FuzzDecodeBox/0881196294cc083f b/mp4/testdata/fuzz/FuzzDecodeBox/0881196294cc083f
new file mode 100644
index 0000000..d93bb12
--- /dev/null
+++ b/mp4/testdata/fuzz/FuzzDecodeBox/0881196294cc083f
@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("\x00\x00\x00\x10senc\x00000\xfd\xfd\xfd\xbc")
\ No newline at end of file