fix: prevent NULL pointer crash in program_output on popen failure

[vietcgi]

Fix: Prevent NULL pointer crash in program_output on popen failure

Problem

The output_program class has a critical bug that causes Falco to crash with SIGSEGV (exit code 139) when popen() fails. This results in complete Falco shutdown and loss of all security monitoring.

Root Causes

1. Uninitialized member variable: m_pfile is not initialized to nullptr, causing undefined behavior on first access
2. Missing NULL check after popen(): If popen() fails and returns NULL, both setvbuf() and fprintf() are called with a NULL pointer
3. No error handling: No logging or recovery when program output fails

Trigger Scenarios

• Invalid program path in program_output configuration
• Missing execute permissions on the program
• Resource exhaustion (ulimit reached, too many processes/files)
• Any system-level popen() failure

Impact

• Severity: Critical - complete Falco crash
• Scope: All users of program_output feature
• Result: Total loss of security monitoring when program fails

Solution

Changes Made

File: userspace/falco/outputs_program.h

• Initialize m_pfile = nullptr to ensure defined behavior

File: userspace/falco/outputs_program.cpp

• Add includes: logger.h, <cerrno>, <cstring>
• Add NULL check after popen() with error logging using falco_logger
• Add defensive NULL check before fprintf()
• Include errno details in error message for debugging

Code Review

Before:

FILE *m_pfile;  // Uninitialized!

void open_pfile() {
    if(m_pfile == nullptr) {  // UB if not initialized
        m_pfile = popen(...);
        if(!m_buffered) {
            setvbuf(m_pfile, ...);  //  Crash if popen failed!
        }
    }
}

void output(const message *msg) {
    open_pfile();
    fprintf(m_pfile, ...);  //  Crash if m_pfile is NULL!
}

After:

FILE *m_pfile = nullptr;  //  Initialized

void open_pfile() {
    if(m_pfile == nullptr) {
        m_pfile = popen(...);
        
        if(m_pfile == nullptr) {  //  Check for failure
            falco_logger::log(ERR, "Failed to open program output: " + 
                             program + " (error: " + strerror(errno) + ")");
            return;
        }
        
        if(!m_buffered) {
            setvbuf(m_pfile, ...);  //  Safe - popen succeeded
        }
    }
}

void output(const message *msg) {
    open_pfile();
    
    if(m_pfile != nullptr) {  //  Defensive check
        fprintf(m_pfile, ...);
    }
}

Design Decisions

1. Error handling pattern: Follows the same pattern as output_http (see userspace/falco/outputs_http.cpp:48-51)
2. Logging level: Uses ERR level consistent with other output failures
3. Fail-safe: Falco continues running even when program output fails
4. Error details: Includes both program path and errno for debugging

Testing

Manual Testing Plan

1. Test invalid program path:

   program_output:
     enabled: true
     program: "/nonexistent/path"

   • Expected: Error logged, Falco continues, no crash

2. Test permission denied:

   program_output:
     enabled: true
     program: "/usr/bin/sudo"  # Without permissions

   • Expected: Error logged with errno details, no crash

3. Test valid program:

   program_output:
     enabled: true
     program: "jq -c ."

   • Expected: Works normally, alerts piped to jq

4. Test keep_alive modes:

   • keep_alive: true - Program started once, reused
   • keep_alive: false - Program restarted per alert
   • Expected: Both modes handle popen failure gracefully

Automated Testing

Regression tests needed:

• Unit test for popen() failure handling
• Integration test with invalid program path
• Verify other outputs still work when program output fails

Build Verification

• Code compiles without warnings (verified locally)
• No changes to public API
• No ABI changes
• Backward compatible

Checklist

• Follows existing error handling patterns in codebase
• Uses standard logging mechanisms (falco_logger)
• Maintains backward compatibility
• Includes proper error messages with context
• Defensive programming - checks before unsafe operations
• No memory leaks (pclose called in cleanup())
• Fails safely - Falco continues even when output fails

Related Issues

• Potentially related to falco 0.40.0 is crashed 139 sometimes #3632 (crash with exit code 139)
• Similar pattern to fixes in output_file and output_http

Additional Notes

This is a defensive fix that prevents a crash scenario that can occur in production. While the program_output feature may not be widely used, when it is configured with an invalid path, the current code causes complete Falco failure rather than graceful degradation.

The fix ensures Falco's core monitoring continues even when ancillary output mechanisms fail.

[poiana]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[poiana]

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

[poiana]

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Needs approval from an approver in each of these files:

• OWNERS

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

[leogr]

/milestone 0.43.0