fix(mp4): reject undersized boxes instead of underflowing the size

CHANGELOG.md

@@ -148,6 +148,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   and the conformance window, and left the chroma format and bit depths unset
   for a `rep_format()` with `chroma_and_bit_depth_vps_present_flag` equal to
   zero instead of inferring them from the preceding one
+- `DecodeContainerChildren` and `DecodeContainerChildrenSR` reported a wrapped
+  parent size for a box too small to hold the fixed fields before its children,
+  as `dref`, `stsd`, `trep` and `meta` can be. Such a box is now rejected up
+  front, naming the declared and the minimum size
+- `DecodeEsdsSR` derived its descriptor size from `hdr.Size-12` without a length
+  check, wrapping for a shorter box. Both the check and the size now measure the
+  payload, which is also correct for an extended-size header
 - `DecodeContainerChildren` now checks the child position against the
   container end before decoding each child, matching
   `DecodeContainerChildrenSR`. A container truncated exactly on a child

mp4/container.go

@@ -96,6 +96,11 @@ func containerSize(children []Box) uint64 {
 
 // DecodeContainerChildren decodes a container box
 func DecodeContainerChildren(hdr BoxHeader, startPos, endPos uint64, r io.Reader) ([]Box, error) {
+	// Guard before the unsigned subtractions below can wrap.
+	if startPos > endPos {
+		return nil, fmt.Errorf("%s: box size %d is too small, needs at least %d bytes",
+			hdr.Name, hdr.Size, hdr.Size+(startPos-endPos))
+	}
 	children := make([]Box, 0, 8)
 	pos := startPos
 	for {
@@ -125,6 +130,11 @@ func DecodeContainerChildren(hdr BoxHeader, startPos, endPos uint64, r io.Reader
 
 // DecodeContainerChildrenSR decodes a container box
 func DecodeContainerChildrenSR(hdr BoxHeader, startPos, endPos uint64, sr bits.SliceReader) ([]Box, error) {
+	// Guard before the unsigned subtractions below can wrap.
+	if startPos > endPos {
+		return nil, fmt.Errorf("%s: box size %d is too small, needs at least %d bytes",
+			hdr.Name, hdr.Size, hdr.Size+(startPos-endPos))
+	}
 	children := make([]Box, 0, 8) // Good initial size
 	pos := startPos
 	initPos := sr.GetPos()

mp4/container_test.go

@@ -3,6 +3,8 @@ package mp4_test
 import (
 	"bytes"
 	"encoding/binary"
+	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/Eyevinn/mp4ff/bits"
@@ -91,3 +93,71 @@ func TestDecodeEmptyContainerKeepsSibling(t *testing.T) {
 	fR, errR := mp4.DecodeFile(bytes.NewReader(data))
 	check(t, "DecodeFile", fR, errR)
 }
+
+// makeUndersizedBox builds a box whose declared size is smaller than the fixed
+// fields its decoder expects before the children, followed by a sibling so that
+// reads past the declared end still find data.
+func makeUndersizedBox(name string, size uint32) []byte {
+	b := make([]byte, 8)
+	binary.BigEndian.PutUint32(b[0:4], size)
+	copy(b[4:8], name)
+	for len(b) < int(size) {
+		b = append(b, 0)
+	}
+	return append(b, makeBox("free", []byte{0, 0, 0, 0})...)
+}
+
+// TestDecodeUndersizedContainerNoSizeUnderflow checks that a container box that
+// declares a size smaller than the fixed fields preceding its children is
+// rejected with a meaningful size, on both decode paths. These boxes pass a
+// children start offset above the 8-byte header (dref, stsd and trep use +16),
+// so the declared end lands before the start and the unsigned difference used
+// to wrap to ~1.8e19 in the error message.
+func TestDecodeUndersizedContainerNoSizeUnderflow(t *testing.T) {
+	for _, name := range []string{"dref", "stsd", "trep"} {
+		for _, size := range []uint32{8, 12, 15} {
+			data := makeUndersizedBox(name, size)
+			t.Run(fmt.Sprintf("%s-%d", name, size), func(t *testing.T) {
+				check := func(label string, err error) {
+					t.Helper()
+					if err == nil {
+						t.Fatalf("%s: expected error for %s size %d, got nil", label, name, size)
+					}
+					// 18446744073709551612 and friends: uint64 wraparound.
+					if strings.Contains(err.Error(), "1844674407370955") {
+						t.Errorf("%s: size underflowed in error: %v", label, err)
+					}
+				}
+				_, errR := mp4.DecodeBox(0, bytes.NewReader(data))
+				check("DecodeBox", errR)
+				_, errS := mp4.DecodeBoxSR(0, bits.NewFixedSliceReader(data))
+				check("DecodeBoxSR", errS)
+			})
+		}
+	}
+}
+
+// TestDecodeUndersizedEsdsNoSizeUnderflow checks the same for esds, whose
+// descriptor size is computed as hdr.Size-12.
+func TestDecodeUndersizedEsdsNoSizeUnderflow(t *testing.T) {
+	for _, size := range []uint32{8, 9, 11} {
+		data := makeUndersizedBox("esds", size)
+		t.Run(fmt.Sprintf("esds-%d", size), func(t *testing.T) {
+			// The size must be rejected as a size, not incidentally by the
+			// descriptor tag check that happens to run first without the guard.
+			check := func(label string, err error) {
+				t.Helper()
+				if err == nil {
+					t.Fatalf("%s: expected error for esds size %d, got nil", label, size)
+				}
+				if !strings.Contains(err.Error(), "too small") {
+					t.Errorf("%s: want size error, got %v", label, err)
+				}
+			}
+			_, errR := mp4.DecodeBox(0, bytes.NewReader(data))
+			check("DecodeBox", errR)
+			_, errS := mp4.DecodeBoxSR(0, bits.NewFixedSliceReader(data))
+			check("DecodeBoxSR", errS)
+		})
+	}
+}

mp4/esds.go

@@ -35,14 +35,17 @@ func DecodeEsds(hdr BoxHeader, startPos uint64, r io.Reader) (Box, error) {
 
 // DecodeEsdsSR - box-specific decode
 func DecodeEsdsSR(hdr BoxHeader, startPos uint64, sr bits.SliceReader) (Box, error) {
+	if hdr.payloadLen() < 4 {
+		return nil, fmt.Errorf("esds: payload size %d is too small, needs at least 4 bytes", hdr.payloadLen())
+	}
 	versionAndFlags := sr.ReadUint32()
 	version := byte(versionAndFlags >> 24)
 
 	e := &EsdsBox{
 		Version: version,
 		Flags:   versionAndFlags & flagsMask,
 	}
-	descSize := uint32(hdr.Size - 12)
+	descSize := uint32(hdr.payloadLen() - 4)
 	var err error
 	e.ESDescriptor, err = DecodeESDescriptor(sr, descSize)
 	if err != nil {