providers/darwin - kern.procargs2 guard against runtime panic (#172)

Prior to this change kern_procargs iterated over the data based
on the argc value without checking if the underlying slice held
enough args.

To prevent a runtime error this adds a check to verify there is more
data before trying to index another argument.

Add a fuzz test to check for panics in the parsing code for
kern.procargs2.

Relates #173

.gitignore

@@ -6,8 +6,6 @@
 _obj
 
 *TEST.out
-main.retry
-testing/ssh_config
-testing/ve
 
-build/
+build/
+**/testdata/fuzz

CHANGELOG.md

@@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Prevent possible runtime panic while reading process arguments on darwin. [#172](https://github.com/elastic/go-sysinfo/pull/172)
+
 ## [1.10.1]
 
 ### Added

providers/darwin/process_darwin.go

@@ -26,6 +26,7 @@ import (
 	"fmt"
 	"os"
 	"strconv"
+	"strings"
 	"syscall"
 	"time"
 
@@ -34,6 +35,8 @@ import (
 	"github.com/elastic/go-sysinfo/types"
 )
 
+var errInvalidProcargs2Data = errors.New("invalid kern.procargs2 data")
+
 func (s darwinSystem) Processes() ([]types.Process, error) {
 	ps, err := unix.SysctlKinfoProcSlice("kern.proc.all")
 	if err != nil {
@@ -171,8 +174,6 @@ func (p *process) Memory() (types.MemoryInfo, error) {
 	}, nil
 }
 
-var nullTerminator = []byte{0}
-
 // wrapper around sysctl KERN_PROCARGS2
 // callbacks params are optional,
 // up to the caller as to which pieces of data they want
@@ -186,32 +187,35 @@ func kern_procargs(pid int, p *process) error {
 		}
 		return err
 	}
-	buf := bytes.NewBuffer(data)
 
+	return parseKernProcargs2(data, p)
+}
+
+func parseKernProcargs2(data []byte, p *process) error {
 	// argc
-	var argc int32
-	if err := binary.Read(buf, binary.LittleEndian, &argc); err != nil {
-		return err
+	if len(data) < 4 {
+		return errInvalidProcargs2Data
 	}
+	argc := binary.LittleEndian.Uint32(data)
+	data = data[4:]
 
 	// exe
-	lines := bytes.Split(buf.Bytes(), nullTerminator)
-	p.exe = string(lines[0])
+	lines := strings.Split(string(data), "\x00")
+	p.exe = lines[0]
 	lines = lines[1:]
 
-	// skip nulls
+	// Skip nulls that may be appended after the exe.
 	for len(lines) > 0 {
-		if len(lines[0]) == 0 {
-			lines = lines[1:]
-			continue
+		if lines[0] != "" {
+			break
 		}
-		break
+		lines = lines[1:]
 	}
 
-	// args
-	for i := 0; i < int(argc); i++ {
-		p.args = append(p.args, string(lines[0]))
-		lines = lines[1:]
+	// argv
+	if c := min(argc, uint32(len(lines))); c > 0 {
+		p.args = lines[:c]
+		lines = lines[c:]
 	}
 
 	// env vars
@@ -221,13 +225,8 @@ func kern_procargs(pid int, p *process) error {
 			break
 		}
 
-		parts := bytes.SplitN(l, []byte{'='}, 2)
-		key := string(parts[0])
-		var value string
-		if len(parts) == 2 {
-			value = string(parts[1])
-		}
-		env[key] = value
+		key, val, _ := strings.Cut(l, "=")
+		env[key] = val
 	}
 	p.env = env
 
@@ -246,3 +245,10 @@ func int8SliceToString(s []int8) string {
 	}
 	return buf.String()
 }
+
+func min(a, b uint32) uint32 {
+	if a < b {
+		return a
+	}
+	return b
+}

providers/darwin/process_darwin_test.go

@@ -20,7 +20,10 @@
 package darwin
 
 import (
+	"bytes"
+	"encoding/binary"
 	"errors"
+	"fmt"
 	"os"
 	"os/exec"
 	"syscall"
@@ -139,3 +142,99 @@ func TestProcesses(t *testing.T) {
 
 	assert.NotZero(t, count, "failed to get process info for any processes")
 }
+
+func TestParseKernProcargs2(t *testing.T) {
+	testCases := []struct {
+		data    []byte
+		process process
+		err     error
+	}{
+		{data: nil, err: errInvalidProcargs2Data},
+		{data: []byte{}, err: errInvalidProcargs2Data},
+		{data: []byte{0xFF, 0xFF, 0xFF, 0xFF}, process: process{env: map[string]string{}}},
+		{data: []byte{0, 0, 0, 0}, process: process{env: map[string]string{}}},
+		{data: []byte{5, 0, 0, 0}, process: process{env: map[string]string{}}},
+		{
+			data: buildKernProcargs2Data(3, "./example", []string{"/Users/test/example", "--one", "--two"}, []string{"TZ=UTC", "FOO="}),
+			process: process{
+				exe:  "./example",
+				args: []string{"/Users/test/example", "--one", "--two"},
+				env: map[string]string{
+					"TZ":  "UTC",
+					"FOO": "",
+				},
+			},
+		},
+	}
+
+	for i, tc := range testCases {
+		tc := tc
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+			var p process
+			err := parseKernProcargs2(tc.data, &p)
+			if tc.err != nil {
+				assert.ErrorIs(t, err, tc.err)
+			} else {
+				assert.EqualValues(t, tc.process, p)
+			}
+		})
+	}
+}
+
+func FuzzParseKernProcargs2(f *testing.F) {
+	f.Add([]byte(nil))
+	f.Add([]byte{0, 0, 0, 0})
+	f.Add([]byte{10, 0, 0, 0})
+	f.Add([]byte{0xFF, 0xFF, 0xFF, 0xFF})
+	f.Add(buildKernProcargs2Data(-1, "./foo", []string{"/Users/john/foo", "-c"}, []string{"TZ=UTC"}))
+	f.Add(buildKernProcargs2Data(2, "./foo", []string{"/Users/john/foo", "-c"}, []string{"TZ=UTC"}))
+	f.Add(buildKernProcargs2Data(100, "./foo", []string{"/Users/john/foo", "-c"}, []string{"TZ=UTC"}))
+
+	f.Fuzz(func(t *testing.T, b []byte) {
+		p := &process{}
+		_ = parseKernProcargs2(b, p)
+	})
+}
+
+// buildKernProcargs2Data builds a response that is similar to what
+// sysctl kern.procargs2 returns.
+func buildKernProcargs2Data(argc int32, exe string, args, envs []string) []byte {
+	// argc
+	data := make([]byte, 4)
+	binary.LittleEndian.PutUint32(data, uint32(argc))
+	buf := bytes.NewBuffer(data)
+
+	// exe with optional extra null padding
+	buf.WriteString(exe)
+	buf.WriteByte(0)
+	buf.WriteByte(0)
+
+	// argv
+	for _, arg := range args {
+		buf.WriteString(arg)
+		buf.WriteByte(0)
+	}
+
+	// env
+	for _, env := range envs {
+		buf.WriteString(env)
+		buf.WriteByte(0)
+	}
+
+	// The returned buffer from the real kern.procargs2 contains more data than
+	// what go-sysinfo parses. This is a rough simulation of that extra data.
+	buf.Write(bytes.Repeat([]byte{0}, 100))
+	buf.WriteString("ptr_munge=")
+	buf.Write(bytes.Repeat([]byte{0}, 18))
+	buf.WriteString("main_stack==")
+	buf.Write(bytes.Repeat([]byte{0}, 43))
+	buf.WriteString("executable_file=0x1a01000010,0x36713a1")
+	buf.WriteString("dyld_file=0x1a01000010,0xfffffff0008839c")
+	buf.WriteString("executable_cdhash=5ca6024f9cdaa3a9fe515bfad77e1acf0f6b15b6")
+	buf.WriteString("executable_boothash=a4a5613c07091ef0a221ee75a924341406eab85e")
+	buf.WriteString("arm64e_abi=os")
+	buf.WriteString("th_port=")
+	buf.Write(bytes.Repeat([]byte{0}, 11))
+
+	return buf.Bytes()
+}