aquasecurity / tracee

Linux Runtime Security and Forensics using eBPF
https://aquasecurity.github.io/tracee/latest
Apache License 2.0
3.64k stars 420 forks source link

Possible bug in Test_EventFilters when a case has many `cmdEvent` and each` cmdEvent` has different number of `expectEvent`. #4255

Open canoriz opened 3 months ago

canoriz commented 3 months ago

Description

This is not an existing bug but a possible bug. I am developing a feature (a new kind of event), when adding a test for my feature, I encountered a bug in integration-tests. My test case looks like this, after running ping, tracee should emit events.SchedProcessExec and events.MyDevelopingEvent.

        {
            name: "xxx",
            policyFiles: []testutils.PolicyFileWithID{
                {
                    Id: 1,
                    PolicyFile: v1beta1.PolicyFile{
                        Metadata: v1beta1.Metadata{
                            Name: "xxx",
                        },
                        Spec: k8s.PolicySpec{
                            Scope:          []string{"global"},
                            DefaultActions: []string{"log"},
                            Rules: []k8s.Rule{
                                {
                                    Event:   "sched_process_exec",
                                    Filters: []string{"comm=ping"},
                                    ...
                                },
                                {
                                    Event:   "sched_process_exit",
                                    Filters: []string{"comm=who"},
                                },
                            },
                        },
                    },
                },
            },
            cmdEvents: []cmdEvents{
                newCmdEvents(
                    "who",
                    0,
                    1*time.Second,
                    []trace.Event{
                        expectEvent(anyHost, "who", testutils.CPUForTests, anyPID, 0, events.SchedProcessExit, orPolNames("xxx"), orPolIDs(1)),
                    },
                    []string{},
                    false,
                ),
                newCmdEvents(
                    "ping -c1 0.0.0.0",
                    0,
                    1*time.Second,
                    []trace.Event{
                        expectEvent(anyHost, "ping", testutils.CPUForTests, anyPID, 0, events.SchedProcessExec, orPolNames("xxx"), orPolIDs(1)),
                        expectEvent(anyHost, "ping", testutils.CPUForTests, anyPID, 0, events.MyDevelopingEvent, orPolNames("xxx"), orPolIDs(1)),
                    },
                    []string{},
                    true,
                ),
            },
            useSyscaller: false,
            coolDown:     0,
            test:         ExpectAllInOrderSequentially,
        },

Line 2482 calculates index of actEvtsCopy, the expression cmdIdx*len(cmd.expectedEvents)+evtIdx will produce wrong index when cmd in cmdEvents have different number of expectedEvents. Take my test case as an example, when comparing output of runCmd("ping"), the correct index should be 1 and 2, but the expression gives 2 and 3 because len(cmd.expectedEvents) of "ping" is 2 and len(cmd.expectedEvent) of "who" is 1. https://github.com/aquasecurity/tracee/blob/a325d6439f319f4988428ae4741ee0eef9cd9b32/tests/integration/event_filters_test.go#L2473-L2482

Output of tracee version:

I think main branch also have this possible bug. I give version v0.21.0 here.

v0.21.0

Output of uname -a:

Linux 6.9.9-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.9.9-1 (2024-07-13) x86_64 GNU/Linux

Additional details

I suggest changing code to below.

    actEvtIdx := 0
    for _, cmd := range cmdEvents {
        syscallsInSets := []string{}
        checkSets := len(cmd.sets) > 0
        if checkSets {
            syscallsInSets = getAllSyscallsInSets(cmd.sets)
        }

        // compare the expected events with the actual events in the same order
        for _, expEvt := range cmd.expectedEvents {
            // runCmds ensures at least same number of events were received in actual,
            // hence no out of range panic here
            actEvt := actEvtsCopy[actEvtIdx]
            actEvtIdx++
geyslan commented 3 months ago

@canoriz thanks, I'm going to check your suggestion.