registry: return errors instead of panicking on perflib data (#2549)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

internal/pdh/registry/collector.go

@@ -52,9 +52,18 @@ func NewCollector[T any](object string, _ []string) (*Collector[T], error) {
 		return nil, fmt.Errorf("expected a struct, got %s", valueType)
 	}
 
+	if err := CounterNameTable.load(); err != nil {
+		return nil, err
+	}
+
+	query := MapCounterToIndex(object)
+	if query == "0" {
+		return nil, fmt.Errorf("perflib object %q not found in the counter name table", object)
+	}
+
 	collector := &Collector[T]{
 		object:         object,
-		query:          MapCounterToIndex(object),
+		query:          query,
 		nameIndexValue: -1,
 		counters:       make(map[string]Counter),
 	}
@@ -163,9 +172,18 @@ func (c *Collector[T]) Collect(dst *[]T) error {
 
 				switch perfCounter.Def.CounterType {
 				case pdh.PERF_ELAPSED_TIME:
+					// A zero frequency would divide by zero. The value is left unset.
+					if perfObject.Frequency <= 0 || counter.FieldIndexValue == -1 {
+						continue
+					}
+
 					rv.Field(counter.FieldIndexValue).
 						SetFloat(float64((perfCounter.Value - pdh.WindowsEpoch) / perfObject.Frequency))
 				case pdh.PERF_100NSEC_TIMER, pdh.PERF_PRECISION_100NS_TIMER:
+					if counter.FieldIndexValue == -1 {
+						continue
+					}
+
 					rv.Field(counter.FieldIndexValue).
 						SetFloat(float64(perfCounter.Value) * pdh.TicksToSecondScaleFactor)
 				default:

internal/pdh/registry/nametable.go

@@ -19,6 +19,7 @@ package registry
 
 import (
 	"bytes"
+	"fmt"
 	"strconv"
 	"sync"
 )
@@ -29,14 +30,14 @@ import (
 // (for many use cases the index is sufficient)
 //
 //nolint:gochecknoglobals
-var CounterNameTable = *QueryNameTable("Counter 009")
+var CounterNameTable = QueryNameTable("Counter 009")
 
 func (p *perfObjectType) LookupName() string {
 	return CounterNameTable.LookupString(p.ObjectNameTitleIndex)
 }
 
 type NameTable struct {
-	once sync.Once
+	mu sync.Mutex
 
 	name string
 
@@ -46,14 +47,22 @@ type NameTable struct {
 	}
 }
 
+// LookupString returns the name for index. It returns an empty string if the
+// index is unknown or the name table could not be loaded.
 func (t *NameTable) LookupString(index uint32) string {
-	t.initialize()
+	if t.load() != nil {
+		return ""
+	}
 
 	return t.table.index[index]
 }
 
+// LookupIndex returns the index for str. It returns 0 if the name is unknown
+// or the name table could not be loaded.
 func (t *NameTable) LookupIndex(str string) uint32 {
-	t.initialize()
+	if t.load() != nil {
+		return 0
+	}
 
 	return t.table.string[str]
 }
@@ -66,33 +75,51 @@ func QueryNameTable(tableName string) *NameTable {
 	}
 }
 
-func (t *NameTable) initialize() {
-	t.once.Do(func() {
-		t.table.index = make(map[uint32]string)
-		t.table.string = make(map[string]uint32)
+// load reads the name table from the registry. A failed load is retried on the
+// next call, so a transient error does not disable the table for good. The
+// maps are not modified once loaded.
+func (t *NameTable) load() error {
+	t.mu.Lock()
+	defer t.mu.Unlock()
 
-		buffer, err := queryRawData(t.name)
-		if err != nil {
-			panic(err)
-		}
+	if t.table.index != nil {
+		return nil
+	}
 
-		r := bytes.NewReader(buffer)
+	buffer, err := queryRawData(t.name)
+	if err != nil {
+		return fmt.Errorf("failed to query perflib name table %q: %w", t.name, err)
+	}
 
-		for {
-			index, err := readUTF16String(r)
-			if err != nil {
-				break
-			}
+	t.table.index, t.table.string = parseNameTable(buffer)
 
-			desc, err := readUTF16String(r)
-			if err != nil {
-				break
-			}
+	return nil
+}
 
-			indexInt, _ := strconv.Atoi(index)
+// parseNameTable parses a name table, a REG_MULTI_SZ of alternating index and
+// name strings. Parsing stops at the first incomplete pair.
+func parseNameTable(buffer []byte) (map[uint32]string, map[string]uint32) {
+	index := make(map[uint32]string)
+	names := make(map[string]uint32)
 
-			t.table.index[uint32(indexInt)] = desc
-			t.table.string[desc] = uint32(indexInt)
+	r := bytes.NewReader(buffer)
+
+	for {
+		indexStr, err := readUTF16String(r)
+		if err != nil {
+			break
+		}
+
+		desc, err := readUTF16String(r)
+		if err != nil {
+			break
 		}
-	})
+
+		indexInt, _ := strconv.ParseUint(indexStr, 10, 32)
+
+		index[uint32(indexInt)] = desc
+		names[desc] = uint32(indexInt)
+	}
+
+	return index, names
 }

internal/pdh/registry/perflib.go

@@ -293,36 +293,71 @@ Many objects have dependencies - if you query one of them, you often get back
 more than you asked for.
 */
 func QueryPerformanceData(query string, counterName string) ([]*PerfObject, error) {
+	// Object and counter names are resolved through the name table.
+	if err := CounterNameTable.load(); err != nil {
+		return nil, err
+	}
+
 	buffer, err := queryRawData(query)
 	if err != nil {
 		return nil, err
 	}
 
+	objects, err := parsePerformanceData(buffer, counterName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse performance data for %q: %w", query, err)
+	}
+
+	return objects, nil
+}
+
+//nolint:gochecknoglobals
+var (
+	perfDataBlockSize          = int64(binary.Size(perfDataBlock{}))
+	perfObjectTypeSize         = int64(binary.Size(perfObjectType{}))
+	perfCounterDefinitionSize  = int64(binary.Size(perfCounterDefinition{}))
+	perfInstanceDefinitionSize = int64(binary.Size(perfInstanceDefinition{}))
+	perfCounterBlockSize       = int64(binary.Size(perfCounterBlock{}))
+)
+
+// errMalformedPerformanceData is returned when the performance data contradicts
+// itself, e.g. it declares more objects than fit into the buffer. Counts,
+// offsets and lengths are checked before they size an allocation or index into
+// the buffer.
+var errMalformedPerformanceData = errors.New("malformed performance data")
+
+// parsePerformanceData parses a PERF_DATA_BLOCK as returned by queryRawData.
+// If counterName is set, only the first object with that name is returned.
+func parsePerformanceData(buffer []byte, counterName string) ([]*PerfObject, error) {
+	bufLen := int64(len(buffer))
 	r := bytes.NewReader(buffer)
 
 	// Read global header
 
 	header := new(perfDataBlock)
 
-	err = header.BinaryReadFrom(r)
-	if err != nil {
-		return nil, fmt.Errorf("failed to read performance data block for %q with: %w", query, err)
+	if err := header.BinaryReadFrom(r); err != nil {
+		return nil, fmt.Errorf("failed to read performance data block: %w", err)
 	}
 
 	// Check for "PERF" signature
 	if header.Signature != [4]uint16{80, 69, 82, 70} {
-		panic("Invalid performance block header")
+		return nil, fmt.Errorf("%w: invalid performance block signature %v", errMalformedPerformanceData, header.Signature)
 	}
 
 	// Parse the performance data
 
-	numObjects := int(header.NumObjectTypes)
+	numObjects := int64(header.NumObjectTypes)
+	objOffset := int64(header.HeaderLength)
+
+	if objOffset < perfDataBlockSize || numObjects*perfObjectTypeSize > bufLen-objOffset {
+		return nil, fmt.Errorf("%w: %d objects at offset %d do not fit into %d bytes", errMalformedPerformanceData, numObjects, objOffset, bufLen)
+	}
+
 	numFilteredObjects := 0
 
 	objects := make([]*PerfObject, numObjects)
 
-	objOffset := int64(header.HeaderLength)
-
 	for i := range numObjects {
 		_, err := r.Seek(objOffset, io.SeekStart)
 		if err != nil {
@@ -344,8 +379,17 @@ func QueryPerformanceData(query string, counterName string) ([]*PerfObject, erro
 			continue
 		}
 
-		numCounterDefs := int(obj.NumCounters)
-		numInstances := int(obj.NumInstances)
+		numCounterDefs := int64(obj.NumCounters)
+		numInstances := int64(obj.NumInstances)
+
+		if numCounterDefs*perfCounterDefinitionSize > bufLen-objOffset-perfObjectTypeSize {
+			return nil, fmt.Errorf("%w: %d counter definitions do not fit into the buffer", errMalformedPerformanceData, numCounterDefs)
+		}
+
+		// Every instance has at least an instance definition and a counter block.
+		if numInstances*(perfInstanceDefinitionSize+perfCounterBlockSize) > bufLen-objOffset {
+			return nil, fmt.Errorf("%w: %d instances do not fit into the buffer", errMalformedPerformanceData, numInstances)
+		}
 
 		// Perf objects can have no instances. The perflib differentiates
 		// between objects with instances and without, but we just create
@@ -420,7 +464,12 @@ func QueryPerformanceData(query string, counterName string) ([]*PerfObject, erro
 					return nil, err
 				}
 
-				name, _ := readUTF16StringAtPos(r, instOffset+int64(inst.NameOffset), inst.NameLength)
+				namePos := instOffset + int64(inst.NameOffset)
+				if namePos+int64(inst.NameLength) > bufLen {
+					return nil, fmt.Errorf("%w: instance name at offset %d with length %d exceeds the buffer", errMalformedPerformanceData, namePos, inst.NameLength)
+				}
+
+				name, _ := readUTF16StringAtPos(r, namePos, inst.NameLength)
 				pos := instOffset + int64(inst.ByteLength)
 
 				offset, counters, err := parseCounterBlock(buffer, r, pos, counterDefs)
@@ -463,10 +512,27 @@ func parseCounterBlock(b []byte, r io.ReadSeeker, pos int64, defs []*PerfCounter
 		return 0, nil, err
 	}
 
+	// The block length includes the length field itself. A shorter block would
+	// make the next instance overlap this one.
+	if int64(block.ByteLength) < perfCounterBlockSize {
+		return 0, nil, fmt.Errorf("%w: counter block at offset %d has length %d", errMalformedPerformanceData, pos, block.ByteLength)
+	}
+
 	counters := make([]*PerfCounter, len(defs))
 
 	for i, def := range defs {
 		valueOffset := pos + int64(def.rawData.CounterOffset)
+		size := counterValueSize(def.rawData)
+
+		end := valueOffset + size
+		if def.HasSecondValue {
+			end = valueOffset + 8 + size
+		}
+
+		if end > int64(len(b)) {
+			return 0, nil, fmt.Errorf("%w: value of counter %s at offset %d exceeds the buffer", errMalformedPerformanceData, def.Name, valueOffset)
+		}
+
 		value := convertCounterValue(def.rawData, b, valueOffset)
 		secondValue := int64(0)
 
@@ -484,6 +550,15 @@ func parseCounterBlock(b []byte, r io.ReadSeeker, pos int64, defs []*PerfCounter
 	return int64(block.ByteLength), counters, nil
 }
 
+// counterValueSize returns the number of bytes convertCounterValue reads.
+func counterValueSize(counterDef *perfCounterDefinition) int64 {
+	if counterDef.CounterSize == 8 {
+		return 8
+	}
+
+	return 4
+}
+
 func convertCounterValue(counterDef *perfCounterDefinition, buffer []byte, valueOffset int64) int64 {
 	/*
 		We can safely ignore the type since we're not interested in anything except the raw value.

internal/pdh/registry/perflib_parse_test.go

@@ -0,0 +1,265 @@
+// SPDX-License-Identifier: Apache-2.0
+//
+// Copyright The Prometheus Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build windows
+
+package registry
+
+import (
+	"bytes"
+	"encoding/binary"
+	"testing"
+
+	"github.com/prometheus-community/windows_exporter/internal/pdh"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/windows"
+)
+
+// syntheticPerfData describes a PERF_DATA_BLOCK with one object and one
+// 64-bit counter, either without instances or with a single instance.
+type syntheticPerfData struct {
+	header       perfDataBlock
+	object       perfObjectType
+	def          perfCounterDefinition
+	instance     perfInstanceDefinition
+	instanceName []uint16
+	block        perfCounterBlock
+	value        uint64
+
+	withInstance bool
+}
+
+func newSyntheticPerfData(withInstance bool) *syntheticPerfData {
+	d := &syntheticPerfData{
+		withInstance: withInstance,
+		instanceName: windows.StringToUTF16("inst"),
+		value:        42,
+	}
+
+	d.header.Signature = [4]uint16{'P', 'E', 'R', 'F'}
+	d.header.LittleEndian = 1
+	d.header.HeaderLength = uint32(perfDataBlockSize)
+	d.header.NumObjectTypes = 1
+
+	d.object.HeaderLength = uint32(perfObjectTypeSize)
+	d.object.DefinitionLength = uint32(perfObjectTypeSize + perfCounterDefinitionSize)
+	d.object.NumCounters = 1
+	d.object.NumInstances = -1 // PERF_NO_INSTANCES
+	d.object.PerfFreq = 10_000_000
+
+	d.def.ByteLength = uint32(perfCounterDefinitionSize)
+	d.def.CounterType = pdh.PERF_COUNTER_LARGE_RAWCOUNT
+	d.def.CounterSize = 8
+	// The value follows the 4-byte block length and 4 bytes of padding.
+	d.def.CounterOffset = 8
+
+	d.block.ByteLength = 16
+
+	if withInstance {
+		nameLength := uint32(len(d.instanceName) * 2)
+
+		d.object.NumInstances = 1
+		d.instance.NameOffset = uint32(perfInstanceDefinitionSize)
+		d.instance.NameLength = nameLength
+		// The name is padded to 8 bytes.
+		d.instance.ByteLength = uint32(perfInstanceDefinitionSize) + (nameLength+7)&^7
+	}
+
+	return d
+}
+
+func (d *syntheticPerfData) bytes(tb testing.TB) []byte {
+	tb.Helper()
+
+	var body bytes.Buffer
+
+	write := func(v any) {
+		require.NoError(tb, binary.Write(&body, bo, v))
+	}
+
+	write(d.def)
+
+	if d.withInstance {
+		write(d.instance)
+		write(d.instanceName)
+		body.Write(make([]byte, int(d.instance.ByteLength)-int(perfInstanceDefinitionSize)-len(d.instanceName)*2))
+	}
+
+	write(d.block)
+	body.Write(make([]byte, 4))
+	write(d.value)
+
+	object := d.object
+	if object.TotalByteLength == 0 {
+		object.TotalByteLength = uint32(perfObjectTypeSize) + uint32(body.Len())
+	}
+
+	header := d.header
+	if header.TotalByteLength == 0 {
+		header.TotalByteLength = uint32(perfDataBlockSize) + object.TotalByteLength
+	}
+
+	var buf bytes.Buffer
+
+	require.NoError(tb, binary.Write(&buf, bo, header))
+	require.NoError(tb, binary.Write(&buf, bo, object))
+	buf.Write(body.Bytes())
+
+	return buf.Bytes()
+}
+
+func TestParsePerformanceData(t *testing.T) {
+	t.Parallel()
+
+	for _, withInstance := range []bool{false, true} {
+		objects, err := parsePerformanceData(newSyntheticPerfData(withInstance).bytes(t), "")
+		require.NoError(t, err)
+		require.Len(t, objects, 1)
+		require.Len(t, objects[0].Instances, 1)
+		require.Len(t, objects[0].Instances[0].Counters, 1)
+		require.Equal(t, int64(42), objects[0].Instances[0].Counters[0].Value)
+
+		if withInstance {
+			require.Equal(t, "inst", objects[0].Instances[0].Name)
+		}
+	}
+}
+
+// TestParsePerformanceDataMalformed checks that inconsistent performance data
+// is reported as an error. The parser runs during Build of the process
+// collector, where a panic would end the exporter.
+func TestParsePerformanceDataMalformed(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name         string
+		withInstance bool
+		mutate       func(d *syntheticPerfData)
+	}{
+		{"invalid signature", false, func(d *syntheticPerfData) { d.header.Signature[0] = 'X' }},
+		{"header length too short", false, func(d *syntheticPerfData) { d.header.HeaderLength = 0 }},
+		{"too many objects", false, func(d *syntheticPerfData) { d.header.NumObjectTypes = 1 << 31 }},
+		{"too many counters", false, func(d *syntheticPerfData) { d.object.NumCounters = 1 << 30 }},
+		{"too many instances", true, func(d *syntheticPerfData) { d.object.NumInstances = 1 << 30 }},
+		{"counter offset out of range", false, func(d *syntheticPerfData) { d.def.CounterOffset = 1 << 31 }},
+		{"counter offset out of range with instance", true, func(d *syntheticPerfData) { d.def.CounterOffset = 1 << 20 }},
+		{"instance name out of range", true, func(d *syntheticPerfData) { d.instance.NameLength = 1 << 31 }},
+		{"empty counter block", true, func(d *syntheticPerfData) { d.block.ByteLength = 0 }},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			d := newSyntheticPerfData(tc.withInstance)
+			tc.mutate(d)
+
+			buffer := d.bytes(t)
+
+			var err error
+
+			require.NotPanics(t, func() { _, err = parsePerformanceData(buffer, "") })
+			require.ErrorIs(t, err, errMalformedPerformanceData)
+		})
+	}
+}
+
+func TestParsePerformanceDataTruncated(t *testing.T) {
+	t.Parallel()
+
+	for _, withInstance := range []bool{false, true} {
+		buffer := newSyntheticPerfData(withInstance).bytes(t)
+
+		for n := range buffer {
+			var err error
+
+			require.NotPanics(t, func() { _, err = parsePerformanceData(buffer[:n], "") }, "length %d", n)
+			require.Error(t, err, "length %d", n)
+		}
+	}
+}
+
+// TestParsePerformanceDataLive parses the data of a real object.
+func TestParsePerformanceDataLive(t *testing.T) {
+	t.Parallel()
+
+	buffer := livePerformanceData(t, "Process")
+
+	objects, err := parsePerformanceData(buffer, "Process")
+	require.NoError(t, err)
+	require.Len(t, objects, 1)
+	require.NotEmpty(t, objects[0].Instances)
+	require.NotEmpty(t, objects[0].CounterDefs)
+}
+
+func livePerformanceData(tb testing.TB, object string) []byte {
+	tb.Helper()
+
+	require.NoError(tb, CounterNameTable.load())
+
+	buffer, err := queryRawData(MapCounterToIndex(object))
+	require.NoError(tb, err)
+
+	return buffer
+}
+
+func FuzzParsePerformanceData(f *testing.F) {
+	f.Add(newSyntheticPerfData(false).bytes(f))
+	f.Add(newSyntheticPerfData(true).bytes(f))
+	f.Add(livePerformanceData(f, "System"))
+	f.Add([]byte{})
+
+	f.Fuzz(func(_ *testing.T, data []byte) {
+		// Must not panic on any input.
+		_, _ = parsePerformanceData(data, "")
+	})
+}
+
+func TestParseNameTable(t *testing.T) {
+	t.Parallel()
+
+	multiSZ := func(s ...string) []byte {
+		var buf bytes.Buffer
+
+		for _, v := range s {
+			require.NoError(t, binary.Write(&buf, bo, windows.StringToUTF16(v)))
+		}
+
+		return buf.Bytes()
+	}
+
+	index, names := parseNameTable(multiSZ("1", "1847", "2", "System", "4", "Memory"))
+	require.Equal(t, map[uint32]string{1: "1847", 2: "System", 4: "Memory"}, index)
+	require.Equal(t, uint32(4), names["Memory"])
+
+	// An incomplete pair and an odd trailing byte are ignored.
+	buffer := append(multiSZ("2", "System", "4"), 'M')
+	index, _ = parseNameTable(buffer)
+	require.Equal(t, map[uint32]string{2: "System"}, index)
+
+	index, _ = parseNameTable(nil)
+	require.Empty(t, index)
+}
+
+func TestNewCollectorUnknownObject(t *testing.T) {
+	t.Parallel()
+
+	type counterValues struct {
+		Name string
+
+		ProcessorQueueLength float64 `perfdata:"Processor Queue Length"`
+	}
+
+	_, err := NewCollector[counterValues]("Object That Does Not Exist", nil)
+	require.ErrorContains(t, err, "not found in the counter name table")
+}

internal/pdh/registry/utf16.go

@@ -43,24 +43,19 @@ func readUTF16StringAtPos(r io.ReadSeeker, absPos int64, length uint32) (string,
 
 // readUTF16String Reads a null-terminated UTF16 string at the current offset.
 func readUTF16String(r io.Reader) (string, error) {
-	var err error
-
 	b := make([]byte, 2)
 	out := make([]uint16, 0, 100)
 
-	for i := 0; err == nil; i += 2 {
-		_, err = r.Read(b)
+	for {
+		// A short read at the end of the buffer is an error, not a code unit.
+		if _, err := io.ReadFull(r, b); err != nil {
+			return "", err
+		}
 
 		if b[0] == 0 && b[1] == 0 {
-			break
+			return windows.UTF16ToString(out), nil
 		}
 
 		out = append(out, bo.Uint16(b))
 	}
-
-	if err != nil {
-		return "", err
-	}
-
-	return windows.UTF16ToString(out), nil
 }