diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f5354c0..082d17b7 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/mp4/container.go b/mp4/container.go index c46d40b1..29380a81 100644 --- a/mp4/container.go +++ b/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() diff --git a/mp4/container_test.go b/mp4/container_test.go index 001810a7..054f649f 100644 --- a/mp4/container_test.go +++ b/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) + }) + } +} diff --git a/mp4/esds.go b/mp4/esds.go index 2b3c2a24..b90bd441 100644 --- a/mp4/esds.go +++ b/mp4/esds.go @@ -35,6 +35,9 @@ 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) @@ -42,7 +45,7 @@ func DecodeEsdsSR(hdr BoxHeader, startPos uint64, sr bits.SliceReader) (Box, err 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 {