Repository: Eyevinn/mp4ff
Pull request #402 (2025-01-19 20:52:51)

fix: ensure mime box is large enough to read
Fix the panic found by fuzzing:

```
--- FAIL: FuzzDecodeBox (478.19s)
    --- FAIL: FuzzDecodeBox (0.00s)
        testing.go:1591: panic: runtime error: index out of range [-1]
            goroutine 4015806 [running]:
            runtime/debug.Stack()
            	/opt/homebrew/Cellar/go/1.23.1/libexec/src/runtime/debug/stack.go:26 +0xc4
            testing.tRunner.func1()
            	/opt/homebrew/Cellar/go/1.23.1/libexec/src/testing/testing.go:1591 +0x21c
            panic({0x1030bc1a0?, 0x1400a1521f8?})
            	/opt/homebrew/Cellar/go/1.23.1/libexec/src/runtime/panic.go:785 +0x124
            github.com/Eyevinn/mp4ff/mp4.DecodeMimeSR({{0x1400a68e9e0?, 0x1400a6a3f20?}, 0x1400a68e9e0?, 0x8?}, 0x14009e8c658?, {0x1030df818, 0x1400a68cd40})
            	/Users/eric/src/mp4ff/mp4/mime.go:35 +0x214
            github.com/Eyevinn/mp4ff/mp4.DecodeMime({{0x1400a68e9e0?, 0x140001207b0?}, 0x1400a68e9e0?, 0x4?}, 0x0, {0x1030d8da0?, 0x1400a6a3f20?})
            	/Users/eric/src/mp4ff/mp4/mime.go:24 +0x184
            github.com/Eyevinn/mp4ff/mp4.DecodeBox(0x0, {0x1030d8da0, 0x1400a6a3f20})
            	/Users/eric/src/mp4ff/mp4/box.go:315 +0x178
            github.com/Eyevinn/mp4ff/mp4.FuzzDecodeBox.func1(0x1400a6c29c0, {0x1400a112d80, 0xc, 0x10})
            	/Users/eric/src/mp4ff/mp4/fuzz_test.go:65 +0x14c
```


Patch:
diff --git a/mp4/mime.go b/mp4/mime.go
index 830eb15..c01499c 100644
--- a/mp4/mime.go
+++ b/mp4/mime.go
@@ -1,6 +1,7 @@
 package mp4
 
 import (
+	"fmt"
 	"io"
 
 	"github.com/Eyevinn/mp4ff/bits"
@@ -31,6 +32,9 @@ func DecodeMimeSR(hdr BoxHeader, startPos uint64, sr bits.SliceReader) (Box, err
 		Version: byte(versionAndFlags >> 24),
 		Flags:   versionAndFlags & flagsMask,
 	}
+	if hdr.payloadLen() < 5 {
+		return nil, fmt.Errorf("mime: box payload size %d less than 5", hdr.payloadLen())
+	}
 	rest := sr.ReadBytes(hdr.payloadLen() - 4)
 	if rest[len(rest)-1] == 0 { // zero-termination
 		b.ContentType = string(rest[:len(rest)-1])
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
diff --git a/mp4/testdata/fuzz/FuzzDecodeBox/77cf6e30648805ea b/mp4/testdata/fuzz/FuzzDecodeBox/77cf6e30648805ea
new file mode 100644
index 0000000..5f053b3
--- /dev/null
+++ b/mp4/testdata/fuzz/FuzzDecodeBox/77cf6e30648805ea
@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("\x00\x00\x00\fmime0000")