Harden the USDT note parser against malformed input

pkg/php/extension/usdt/util.go

@@ -16,6 +16,12 @@ import (
 // "this probe is absent" apart from "this binary could not be read".
 var ErrProbeNotFound = errors.New("probe not found")
 
+// maxNoteDescSize bounds a single stapsdt note descriptor. A real descriptor is
+// three addresses plus a few short strings, so this is far larger than any
+// legitimate note; it exists to reject a corrupt descsz before it is used to
+// size an allocation.
+const maxNoteDescSize int32 = 64 * 1024
+
 // Note represents a SystemTap note.
 type Note struct {
 	Location              uint64
@@ -52,11 +58,14 @@ func getLocationFromProbe(path, provider, probe string) (*Note, error) {
 		return nil, errors.New("SDT note section not found")
 	}
 
-	addrsz := 4
-	if f.Class == elf.ELFCLASS64 {
-		addrsz = 8
+	// Only 64-bit binaries are supported: the extension and addon are always
+	// built 64-bit, and the address reads below assume eight-byte fields.
+	if f.Class != elf.ELFCLASS64 {
+		return nil, fmt.Errorf("unsupported ELF class %s: only 64-bit binaries are supported", f.Class)
 	}
 
+	addrsz := 8
+
 	r := sec.Open()
 	base := sdtBaseAddr(f)
 	for {
@@ -70,11 +79,23 @@ func getLocationFromProbe(path, provider, probe string) (*Note, error) {
 			return nil, err
 		}
 
+		if namesz < 0 {
+			return nil, fmt.Errorf("malformed stapsdt note: negative namesz: %d", namesz)
+		}
+
 		err = binary.Read(r, f.ByteOrder, &descsz)
 		if err != nil {
 			return nil, err
 		}
 
+		// A valid descriptor holds at least the three addresses; reject anything
+		// smaller or implausibly large before it sizes the allocation below, so a
+		// corrupt note cannot wrap to a huge make or leave the field reads that
+		// follow out of bounds.
+		if descsz < int32(3*addrsz) || descsz > maxNoteDescSize {
+			return nil, fmt.Errorf("malformed stapsdt note: descsz out of range: %d", descsz)
+		}
+
 		// skip note type
 		_, err := r.Seek(4, io.SeekCurrent)
 		if err != nil {
@@ -97,10 +118,16 @@ func getLocationFromProbe(path, provider, probe string) (*Note, error) {
 			return nil, err
 		}
 
+		d, err := parseNoteDesc(desc, addrsz, f.ByteOrder)
+		if err != nil {
+			return nil, err
+		}
+
 		note := Note{
-			Location:  f.ByteOrder.Uint64(desc[0:addrsz]),
-			Base:      f.ByteOrder.Uint64(desc[addrsz : 2*addrsz]),
-			Semaphore: f.ByteOrder.Uint64(desc[2*addrsz : 3*addrsz]),
+			Location:  d.location,
+			Base:      d.base,
+			Semaphore: d.semaphore,
+			Args:      d.args,
 			bo:        f.ByteOrder,
 		}
 
@@ -123,30 +150,69 @@ func getLocationFromProbe(path, provider, probe string) (*Note, error) {
 			}
 		}
 
-		idx := 3 * addrsz
-		providersz := bytes.IndexByte(desc[idx:], 0)
-		pv := string(desc[idx : idx+providersz])
+		if provider == d.provider && probe == d.probe {
+			return &note, nil
+		}
+	}
+
+	return nil, fmt.Errorf("%w: %s in provider %s", ErrProbeNotFound, probe, provider)
+}
+
+// noteDesc is the decoded body of one stapsdt note.
+type noteDesc struct {
+	location  uint64
+	base      uint64
+	semaphore uint64
+	provider  string
+	probe     string
+	args      string
+}
 
-		idx += providersz + 1
-		probesz := bytes.IndexByte(desc[idx:], 0)
-		pb := string(desc[idx : idx+probesz])
+// parseNoteDesc decodes one stapsdt note descriptor: three addresses followed
+// by the null-terminated provider, probe and argument strings. It returns an
+// error rather than panicking on a descriptor too short for the addresses or
+// carrying an unterminated provider or probe name.
+func parseNoteDesc(desc []byte, addrsz int, bo binary.ByteOrder) (noteDesc, error) {
+	if addrsz != 8 {
+		return noteDesc{}, fmt.Errorf("unsupported address size: %d", addrsz)
+	}
 
-		// The arguments string follows immediately after the probe name's null terminator.
-		idx += probesz + 1
-		if idx < len(desc) {
-			argssz := bytes.IndexByte(desc[idx:], 0)
-			if argssz < 0 {
-				argssz = len(desc) - idx
-			}
-			note.Args = string(desc[idx : idx+argssz])
-		}
+	if len(desc) < 3*addrsz {
+		return noteDesc{}, fmt.Errorf("malformed stapsdt note: descriptor too short: %d", len(desc))
+	}
 
-		if provider == pv && probe == pb {
-			return &note, nil
+	d := noteDesc{
+		location:  bo.Uint64(desc[0:addrsz]),
+		base:      bo.Uint64(desc[addrsz : 2*addrsz]),
+		semaphore: bo.Uint64(desc[2*addrsz : 3*addrsz]),
+	}
+
+	idx := 3 * addrsz
+
+	providersz := bytes.IndexByte(desc[idx:], 0)
+	if providersz < 0 {
+		return noteDesc{}, errors.New("malformed stapsdt note: unterminated provider name")
+	}
+	d.provider = string(desc[idx : idx+providersz])
+
+	idx += providersz + 1
+	probesz := bytes.IndexByte(desc[idx:], 0)
+	if probesz < 0 {
+		return noteDesc{}, errors.New("malformed stapsdt note: unterminated probe name")
+	}
+	d.probe = string(desc[idx : idx+probesz])
+
+	// The arguments string follows immediately after the probe name's null terminator.
+	idx += probesz + 1
+	if idx < len(desc) {
+		argssz := bytes.IndexByte(desc[idx:], 0)
+		if argssz < 0 {
+			argssz = len(desc) - idx
 		}
+		d.args = string(desc[idx : idx+argssz])
 	}
 
-	return nil, fmt.Errorf("%w: %s in provider %s", ErrProbeNotFound, probe, provider)
+	return d, nil
 }
 
 func offset(f *elf.File, addr uint64) uint64 {

pkg/php/extension/usdt/util_test.go

@@ -0,0 +1,95 @@
+package usdt
+
+import (
+	"encoding/binary"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// makeDesc builds a well-formed note descriptor: three little-endian addresses
+// followed by null-terminated provider, probe and args strings.
+func makeDesc(location, base, semaphore uint64, provider, probe, args string) []byte {
+	desc := make([]byte, 3*8)
+	binary.LittleEndian.PutUint64(desc[0:8], location)
+	binary.LittleEndian.PutUint64(desc[8:16], base)
+	binary.LittleEndian.PutUint64(desc[16:24], semaphore)
+
+	desc = append(desc, []byte(provider)...)
+	desc = append(desc, 0)
+	desc = append(desc, []byte(probe)...)
+	desc = append(desc, 0)
+	desc = append(desc, []byte(args)...)
+	desc = append(desc, 0)
+
+	return desc
+}
+
+func TestParseNoteDesc(t *testing.T) {
+	desc := makeDesc(0x1000, 0x2000, 0x3000, "compass", "fpm_function", "-8@%rax -8@%rcx")
+
+	d, err := parseNoteDesc(desc, 8, binary.LittleEndian)
+	require.NoError(t, err)
+
+	assert.Equal(t, uint64(0x1000), d.location)
+	assert.Equal(t, uint64(0x2000), d.base)
+	assert.Equal(t, uint64(0x3000), d.semaphore)
+	assert.Equal(t, "compass", d.provider)
+	assert.Equal(t, "fpm_function", d.probe)
+	assert.Equal(t, "-8@%rax -8@%rcx", d.args)
+}
+
+func TestParseNoteDesc_NoArgs(t *testing.T) {
+	desc := makeDesc(1, 2, 3, "compass", "canary", "")
+
+	d, err := parseNoteDesc(desc, 8, binary.LittleEndian)
+	require.NoError(t, err)
+
+	assert.Equal(t, "canary", d.probe)
+	assert.Empty(t, d.args)
+}
+
+func TestParseNoteDesc_Rejects(t *testing.T) {
+	tests := []struct {
+		name string
+		desc []byte
+	}{
+		{name: "too short for the addresses", desc: make([]byte, 8)},
+		{name: "empty", desc: nil},
+		{
+			name: "unterminated provider",
+			desc: append(make([]byte, 3*8), []byte("compass")...), // no null terminator
+		},
+		{
+			name: "unterminated probe",
+			desc: func() []byte {
+				d := make([]byte, 3*8)
+				d = append(d, []byte("compass")...)
+				d = append(d, 0)
+				d = append(d, []byte("fpm_function")...) // no terminator
+				return d
+			}(),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := parseNoteDesc(tt.desc, 8, binary.LittleEndian)
+			assert.Error(t, err)
+		})
+	}
+}
+
+// FuzzParseNoteDesc asserts no descriptor body, however malformed, can panic.
+func FuzzParseNoteDesc(f *testing.F) {
+	f.Add(makeDesc(0x1000, 0x2000, 0x3000, "compass", "fpm_function", "-8@%rax"))
+	f.Add(make([]byte, 3*8))
+	f.Add([]byte{})
+	f.Add([]byte("compass\x00fpm_function\x00"))
+
+	f.Fuzz(func(_ *testing.T, desc []byte) {
+		// The contract under test: parseNoteDesc returns, it never panics.
+		_, _ = parseNoteDesc(desc, 8, binary.LittleEndian)
+	})
+}