Skip to content

Commit 367878e

Browse files
rabbitstackrabbitstack
authored
fix: Process command line normalization (#125)
* fix process cmdline normalization * lint fix, simplify args splitting
1 parent c75d4e0 commit 367878e

6 files changed

Lines changed: 101 additions & 49 deletions

File tree

‎pkg/kevent/kparam.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,17 @@ func (kpars Kparams) Set(name string, value kparams.Value, typ kparams.Type) err
117117
return nil
118118
}
119119

120+
// SetValue replaces the value for the given parameter name. It will return an error
121+
// if the supplied parameter is not present in the parameter map.
122+
func (kpars Kparams) SetValue(name string, value kparams.Value) error {
123+
_, err := kpars.findParam(name)
124+
if err != nil {
125+
return fmt.Errorf("setting the value on a missing %q parameter is not allowed", name)
126+
}
127+
kpars[name].Value = value
128+
return nil
129+
}
130+
120131
// Get returns the raw value for given parameter name. It is the responsibility of the caller to probe type assertion
121132
// on the value before yielding its underlying type.
122133
func (kpars Kparams) Get(name string) (kparams.Value, error) {
@@ -265,6 +276,16 @@ func (kpars Kparams) GetUint32(name string) (uint32, error) {
265276
return v, nil
266277
}
267278

279+
// MustGetUint32 returns the underlying uint32 value parameter. It panics if
280+
// an error occurs while trying to get the parameter.
281+
func (kpars Kparams) MustGetUint32(name string) uint32 {
282+
v, err := kpars.GetUint32(name)
283+
if err != nil {
284+
panic(err)
285+
}
286+
return v
287+
}
288+
268289
// GetInt32 returns the underlying int32 value from the parameter.
269290
func (kpars Kparams) GetInt32(name string) (int32, error) {
270291
kpar, err := kpars.findParam(name)

‎pkg/kevent/kparam_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,9 @@ func TestKparams(t *testing.T) {
5959
mode := filemode.(fs.FileShareMode)
6060

6161
assert.Equal(t, "r-d", mode.String())
62+
63+
require.NoError(t, kpars.SetValue(kparams.FileName, "\\Device\\HarddiskVolume2\\Windows\\system32\\KERNEL32.dll"))
64+
filename1, err := kpars.GetString(kparams.FileName)
65+
require.NoError(t, err)
66+
assert.Equal(t, "\\Device\\HarddiskVolume2\\Windows\\system32\\KERNEL32.dll", filename1)
6267
}

‎pkg/kstream/interceptors/ps_windows.go‎

Lines changed: 41 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import (
3939
)
4040

4141
// systemRootRegexp is the regular expression for detecting path with unexpanded SystemRoot environment variable
42-
var systemRootRegexp = regexp.MustCompile(`%SystemRoot%|\\SystemRoot`)
42+
var systemRootRegexp = regexp.MustCompile(`%SystemRoot%|^\\SystemRoot|%systemroot%`)
4343

4444
// procYaraScans stores the total count of yara process scans
4545
var procYaraScans = expvar.NewInt("yara.proc.scans")
@@ -72,29 +72,37 @@ func newPsInterceptor(snap ps.Snapshotter, yara yara.Scanner) KstreamInterceptor
7272

7373
func (ps psInterceptor) Intercept(kevt *kevent.Kevent) (*kevent.Kevent, bool, error) {
7474
switch kevt.Type {
75-
case ktypes.CreateProcess, ktypes.TerminateProcess, ktypes.EnumProcess:
76-
comm, err := kevt.Kparams.GetString(kparams.Comm)
75+
case ktypes.CreateProcess,
76+
ktypes.TerminateProcess,
77+
ktypes.EnumProcess:
78+
cmdline, err := kevt.Kparams.GetString(kparams.Comm)
7779
if err != nil {
7880
return kevt, true, err
7981
}
82+
// if leading/trailing quotes are found, get rid of them
83+
if cmdline[0] == '"' && cmdline[len(cmdline)-1] == '"' {
84+
cmdline = cmdline[1 : len(cmdline)-1]
85+
}
86+
// expand all variations of the SystemRoot env variable
87+
if systemRootRegexp.MatchString(cmdline) {
88+
cmdline = systemRootRegexp.ReplaceAllString(cmdline, os.Getenv("SystemRoot"))
89+
}
8090
// some system processes are reported without the path in command line
81-
if !strings.Contains(comm, `\\:`) {
82-
_, ok := sysProcs[comm]
91+
if strings.Index(cmdline, `:\\`) != 1 {
92+
proc, _ := kevt.Kparams.GetString(kparams.ProcessName)
93+
_, ok := sysProcs[proc]
8394
if ok {
84-
_ = kevt.Kparams.Set(kparams.Comm, filepath.Join(os.Getenv("SystemRoot"), comm), kparams.UnicodeString)
95+
cmdline = filepath.Join(os.Getenv("SystemRoot"), "System32", cmdline)
8596
}
8697
}
87-
// to compose the full executable string we extract the path
88-
// from the process's command line by expanding the `SystemRoot`
89-
// env variable accordingly and also removing rubbish characters
90-
i := strings.Index(comm, ".exe")
98+
// append executable path parameter
99+
i := strings.Index(strings.ToLower(cmdline), ".exe")
91100
if i > 0 {
92-
exe := strings.Replace(comm[0:i+4], "\"", "", -1)
93-
if strings.Contains(exe, "SystemRoot") {
94-
exe = systemRootRegexp.ReplaceAllString(exe, os.Getenv("SystemRoot"))
95-
}
101+
exe := cmdline[0 : i+4]
96102
kevt.Kparams.Append(kparams.Exe, kparams.UnicodeString, exe)
97103
}
104+
_ = kevt.Kparams.SetValue(kparams.Comm, cmdline)
105+
98106
// convert hexadecimal PID values to integers
99107
pid, err := kevt.Kparams.GetHexAsUint32(kparams.ProcessID)
100108
if err != nil {
@@ -116,10 +124,9 @@ func (ps psInterceptor) Intercept(kevt *kevent.Kevent) (*kevent.Kevent, bool, er
116124
// get the process's start time and append it to the parameters
117125
started, err := getStartTime(pid)
118126
if err != nil {
119-
log.Warnf("couldn't get process (%d) start time: %v", pid, err)
120-
} else {
121-
_ = kevt.Kparams.Append(kparams.StartTime, kparams.Time, started)
127+
started = kevt.Timestamp
122128
}
129+
_ = kevt.Kparams.Append(kparams.StartTime, kparams.Time, started)
123130
}
124131
if ps.yara != nil && kevt.Type == ktypes.CreateProcess {
125132
// run yara scanner on the target process
@@ -133,10 +140,10 @@ func (ps psInterceptor) Intercept(kevt *kevent.Kevent) (*kevent.Kevent, bool, er
133140
}
134141
return kevt, false, ps.snap.Write(kevt)
135142
}
136-
137143
return kevt, false, ps.snap.Remove(kevt)
138-
139-
case ktypes.CreateThread, ktypes.TerminateThread, ktypes.EnumThread:
144+
case ktypes.CreateThread,
145+
ktypes.TerminateThread,
146+
ktypes.EnumThread:
140147
pid, err := kevt.Kparams.GetHexAsUint32(kparams.ProcessID)
141148
if err != nil {
142149
return kevt, true, err
@@ -151,43 +158,40 @@ func (ps psInterceptor) Intercept(kevt *kevent.Kevent) (*kevent.Kevent, bool, er
151158
if err := kevt.Kparams.Set(kparams.ThreadID, tid, kparams.TID); err != nil {
152159
return kevt, true, err
153160
}
154-
155161
if kevt.Type != ktypes.TerminateThread {
156162
return kevt, false, ps.snap.Write(kevt)
157163
}
158-
159164
return kevt, false, ps.snap.Remove(kevt)
160-
161-
case ktypes.OpenProcess, ktypes.OpenThread:
165+
case ktypes.OpenProcess,
166+
ktypes.OpenThread:
162167
pid, err := kevt.Kparams.GetUint32(kparams.ProcessID)
163168
if err != nil {
164169
return kevt, true, err
165170
}
171+
166172
proc := ps.snap.Find(pid)
167173
if proc != nil {
168-
kevt.Kparams.Append(kparams.Exe, kparams.UnicodeString, proc.Exe)
169-
kevt.Kparams.Append(kparams.ProcessName, kparams.UnicodeString, proc.Name)
174+
kevt.Kparams.Append(kparams.Exe, kparams.UnicodeString, proc.Exe).
175+
Append(kparams.ProcessName, kparams.UnicodeString, proc.Name)
170176
}
171177
_ = kevt.Kparams.Set(kparams.ProcessID, pid, kparams.PID)
178+
172179
// format the status code
173-
status, err := kevt.Kparams.GetUint32(kparams.NTStatus)
174-
if err == nil {
175-
_ = kevt.Kparams.Set(kparams.NTStatus, formatStatus(status, kevt), kparams.UnicodeString)
176-
}
180+
status := kevt.Kparams.MustGetUint32(kparams.NTStatus)
181+
_ = kevt.Kparams.Set(kparams.NTStatus, formatStatus(status, kevt), kparams.UnicodeString)
182+
177183
// convert desired access mask to hex value and transform
178184
// the access mask to a list of symbolical names
179-
desiredAccess, err := kevt.Kparams.GetUint32(kparams.DesiredAccess)
180-
if err == nil {
181-
_ = kevt.Kparams.Set(kparams.DesiredAccess, toHex(desiredAccess), kparams.AnsiString)
182-
}
185+
access := kevt.Kparams.MustGetUint32(kparams.DesiredAccess)
186+
_ = kevt.Kparams.Set(kparams.DesiredAccess, toHex(access), kparams.AnsiString)
187+
183188
if kevt.Type == ktypes.OpenProcess {
184-
kevt.Kparams.Append(kparams.DesiredAccessNames, kparams.Slice, process.DesiredAccess(desiredAccess).Flags())
189+
kevt.Kparams.Append(kparams.DesiredAccessNames, kparams.Slice, process.DesiredAccess(access).Flags())
185190
} else {
186-
kevt.Kparams.Append(kparams.DesiredAccessNames, kparams.Slice, thread.DesiredAccess(desiredAccess).Flags())
191+
kevt.Kparams.Append(kparams.DesiredAccessNames, kparams.Slice, thread.DesiredAccess(access).Flags())
187192
}
188193
return kevt, false, nil
189194
}
190-
191195
return kevt, true, nil
192196
}
193197

‎pkg/kstream/interceptors/ps_windows_test.go‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,9 @@ func TestPsInterceptorIntercept(t *testing.T) {
7575
assert.Equal(t, "C:\\Windows\\System32\\smss.exe", exe)
7676

7777
tpid := fmt.Sprintf("%x", os.Getpid())
78+
7879
kpars2 := kevent.Kparams{
79-
kparams.Comm: {Name: kparams.Comm, Type: kparams.UnicodeString, Value: "C:\\Windows\\System32\\smss.exe"},
80+
kparams.Comm: {Name: kparams.Comm, Type: kparams.UnicodeString, Value: "\"C:\\Windows\\System32\\smss.exe\""},
8081
kparams.ProcessID: {Name: kparams.ProcessID, Type: kparams.HexInt32, Value: kparams.Hex(tpid)},
8182
kparams.ProcessParentID: {Name: kparams.ProcessParentID, Type: kparams.HexInt32, Value: kparams.Hex("26c")},
8283
}
@@ -89,4 +90,22 @@ func TestPsInterceptorIntercept(t *testing.T) {
8990
require.NoError(t, err)
9091

9192
require.True(t, kevt2.Kparams.Contains(kparams.StartTime))
93+
94+
cmdline, _ := kevt2.Kparams.GetString(kparams.Comm)
95+
require.Equal(t, "C:\\Windows\\System32\\smss.exe", cmdline)
96+
97+
kpars3 := kevent.Kparams{
98+
kparams.Comm: {Name: kparams.Comm, Type: kparams.UnicodeString, Value: "csrss.exe"},
99+
kparams.ProcessName: {Name: kparams.ProcessName, Type: kparams.UnicodeString, Value: "csrss.exe"},
100+
}
101+
102+
kevt3 := &kevent.Kevent{
103+
Type: ktypes.CreateProcess,
104+
Kparams: kpars3,
105+
}
106+
107+
_, _, err = psi.Intercept(kevt3)
108+
require.NoError(t, err)
109+
cmdline1, _ := kevt3.Kparams.GetString(kparams.Comm)
110+
require.Equal(t, "C:\\Windows\\System32\\csrss.exe", cmdline1)
92111
}

‎pkg/ps/types/types_windows.go‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -218,17 +218,7 @@ func NewFromKcap(buf []byte) (*PS, error) {
218218
return &ps, nil
219219
}
220220

221-
func splitArgs(comm string) []string {
222-
ext := strings.Index(comm, ".exe")
223-
if ext == -1 {
224-
return []string{}
225-
}
226-
ext += 5
227-
if ext > len(comm) {
228-
return []string{}
229-
}
230-
return strings.Fields(comm[ext:])
231-
}
221+
func splitArgs(cmdline string) []string { return strings.Fields(cmdline) }
232222

233223
// AddThread adds a thread to process's state descriptor.
234224
func (ps *PS) AddThread(thread Thread) {

‎pkg/ps/types/types_windows_test.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package types
2020

2121
import (
2222
"github.com/magiconair/properties/assert"
23+
"github.com/stretchr/testify/require"
2324
"testing"
2425
)
2526

@@ -59,3 +60,15 @@ func TestVisit(t *testing.T) {
5960

6061
assert.Equal(t, expected1, parents1)
6162
}
63+
64+
func TestPSArgs(t *testing.T) {
65+
ps := NewPS(
66+
233,
67+
4532,
68+
"spotify.exe",
69+
"",
70+
"C:\\Users\\admin\\AppData\\Roaming\\Spotify\\Spotify.exe --type=crashpad-handler /prefetch:7 --max-uploads=5 --max-db-size=20 --max-db-age=5 --monitor-self-annotation=ptype=crashpad-handler \"--metrics-dir=C:\\Users\\admin\\AppData\\Local\\Spotify\\User Data\" --url=https://crashdump.spotify.com:443/ --annotation=platform=win32 --annotation=product=spotify",
71+
Thread{}, nil)
72+
require.Len(t, ps.Args, 12)
73+
require.Equal(t, "/prefetch:7", ps.Args[2])
74+
}

0 commit comments

Comments
 (0)