tetragon: Check the returned data length for exec arguments

[olsajiri]

It can actually happen that there will be no arguments
or filename, in which case we need to check the data
length and api.EventErrorFilename flag.

Signed-off-by: Jiri Olsa [email protected]

[willfindlay]

@olsajiri can we do similar bounds checking here? This could also panic if Index() returns -1 (which it can).

tetragon/pkg/sensors/exec/exec.go

Lines 113 to 116 in d6d4ade

	} else {
	n := bytes.Index(args, []byte{0x00})
	proc.Filename = string(args[:n])
	args = args[n+1:]

[jrfastab]

And unit test would be great.

[olsajiri]

And unit test would be great.

this seems to happen when probe_read_str fails to read the filename,
I'll check but not sure there's easy way to simulate that

[olsajiri]

And unit test would be great.

this seems to happen when probe_read_str fails to read the filename, I'll check but not sure there's easy way to simulate that

fwiw I managed to reproduce with:

--- a/bpf/process/bpf_execve_event.c
+++ b/bpf/process/bpf_execve_event.c
@@ -116,7 +116,10 @@ event_filename_builder(void *ctx, struct msg_process *curr, __u32 curr_pid,
        earg = (void *)curr + offsetof(struct msg_process, args);
 
        size = probe_read_str(earg, MAXARGLENGTH - 1, filename);
-       if (size < 0) {
+       if (1) {
+               flags |= EVENT_ERROR_FILENAME;
+               size = 0;
+       } else if (size < 0) {
                flags |= EVENT_ERROR_FILENAME;
                size = 0;
        } else if (size == MAXARGLENGTH - 1) {

and the change fixes the crash.. maybe we could have a way to instrument bpf program to trigger the error for some binary name and test that.. but that'd mean more changes

[kkourt]

maybe we could have a way to instrument bpf program to trigger the error for some binary name and test that.. but that'd mean more changes

👍

A simpler solution might be to add some unit tests on the go side for this.