Skip to content

PIPE-10460 FuzzerUtilLinux#6

Draft
SB-PawanN wants to merge 5 commits into
masterfrom
PIPE-10460-FuzzerUtilLinux
Draft

PIPE-10460 FuzzerUtilLinux#6
SB-PawanN wants to merge 5 commits into
masterfrom
PIPE-10460-FuzzerUtilLinux

Conversation

@SB-PawanN

@SB-PawanN SB-PawanN commented Jul 2, 2026

Copy link
Copy Markdown

[SECURITY] Harden Linux ExecuteCommand Path for CWE-78/CWE-88 (PIPE-10460)
This PR fixes command and argument injection risk in the Linux ExecuteCommand path by replacing shell-based system execution with fork + execvp and strict allowlist validation in FuzzerUtilLinux.cpp.

Problem
The previous Linux implementation executed a raw command string through a shell, enabling:

  • CWE-78: OS command injection through shell metacharacters
  • CWE-88: argument injection through unsafe input handling

Solution

  • Removed shell invocation from Linux ExecuteCommand
  • Added allowlist-based parsing/validation for command arguments
  • Executed commands via explicit argv with fork + execvp
  • Rejected unsafe metacharacter/escape patterns used in injection payloads

Testing

  • Security tests validate blocking of common injection payloads
  • Valid command flows were verified
  • Build/test checks were run for regression confidence

Important Scope Note
This PR is intentionally scoped to the Linux ExecuteCommand path only.
It does not remediate remaining shell-based execution paths on POSIX that still use popen:

  • FuzzerUtil.cpp:208
  • FuzzerUtilPosix.cpp:112
  • FuzzerUtilPosix.cpp:113
    Those paths require a follow-up fix to move to a no-shell model (pipe + fork + execve with explicit argv).

JIRA: PIPE-10460
Deadline: July 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens libFuzzer’s Linux ExecuteCommand() implementation by replacing system() (shell invocation) with a fork()/execvp()-based execution path plus character allowlist validation, aiming to mitigate command/argument injection risks.

Changes:

  • Replaced Linux ExecuteCommand() from system(Command.c_str()) to fork() + execvp() execution.
  • Added allowlist-based parsing of the command string into an argv array, rejecting disallowed characters and malformed escapes.
  • Added waitpid() handling to collect the child status.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/Fuzzer/FuzzerUtilLinux.cpp Outdated
Comment thread lib/Fuzzer/FuzzerUtilLinux.cpp Outdated
Comment on lines +29 to +31
// Parse command as an argv list with allowlist validation.
// Backslash escapes allow literal special characters (e.g. space) inside
// arguments while rejecting malformed escape sequences.
Comment thread lib/Fuzzer/FuzzerUtilLinux.cpp
Copilot AI added 3 commits July 2, 2026 12:45
Extended ExecuteCommand to support output redirection (>) and stderr
merging (2>&1) without invoking a shell, addressing review comment
r3512544355. The implementation:

- Parses ">" and "2>&1" syntax in the command string
- Validates output filenames using the same allowlist rules
- Implements redirection via open()/dup2() in the child process
- Maintains security by avoiding shell invocation

This allows FuzzerDriver.cpp's usage (line 210) to work correctly while
preserving the security hardening against command injection.
Addressed code review feedback:
- Fixed return value to properly extract exit code using WEXITSTATUS()
- Made isspace() usage consistent with unsigned char casts
- Changed file permissions from 0644 to 0600 for better security
- Added comment explaining restrictive permissions

These improvements enhance robustness and security while maintaining
the shell-free output redirection functionality.
Removed unused 'Start' variable that was calculated but never used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants