Change in comparison logic behaviour in 0.38.0

[harveyrendell]

Describe the bug

We recently upgraded from 0.36.2 to 0.38.0 and noticed some of our custom rules were not evaluating the same as previously. I narrowed it down to one specific macro which seems to have changed behaviour between versions.

The comparison is:

- macro: system_users
  condition: (user.name in (bin, daemon, games, lp, mail, nobody, sshd, sync, uucp, www-data))

- macro: non_privileged_users
  condition: ((not system_users) and (user.loginuid >= 1000))

The comparison user.loginuid >= 1000 does not seem to evaluate to False when its value is -1. This was the behaviour in 0.36.2 but not in 0.38.0

This can be worked around by adding another explicit condition: (user.loginuid != -1 and user.loginuid >= 1000)

How to reproduce it

- macro: recreate_issue
  condition: (user.loginuid >= 1000)

- rule: Tracking Files Deletion or Renaming
  condition: >
    ( evt.type in (unlink, unlinkat, rename, renameat)
      and evt.dir = <
      and recreate_issue )
  output: A user has successfully deleted or renamed a file. file=%fd.name name=%user.name uid=%user.uid loginuser=%user.loginname loginuid=%user.loginuid command=%proc.cmdline
  priority: INFO

Expected behaviour

We expect that this rule will trigger on file renames for logged in users which are not root.

More specifically, we expect using the greater than or equal to operator in the following condition: user.loginuid >= 1000 will evaluate to False when user.loginuid is -1

Screenshots

Example output from falco on a server running docker:

{"hostname":"********","output":"21:47:51.530700201: Informational A user has successfully deleted or renamed a file. file=<NA> loginuser=<NA> loginuid=-1 user=root command=dockerd -H fd:// --containerd=/run/containerd/containerd.sock --default-ulimit nofile=32768:65536","priority":"Informational","rule":"Tracking Files Deletion or Renaming","source":"syscall","tags":["cis"],"time":"2024-06-11T21:47:51.530700201Z", "output_fields": {"evt.time":1718142471530700201,"fd.name":null,"proc.cmdline":"dockerd -H fd:// --containerd=/run/containerd/containerd.sock --default-ulimit nofile=32768:65536","user.loginname":"<NA>","user.loginuid":-1,"user.name":"root"}}

Environment

• Falco version: 0.38.0 (x86_64)

• System info: Linux version 4.14.344-262.563.amzn2.x86_64 (mockbuild@ip-10-0-35-189) (gcc version 7.3.1 20180712 (Red Hat 7.3.1-17) (GCC)) #1 SMP Fri May 1718:07:48 UTC 2024

• Cloud provider or hardware configuration: AWS
• OS: Amazon Linux 2

• Kernel: Linux 4.14.344-262.563.amzn2.x86_64

• Installation method: RPM

Additional context

[incertum]

Thank you. We fixed the data type for the loginuid in this PR https://github.com/falcosecurity/libs/pull/1192/files#diff-70b8039f1cd1e5664df22d0b231919b7b042dfdf223134fb8356a4b7f4d0da02, @jasondellaluce would you have ideas how this could have affected the filtering logic?

[LucaGuerra]

I believe this might be more impacting than we thought. The data is right, but a signed comparison is being evaluated as unsigned. I suspect this line which sets a template but does not use template types:

https://github.com/falcosecurity/libs/blob/5c5edf2f58b6df59b206181af45116bd8962b9a4/userspace/libsinsp/filter_compare.cpp#L418

Let me check. This is probably good for 0.38.1

Edit: yep, that's a bug. We will release 0.38.1 soon to fix this and other things that have been reported.

/milestone 0.38.1

[harveyrendell]

Thanks for the quick turnaround on this! Much appreciated.

Happy for this to be closed now as completed.

[LucaGuerra]

Falco 0.38.1 is now available 🎉