Skip to content

Commit

Permalink
Set size limit when allocating binary offsets.
Browse files Browse the repository at this point in the history
Fixes tests and potential real life panics, but is not ideal.
This is a temporary solution.

For #28
  • Loading branch information
groob committed Apr 25, 2020
1 parent 7a4ce50 commit 2f574f4
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 24 deletions.
22 changes: 8 additions & 14 deletions binary_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,7 @@ type binaryParser struct {
io.ReadSeeker // reader for plist data
}

// safeUint64Slice returns a slice of uint64. If the desired size of the slice
// is too large to fit in memory, it returns an error rather than panicking.
func safeUint64Slice(size uint64) (slice []uint64, err error) {
defer func() {
if r := recover(); r != nil {
slice = nil
err = r.(error)
}
}()
return make([]uint64, size), nil
}
const numObjectsMax = 4 << 20

// newBinaryParser takes in a ReadSeeker for the bytes of a binary plist and
// returns a parser after reading the offset table and trailer.
Expand All @@ -58,10 +48,14 @@ func newBinaryParser(r io.ReadSeeker) (*binaryParser, error) {
if _, err := bp.Seek(int64(bp.OffsetTableOffset), io.SeekStart); err != nil {
return nil, fmt.Errorf("plist: couldn't seek to start of offset table: %v", err)
}
var err error
if bp.OffsetTable, err = safeUint64Slice(bp.NumObjects); err != nil {
return nil, err

// numObjectsMax is arbitrary. Please fix.
// TODO(github.com/groob/plist/issues/28)
if bp.NumObjects > numObjectsMax {
return nil, fmt.Errorf("plist: offset size larger than expected %d", numObjectsMax)
}

bp.OffsetTable = make([]uint64, bp.NumObjects)
if bp.OffsetIntSize > 8 {
return nil, fmt.Errorf("plist: can't decode when offset int size (%d) is greater than 8", bp.OffsetIntSize)
}
Expand Down
26 changes: 16 additions & 10 deletions decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,18 +456,24 @@ func TestUnmarshaler(t *testing.T) {
}

func TestFuzzCrashers(t *testing.T) {
infos, err := ioutil.ReadDir("testdata/crashers")
dir := filepath.Join("testdata", "crashers")
testDir, err := ioutil.ReadDir(dir)
if err != nil {
t.Fatal(err)
t.Fatalf("reading dir %q: %s", dir, err)
}

for _, info := range infos {
crasher, err := ioutil.ReadFile(filepath.Join("testdata", "crashers", info.Name()))
if err != nil {
t.Fatal(err)
}
var i interface{}
Unmarshal(crasher, &i)
}
for _, tc := range testDir {
tc := tc
t.Run(tc.Name(), func(t *testing.T) {
t.Parallel()

crasher, err := ioutil.ReadFile(filepath.Join("testdata", "crashers", tc.Name()))
if err != nil {
t.Fatal(err)
}

var i interface{}
Unmarshal(crasher, &i)
})
}
}

0 comments on commit 2f574f4

Please sign in to comment.