Skip to content

Hardening: Avoid shell-based popen usage in InspectFile operator#3489

Draft
praneet390 wants to merge 1 commit intoowasp-modsecurity:v3/masterfrom
praneet390:hardening-inspectfile-exec
Draft

Hardening: Avoid shell-based popen usage in InspectFile operator#3489
praneet390 wants to merge 1 commit intoowasp-modsecurity:v3/masterfrom
praneet390:hardening-inspectfile-exec

Conversation

@praneet390
Copy link

This PR proposes a security hardening improvement to the InspectFile operator external command execution path.

Currently, InspectFile::evaluate constructs a command string and invokes it using popen(), which executes via the system shell.

While current usage paths (e.g., FILES_TMPNAMES) pass engine-generated sanitized filenames and are not attacker-controlled, invoking a shell with concatenated arguments is a fragile pattern and increases risk if future variable sources expand.

This PR starts discussion toward replacing shell-based execution with argument-safe process execution (execve/spawn with explicit argv array), removing shell interpretation from the execution path.

Security benefits:

  • removes shell metacharacter interpretation risk
  • prevents command injection under misuse
  • enforces argument boundaries
  • improves defense-in-depth

This PR currently adds a security hardening note and opens discussion on the preferred cross-platform implementation approach for v2 and v3.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 6, 2026

@praneet390
Copy link
Author

Context: This PR comes from a security review of the @inspectFile operator execution path.

While current data flow does not allow command injection due to engine-controlled inputs (e.g., FILES_TMPNAMES via mkstemp), the popen + concatenated string pattern is shell-based and fragile by design.

Opening this draft PR to discuss a possible migration toward argv-based exec/spawn for defense-in-depth and safer future semantics across v2 and v3.

Happy to implement the change following maintainer guidance on preferred cross-platform approach.

@airween airween added the 3.x Related to ModSecurity version 3.x label Feb 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.x Related to ModSecurity version 3.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants