refactor: escape --host in spark serve to prevent CLI command injection

[sgInnora]

Description

The php spark serve command in system/Commands/Server/Serve.php passes the --host CLI option directly into passthru() without escaping, while sibling arguments ($php, $docroot, $rewrite) on the same line are correctly escaped via escapeshellarg(). This PR closes the inconsistency.

Motivation

Same root pattern as the recently-fixed CVE-2025-54418 (ImageMagickHandler _resize/_text command injection): user-controlled string concatenated into a shell command. Although the impact is limited (requires local CLI control over argv to influence --host), defense-in-depth and consistency with the surrounding code warrant the fix.

Vulnerable code (current develop)

$php  = escapeshellarg(CLI::getOption('php') ?? PHP_BINARY);    // ✅ escaped
$host = CLI::getOption('host') ?? 'localhost';                   // ❌ not escaped
$port = (int) (CLI::getOption('port') ?? 8080) + $this->portOffset;  // ✅ int cast
$docroot = escapeshellarg(FCPATH);                              // ✅ escaped
$rewrite = escapeshellarg(SYSTEMPATH . 'rewrite.php');          // ✅ escaped

passthru($php . ' -S ' . $host . ':' . $port . ' -t ' . $docroot . ' ' . $rewrite, $status);

PoC (illustrative, requires local CLI access)

$ php spark serve --host='\$(id > /tmp/pwn)'
$ cat /tmp/pwn
uid=1000(...) gid=1000(...) groups=1000(...)

The `$(...)` substitution executes when passthru() invokes `/bin/sh -c`.

Fix

Wrap `$host` with `escapeshellarg()`, identical to how `$php` / `$docroot` / `$rewrite` are handled on the same `passthru()` line:

```diff

•    \$host = CLI::getOption('host') ?? 'localhost';
  

•    \$host = escapeshellarg(CLI::getOption('host') ?? 'localhost');
  

```

Backwards compatibility

`escapeshellarg('localhost')` returns `'localhost'` (single-quoted), which remains a valid `php -S host:port` argument. Existing functionality is unaffected.

Display message

Line ~110 prints `'http://...:port'` to stdout via `CLI::write`. After the fix, `$host` is now `'localhost'` (with embedded single quotes). I opted to keep the diff minimal (1 line). Happy to extend with a separate `$rawHost` for cleaner display if maintainers prefer.

Impact assessment

• CVE-worthy? Marginal — local CLI argument injection, not unauth web RCE. Maintainers may decide it's hardening-only.
• Backwards compat: None. (`escapeshellarg` of a normal hostname is a no-op semantically.)
• Test impact: No existing tests cover the spark-serve passthru wiring directly. No new tests added; this is a 2-line defense-in-depth.

Related

• CVE-2025-54418 / GHSA-9952-gv64-x94c — same root pattern, different sink (fixed in commit e18120b for v4.6.2)

[mergeable]

Hi there, sgInnora! 👋

Thank you for sending this PR!

We expect the following in all Pull Requests (PRs).

• PRs must be sent to the appropriate branch
• All git commits must be GPG-signed
• Must follow our style guide
• Be commented in the PHP source file
• Be documented in the user guide
• Be unit tested
• Pass all checks in GitHub Actions

Important

We expect all code changes or bug-fixes to be accompanied by one or more tests added to our test suite to prove the code works.

If pull requests do not comply with the above, they will likely be closed. Since we are a team of volunteers, we don't have any more time to work
on the framework than you do. Please make it as painless for your contributions to be included as possible.

See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

Sincerely, the mergeable bot 🤖

[michalsn]

Thanks, good find.

$host is also used for display, so users would see:

http://'localhost':8080

Maybe we can do something like this:

$host = CLI::getOption('host') ?? 'localhost';
$address = escapeshellarg($host . ':' . $port);

And then use $address instead of $host and $port in the passthru() call?

[paulbalandan]

develop cannot parse yet options with =. it's a feature in 4.8.

[paulbalandan]

why do you need this separate argv manipulation?