Skip to content

fix(userspace): open pidfile with O_NOFOLLOW to prevent symlink TOCTOU#3871

Open
alexmchughdev wants to merge 1 commit intofalcosecurity:masterfrom
alexmchughdev:pidfile-nofollow
Open

fix(userspace): open pidfile with O_NOFOLLOW to prevent symlink TOCTOU#3871
alexmchughdev wants to merge 1 commit intofalcosecurity:masterfrom
alexmchughdev:pidfile-nofollow

Conversation

@alexmchughdev
Copy link
Copy Markdown

What type of PR is this?

/kind bug

What this PR does / why we need it:

Hardens the pidfile open path in userspace/falco/app/actions/pidfile.cpp against a symlink TOCTOU. Previously the pidfile was created via std::ofstream::open(), which has no portable O_NOFOLLOW equivalent. If an operator configures a pidfile path in a directory writable by an unprivileged user, that user could pre-place a symlink and trick a root-running Falco into clobbering an arbitrary file with the PID on startup.

The pidfile path is operator-controlled and typically lives in a root-owned directory like /var/run, so this is not exploitable in the standard threat model — this is a defence-in-depth fix.

The change replaces the std::ofstream open with a raw POSIX open() using O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW | O_CLOEXEC and writes the PID with dprintf. The open() will now fail with ELOOP if the path is a symlink. Error reporting goes through falco_strerror_r matching the pattern in create_signal_handlers.cpp. Behaviour is unchanged for legitimate users.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

  • No behaviour change for legitimate operators — same path, same content (PID + newline), same exit(-1) on failure as before.
  • The O_CLOEXEC flag is also added so the fd does not leak into any later forked program-output children.
  • A short comment explains why O_NOFOLLOW is there so it is not removed during a future cleanup.

Does this PR introduce a user-facing change?:

fix(userspace): open the pidfile with O_NOFOLLOW to prevent a symlink TOCTOU when the pidfile path is in an attacker-writable directory

@poiana
Copy link
Copy Markdown
Contributor

poiana commented May 7, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: alexmchughdev
Once this PR has been reviewed and has the lgtm label, please assign lucaguerra for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana requested review from Kaizhe and irozzo-1A May 7, 2026 16:04
@poiana
Copy link
Copy Markdown
Contributor

poiana commented May 7, 2026

Welcome @alexmchughdev! It looks like this is your first PR to falcosecurity/falco 🎉

@poiana poiana added the size/M label May 7, 2026
@ekoops
Copy link
Copy Markdown
Contributor

ekoops commented May 8, 2026

Hey @alexmchughdev , unfortunately this PR breaks windows build. Could you please take a look? https://github.com/falcosecurity/falco/actions/runs/25507407572/job/75014856179?pr=3871

@alexmchughdev alexmchughdev force-pushed the pidfile-nofollow branch 2 times, most recently from ebd2881 to 2045edd Compare May 8, 2026 13:33
The pidfile path is operator-controlled and typically lives in a
root-owned directory like /var/run, so this is not exploitable in
the standard threat model. However, if an operator configures a
pidfile path in a directory writable by an unprivileged user, that
user could pre-place a symlink to cause Falco (running as root) to
overwrite an arbitrary file with the PID on startup.

Replace the std::ofstream open with a raw POSIX open() using
O_NOFOLLOW | O_CLOEXEC on non-Windows platforms. The open will now
fail with ELOOP if the path is a symlink, closing the
defence-in-depth gap with no behaviour change for legitimate users.

Windows builds retain the original std::ofstream path since
O_NOFOLLOW / O_CLOEXEC are not available on the Windows toolchain
and the Linux-as-root threat model that motivates the hardening
does not apply to Falco's Windows build.

Signed-off-by: Alex McHugh <alexanderedwardmchugh@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

3 participants