internal/trace/v2: avoid several panics for malformed traces

This addresses some panics (out of bounds slice accesses and nil pointer
dereferences) when parsing malformed data. These were found via light
fuzzing, not by any rigorous means, and more potential panics probably
exist.

Fixes #64878.
Fixes #64879.

Change-Id: I4085788ba7dc91fec62e4abd88f50777577db42f
Reviewed-on: https://go-review.googlesource.com/c/go/+/552995
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

src/internal/trace/v2/base.go

@@ -9,6 +9,7 @@ package trace
 
 import (
 	"fmt"
+	"math"
 	"strings"
 
 	"internal/trace/v2/event"
@@ -123,8 +124,12 @@ func (d *dataTable[EI, E]) compactify() {
 			minID = id
 		}
 	}
+	if maxID >= math.MaxInt {
+		// We can't create a slice big enough to hold maxID elements
+		return
+	}
 	// We're willing to waste at most 2x memory.
-	if int(maxID-minID) > 2*len(d.sparse) {
+	if int(maxID-minID) > max(len(d.sparse), 2*len(d.sparse)) {
 		return
 	}
 	if int(minID) > len(d.sparse) {
@@ -146,7 +151,7 @@ func (d *dataTable[EI, E]) get(id EI) (E, bool) {
 	if id == 0 {
 		return *new(E), true
 	}
-	if int(id) < len(d.dense) {
+	if uint64(id) < uint64(len(d.dense)) {
 		if d.present[id/8]&(uint8(1)<<(id%8)) != 0 {
 			return d.dense[id], true
 		}

src/internal/trace/v2/batchcursor.go

@@ -68,7 +68,7 @@ func readTimedBaseEvent(b []byte, e *baseEvent) (int, timestamp, error) {
 	// Get the event type.
 	typ := event.Type(b[0])
 	specs := go122.Specs()
-	if int(typ) > len(specs) {
+	if int(typ) >= len(specs) {
 		return 0, 0, fmt.Errorf("found invalid event type: %v", typ)
 	}
 	e.typ = typ
@@ -82,11 +82,17 @@ func readTimedBaseEvent(b []byte, e *baseEvent) (int, timestamp, error) {
 
 	// Read timestamp diff.
 	ts, nb := binary.Uvarint(b[n:])
+	if nb <= 0 {
+		return 0, 0, fmt.Errorf("found invalid uvarint for timestamp")
+	}
 	n += nb
 
 	// Read the rest of the arguments.
 	for i := 0; i < len(spec.Args)-1; i++ {
 		arg, nb := binary.Uvarint(b[n:])
+		if nb <= 0 {
+			return 0, 0, fmt.Errorf("found invalid uvarint")
+		}
 		e.args[i] = arg
 		n += nb
 	}

src/internal/trace/v2/order.go

@@ -92,6 +92,9 @@ func (o *ordering) advance(ev *baseEvent, evt *evTable, m ThreadID, gen uint64)
 	case go122.EvProcStatus:
 		pid := ProcID(ev.args[0])
 		status := go122.ProcStatus(ev.args[1])
+		if int(status) >= len(go122ProcStatus2ProcState) {
+			return curCtx, false, fmt.Errorf("invalid status for proc %d: %d", pid, status)
+		}
 		oldState := go122ProcStatus2ProcState[status]
 		if s, ok := o.pStates[pid]; ok {
 			if status == go122.ProcSyscallAbandoned && s.status == go122.ProcSyscall {
@@ -268,6 +271,10 @@ func (o *ordering) advance(ev *baseEvent, evt *evTable, m ThreadID, gen uint64)
 		gid := GoID(ev.args[0])
 		mid := ThreadID(ev.args[1])
 		status := go122.GoStatus(ev.args[2])
+
+		if int(status) >= len(go122GoStatus2GoState) {
+			return curCtx, false, fmt.Errorf("invalid status for goroutine %d: %d", gid, status)
+		}
 		oldState := go122GoStatus2GoState[status]
 		if s, ok := o.gStates[gid]; ok {
 			if s.status != status {
@@ -758,7 +765,11 @@ func (o *ordering) advance(ev *baseEvent, evt *evTable, m ThreadID, gen uint64)
 		// ever reference curCtx.P. However, be lenient about this like we are with
 		// GCMarkAssistActive; there's no reason the runtime couldn't change to block
 		// in the middle of a sweep.
-		if err := o.pStates[pid].activeRange(makeRangeType(typ, 0), gen == o.initialGen); err != nil {
+		pState, ok := o.pStates[pid]
+		if !ok {
+			return curCtx, false, fmt.Errorf("encountered GCSweepActive for unknown proc %d", pid)
+		}
+		if err := pState.activeRange(makeRangeType(typ, 0), gen == o.initialGen); err != nil {
 			return curCtx, false, err
 		}
 		return curCtx, true, nil
@@ -790,7 +801,11 @@ func (o *ordering) advance(ev *baseEvent, evt *evTable, m ThreadID, gen uint64)
 		// N.B. Like GoStatus, this can happen at any time, because it can
 		// reference a non-running goroutine. Don't check anything about the
 		// current scheduler context.
-		if err := o.gStates[gid].activeRange(makeRangeType(typ, 0), gen == o.initialGen); err != nil {
+		gState, ok := o.gStates[gid]
+		if !ok {
+			return curCtx, false, fmt.Errorf("uninitialized goroutine %d found during %s", gid, go122.EventString(typ))
+		}
+		if err := gState.activeRange(makeRangeType(typ, 0), gen == o.initialGen); err != nil {
 			return curCtx, false, err
 		}
 		return curCtx, true, nil
@@ -917,6 +932,10 @@ func (s *gState) beginRegion(r userRegion) error {
 
 // endRegion ends a user region on the goroutine.
 func (s *gState) endRegion(r userRegion) error {
+	if len(s.regions) == 0 {
+		// We do not know about regions that began before tracing started.
+		return nil
+	}
 	if next := s.regions[len(s.regions)-1]; next != r {
 		return fmt.Errorf("misuse of region in goroutine %v: region end %v when the inner-most active region start event is %v", s.id, r, next)
 	}

src/internal/trace/v2/reader.go

@@ -157,6 +157,9 @@ func (r *Reader) ReadEvent() (e Event, err error) {
 	}
 	// Try to advance the head of the frontier, which should have the minimum timestamp.
 	// This should be by far the most common case
+	if len(r.frontier) == 0 {
+		return Event{}, fmt.Errorf("broken trace: frontier is empty:\n[gen=%d]\n\n%s\n%s\n", r.gen.gen, dumpFrontier(r.frontier), dumpOrdering(&r.order))
+	}
 	bc := r.frontier[0]
 	if ctx, ok, err := r.order.advance(&bc.ev, r.gen.evTable, bc.m, r.gen.gen); err != nil {
 		return Event{}, err

src/internal/trace/v2/reader_test.go

@@ -46,6 +46,53 @@ func TestReaderGolden(t *testing.T) {
 	}
 }
 
+func FuzzReader(f *testing.F) {
+	// Currently disabled because the parser doesn't do much validation and most
+	// getters can be made to panic. Turn this on once the parser is meant to
+	// reject invalid traces.
+	const testGetters = false
+
+	f.Fuzz(func(t *testing.T, b []byte) {
+		r, err := trace.NewReader(bytes.NewReader(b))
+		if err != nil {
+			return
+		}
+		for {
+			ev, err := r.ReadEvent()
+			if err != nil {
+				break
+			}
+
+			if !testGetters {
+				continue
+			}
+			// Make sure getters don't do anything that panics
+			switch ev.Kind() {
+			case trace.EventLabel:
+				ev.Label()
+			case trace.EventLog:
+				ev.Log()
+			case trace.EventMetric:
+				ev.Metric()
+			case trace.EventRangeActive, trace.EventRangeBegin:
+				ev.Range()
+			case trace.EventRangeEnd:
+				ev.Range()
+				ev.RangeAttributes()
+			case trace.EventStateTransition:
+				ev.StateTransition()
+			case trace.EventRegionBegin, trace.EventRegionEnd:
+				ev.Region()
+			case trace.EventTaskBegin, trace.EventTaskEnd:
+				ev.Task()
+			case trace.EventSync:
+			case trace.EventStackSample:
+			case trace.EventBad:
+			}
+		}
+	})
+}
+
 func testReader(t *testing.T, tr io.Reader, exp *testtrace.Expectation) {
 	r, err := trace.NewReader(tr)
 	if err != nil {

src/internal/trace/v2/testdata/fuzz/FuzzReader/0cb1786dee0f090b

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x01\x0100\x85\x00\x190000\x01\x0100\x88\x00\b0000000")

src/internal/trace/v2/testdata/fuzz/FuzzReader/1e45307d5b2ec36d

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x01000\x85\x00\b0001")

src/internal/trace/v2/testdata/fuzz/FuzzReader/2b05796f9b2fc48d

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x01\x0100\x85\x00-0000\x01\x0100\x88\x00\b0000000")

src/internal/trace/v2/testdata/fuzz/FuzzReader/2b9be9aebe08d511

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x01\x0100\x85\x00\x0f00\x120\x01\x0100\x88\x00\b0000000")

src/internal/trace/v2/testdata/fuzz/FuzzReader/344331b314da0b08

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x01\x0100\x85\x00\b0000\x01\x01\xff00\xb8\x00\x1900\xff\xff\xff\xff\xff\xff\xff\xff\xff\x01\x04\x1900\xff\xff\xff\xff\xff\xff\xff\xff\xff\x01\x04\x1900\xff\xff\xff\xff\xff\xff\xff\xff\xff\x01\x04\x1901\xff\xff\xff\xff\xff\xff\xff\xff0\x800")

src/internal/trace/v2/testdata/fuzz/FuzzReader/365d7b5b633b3f97

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x0100\x8c0\x85\x00\b0000")

src/internal/trace/v2/testdata/fuzz/FuzzReader/56f073e57903588c

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x01\x0100\x85\x00\x1f0000\x01\x0100\x88\x00\b0000000")

src/internal/trace/v2/testdata/fuzz/FuzzReader/aeb749b6bc317b66

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x01000\x85\x00\b0000")

src/internal/trace/v2/testdata/fuzz/FuzzReader/closing-unknown-region

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x01\x01\xff\xff\xff\xff\xff\xff\xff\xff\xff\x01\x87ߕ\xb4\x99\xb2\x06\x05\b\xa8ֹ\a\x01\x01\xf6\x9f\n\x9fÕ\xb4\x99\xb2\x06\x11\r\xa7\x02\x00\x01\x19\x05\x01\xf6\x9f\n\x02+\x04\x01\x00\x00")

src/internal/trace/v2/testdata/fuzz/FuzzReader/d478e18d2d6756b7

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x01\x0100\x85\x00\"0000\x01\x0100\x88\x00\b0000000")

src/internal/trace/v2/testdata/fuzz/FuzzReader/d91203cd397aa0bc

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x01001\x85\x00\b0000")

src/internal/trace/v2/testdata/fuzz/FuzzReader/invalid-proc-state

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x01\x01\xff\xff\xff\xff\xff\xff\xff\xff\xff\x01\x87ߕ\xb4\x99\xb2\x06\x05\b\xa8ֹ\a\x01\x01\xff\xff\xff\xff\xff\xff\xff\xff\xff\x01\x94镴\x99\xb2\x06\x05\r\xa7\x02\x00E")

src/internal/trace/v2/testdata/fuzz/FuzzReader/large-id

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x01\x01\xff\xff\xff\xff\xff\xff\xff\xff\xff\x01\x87ߕ\xb4\x99\xb2\x06\x05\b\xa8ֹ\a\x01\x01\xff\xff\xff\xff\xff\xff\xff\xff\xff\x01\x94镴\x99\xb2\x06\f\x02\x03\xff\xff\xff\xff\xff\xff\xff\x9f\x1d\x00")

src/internal/trace/v2/testdata/fuzz/FuzzReader/malformed-timestamp

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("go 1.22 trace\x00\x00\x00\x01\x01\xff\xff\xff\xff\xff\xff\xff\xff\xff\x01\x87ߕ\xb4\x99\xb2\x06\x05\b\xa8ֹ\a\x01\x01\xfa\x9f\n\xa5ѕ\xb4\x99\xb2\x06\x0e\n\x97\x96\x96\x96\x96\x96\x96\x96\x96\x96\x01\x01\x01")