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

system/Commands/Server/Serve.php

@@ -95,6 +95,11 @@ public function run(array $params)
  $host = CLI::getOption('host') ?? 'localhost';
  $port = (int) (CLI::getOption('port') ?? 8080) + $this->portOffset;
 
+ // Build a single shell-escaped host:port argument so the resulting
+ // command is safe regardless of what the user passed via --host,
+ // while $host remains in its raw form for the display below.
+ $address = escapeshellarg($host . ':' . $port);
+
  // Get the party started.
  CLI::write('CodeIgniter development server started on http://' . $host . ':' . $port, 'green');
  CLI::write('Press Control-C to stop.');
@@ -108,7 +113,7 @@ public function run(array $params)
  // Call PHP's built-in webserver, making sure to set our
  // base path to the public folder, and to use the rewrite file
  // to ensure our environment is set and it simulates basic mod_rewrite.
- passthru($php . ' -S ' . $host . ':' . $port . ' -t ' . $docroot . ' ' . $rewrite, $status);
+ passthru($php . ' -S ' . $address . ' -t ' . $docroot . ' ' . $rewrite, $status);
 
  if ($status !== EXIT_SUCCESS && $this->portOffset < $this->tries) {
  $this->portOffset++;

tests/system/Commands/Server/ServeTest.php

@@ -0,0 +1,98 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * This file is part of CodeIgniter 4 framework.
+ *
+ * (c) CodeIgniter Foundation <admin@codeigniter.com>
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace CodeIgniter\Commands\Server;
+
+use CodeIgniter\Test\CIUnitTestCase;
+use CodeIgniter\Test\StreamFilterTrait;
+use PHPUnit\Framework\Attributes\Group;
+
+/**
+ * Override the global passthru() within this namespace so the unit test
+ * can capture the shell command that Serve::run() would otherwise execute.
+ *
+ * @internal
+ */
+function passthru(string $command, ?int &$result_code = null): void
+{
+ ServeTest::$capturedCommand = $command;
+ $result_code = 0;
+}
+
+/**
+ * @internal
+ */
+#[Group('Others')]
+final class ServeTest extends CIUnitTestCase
+{
+ use StreamFilterTrait;
+
+ /**
+ * Captured shell command from the namespaced passthru() stub above.
+ */
+ public static string $capturedCommand = '';
+
+ protected function setUp(): void
+ {
+ parent::setUp();
+
+ self::$capturedCommand = '';
+ $_SERVER['argv'] = ['spark', 'serve'];
+ }
+
+ protected function tearDown(): void
+ {
+ // Restore default argv so other tests are not affected.
+ $_SERVER['argv'] = ['spark'];
+
+ parent::tearDown();
+ }
+
+ public function testServeRunsWithDefaultHostAndPort(): void
+ {
+ command('serve');
+
+ $this->assertStringContainsString(' -S ', self::$capturedCommand);
+ $this->assertStringContainsString("'localhost:8080'", self::$capturedCommand);
+ }
+
+ public function testServeEscapesShellMetacharactersInHost(): void
+ {
+ $_SERVER['argv'] = ['spark', 'serve', '--host', '$(id)'];
+
+ command('serve --host="$(id)"');
+
+ // The literal '$(id)' must remain inside single quotes so /bin/sh -c
+ // never expands it into a sub-shell. escapeshellarg() wraps the
+ // entire host:port pair in single quotes and escapes any embedded
+ // single quote.
+ $this->assertStringNotContainsString(' $(id) ', self::$capturedCommand);
+ $this->assertMatchesRegularExpression(
+ "/'[^']*\\\$\\(id\\)[^']*:[0-9]+'/",
+ self::$capturedCommand,
+ 'host:port must be wrapped in single quotes by escapeshellarg()',
+ );
+ }
+
+ public function testServeEscapesEmbeddedSingleQuoteInHost(): void
+ {
+ $_SERVER['argv'] = ['spark', 'serve', '--host', "evil'host"];
+
+ command("serve --host=\"evil'host\"");
+
+ // escapeshellarg() turns a single quote into '\'' inside the wrapper,
+ // so the dangerous quote can never break out of the argument.
+ $this->assertStringNotContainsString(" evil'host:", self::$capturedCommand);
+ $this->assertStringContainsString("'\\''", self::$capturedCommand);
+ }
+}