Skip to content

Commit

Permalink
Fix out-of-memory errors with mdat boxes and remove size-based checks
Browse files Browse the repository at this point in the history
While patch abema#150 is able to prevent RAM exhaustion with the majority of
small, specially-crafted strings, it isn'effective against strings that
contain mdat boxes. A short string is able to cause RAM exhaustion by
setting the mdat box size to a big number.

This PR fixes the issue by replacing size-based checks with checks on
the effective size of the underlying buffer, that are performed by
using Seek(). In this way, an attacker may cause DoS errors if and only
if he is able to upload an amount of data equal to the size of the RAM
of the machine, and if there are no size checks before passing the
buffer to ReadBoxStructure() or Unmarshal().

Size-based checks are performed only in case of non-uint8 slices, since
it's not possible to know in advance the overall size of a generic
slice.
  • Loading branch information
aler9 committed Oct 1, 2023
1 parent efaaa51 commit 483be13
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 51 deletions.
5 changes: 3 additions & 2 deletions box_types_iso14496_12.go
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,9 @@ func (hdlr *Hdlr) OnReadName(r bitio.ReadSeeker, leftBits uint64, ctx Context) (
hdlr.Name = ""
return 0, true, nil
}
if size > 1024 {
return 0, false, errors.New("too large hdlr box")

if !readerHasSize(r, size) {
return 0, false, fmt.Errorf("not enough bits")
}

buf := make([]byte, size)
Expand Down
38 changes: 0 additions & 38 deletions box_types_iso14496_12_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"io"
"math"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -2784,43 +2783,6 @@ func TestFtypCompatibleBrands(t *testing.T) {
require.False(t, ftyp.HasCompatibleBrand(BrandISO7()))
}

func TestHdlrLargeSize(t *testing.T) {
t.Run("no-error", func(t *testing.T) {
bin := append([]byte{
0, // version
0x00, 0x00, 0x00, // flags
0x00, 0x00, 0x00, 0x00,
'v', 'i', 'd', 'e', // handler type
0x00, 0x00, 0x00, 0x00, // reserved
0x00, 0x00, 0x00, 0x00, // reserved
0x00, 0x00, 0x00, 0x00, // reserved
}, []byte(strings.Repeat("x", 1024))...)
dst := Hdlr{}
r := bytes.NewReader(bin)
n, err := Unmarshal(r, uint64(len(bin)), &dst, Context{})
require.NoError(t, err)
assert.Equal(t, uint64(len(bin)), n)
assert.Equal(t, strings.Repeat("x", 1024), dst.Name)
})

t.Run("error", func(t *testing.T) {
bin := append([]byte{
0, // version
0x00, 0x00, 0x00, // flags
0x00, 0x00, 0x00, 0x00,
'v', 'i', 'd', 'e', // handler type
0x00, 0x00, 0x00, 0x00, // reserved
0x00, 0x00, 0x00, 0x00, // reserved
0x00, 0x00, 0x00, 0x00, // reserved
}, []byte(strings.Repeat("x", 1025))...)
dst := Hdlr{}
r := bytes.NewReader(bin)
_, err := Unmarshal(r, uint64(len(bin)), &dst, Context{})
require.Error(t, err)
assert.Equal(t, "too large hdlr box", err.Error())
})
}

func TestHdlrUnmarshalHandlerName(t *testing.T) {
testCases := []struct {
name string
Expand Down
41 changes: 30 additions & 11 deletions marshaller.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,34 @@ import (
)

const (
anyVersion = math.MaxUint8
maxInitialSliceCapacity = 100 * 1024
anyVersion = math.MaxUint8
)

var ErrUnsupportedBoxVersion = errors.New("unsupported box version")

func readerHasSize(reader bitio.ReadSeeker, size uint64) bool {
pre, err := reader.Seek(0, io.SeekCurrent)
if err != nil {
return false
}

end, err := reader.Seek(0, io.SeekEnd)
if err != nil {
return false
}

if uint64(end-pre) < size {
return false
}

_, err = reader.Seek(pre, io.SeekStart)
if err != nil {
return false
}

return true
}

type marshaller struct {
writer bitio.Writer
wbits uint64
Expand Down Expand Up @@ -418,25 +440,22 @@ func (u *unmarshaller) unmarshalSlice(v reflect.Value, fi *fieldInstance) error
}
}

if length > math.MaxInt32 {
return fmt.Errorf("out of memory: requestedSize=%d", length)
}

if u.rbits%8 == 0 && elemType.Kind() == reflect.Uint8 && fi.size == 8 {
totalSize := length * uint64(fi.size) / 8
capacity := totalSize
if u.dst.GetType() != BoxTypeMdat() && capacity > maxInitialSliceCapacity {
capacity = maxInitialSliceCapacity

if !readerHasSize(u.reader, totalSize) {
return fmt.Errorf("not enough bits")
}
buf := bytes.NewBuffer(make([]byte, 0, capacity))

buf := bytes.NewBuffer(make([]byte, 0, totalSize))
if _, err := io.CopyN(buf, u.reader, int64(totalSize)); err != nil {
return err
}
slice = reflect.ValueOf(buf.Bytes())
u.rbits += uint64(totalSize) * 8

} else {
slice = reflect.MakeSlice(v.Type(), 0, int(length))
slice = reflect.MakeSlice(v.Type(), 0, 0)
for i := 0; ; i++ {
if fi.length != LengthUnlimited && uint(i) >= fi.length {
break
Expand Down

0 comments on commit 483be13

Please sign in to comment.