tracee icon indicating copy to clipboard operation
tracee copied to clipboard

ParseArgs unnecessary casting leads to wrong values

Open rscampos opened this issue 10 months ago • 4 comments

Description

I'll open an issue since I don't if the current approach is correct or not. Once we discuss it, I can submit a PR.

I was reviewing https://github.com/aquasecurity/tracee/pull/4442 when I realized that some casting might lead to incorrect values.

The syscall execveat has the field flags as type int in the definition: https://github.com/aquasecurity/tracee/blob/8d22a0791f98d4a1df4e30c95bdb9d9eb3c6801f/pkg/events/core.go#L8073

But here happens a cast to uint64 before passing parseExecFlag: https://github.com/aquasecurity/tracee/blob/8d22a0791f98d4a1df4e30c95bdb9d9eb3c6801f/pkg/events/parse_args.go#L124-L129

The issue happens when we call execveat using flag=-1. The value will be converted to 0xFFFFFFFFFFFFFFFF, setting all flags:

sudo ./dist/tracee -e execveat -s comm=python3.10
TIME             UID    COMM             PID     TID     RET              EVENT                     ARGS
11:29:25:554926  1000   python3.10       638196  638196  0                execveat                  dirfd: -100, pathname: /bin/ls, argv: [/bin/ls], envp: 0x0, flags: AT_EMPTY_PATH|AT_SYMLINK_NOFOLLOW|AT_EACCESS|AT_REMOVEDIR|AT_NO_AUTOMOUNT|AT_STATX_SYNC_TYPE|AT_STATX_FORCE_SYNC|AT_STATX_DONT_SYNC|AT_RECURSIVE
11:29:25:554926  1000   python3.10       638196  638196  -22              execveat                  dirfd: -100, pathname: /bin/ls, argv: [/bin/ls], envp: 0x0, flags: AT_EMPTY_PATH|AT_SYMLINK_NOFOLLOW|AT_EACCESS|AT_REMOVEDIR|AT_NO_AUTOMOUNT|AT_STATX_SYNC_TYPE|AT_STATX_FORCE_SYNC|AT_STATX_DONT_SYNC|AT_RECURSIVE

Usually, this behavior occurs when the type of the event in the definition is signed and we cast the negative value to unsigned. I think each function should have the signature aligned with the type of the field from the event defined in the definition.

Output of tracee version:

(paste your output here)

Output of uname -a:

(paste your output here)

Additional details

Python code to call execveat with flag=-1.

import ctypes
import errno
import os

SYS_EXECVEAT = 281  # ARM

AT_FDCWD = -100
exec_path = b"/bin/ls"
argv = (ctypes.c_char_p * 2)()
argv[0] = exec_path
argv[1] = None  # Null-terminated argument list

# Environment variables (null-terminated array)
envp = (ctypes.c_char_p * 1)()
envp[0] = None

# trigger
flags = -1

libc = ctypes.CDLL("libc.so.6", use_errno=True)

ret = libc.syscall(SYS_EXECVEAT, AT_FDCWD, exec_path, ctypes.byref(argv), ctypes.byref(envp), flags)

if ret == -1:
    err = ctypes.get_errno()
    print(f"Error: {os.strerror(err)} (errno={err})")
    if err == errno.ENOENT:
        print("File not found.")
    elif err == errno.EACCES:
        print("Permission denied.")
    else:
        print("Unexpected error occurred.")
else:
    print("Execveat syscall succeeded.")

rscampos avatar Jan 14 '25 17:01 rscampos

I totally agree.

But before tackling it let's get this one in?

https://github.com/aquasecurity/tracee/pull/4279

It has fixes and improvements which need to be followed.

geyslan avatar Jan 14 '25 18:01 geyslan

looks good to me, should I add it to my pr? @rscampos #4442

ShohamBit avatar Jan 15 '25 09:01 ShohamBit

@ShohamBit I think we should have another PR for that since we need to do some refactoring and testing, WDYT?

rscampos avatar Jan 15 '25 11:01 rscampos

sounds good

ShohamBit avatar Jan 15 '25 11:01 ShohamBit