Add Publisher restrictions

app/Config/Publisher.php

@@ -0,0 +1,28 @@
+<?php
+
+namespace Config;
+
+use CodeIgniter\Config\Publisher as BasePublisher;
+
+/**
+ * Publisher Configuration
+ *
+ * Defines basic security restrictions for the Publisher class
+ * to prevent abuse by injecting malicious files into a project.
+ */
+class Publisher extends BasePublisher
+{
+ /**
+ * A list of allowed destinations with a (pseudo-)regex
+ * of allowed files for each destination.
+ * Attempts to publish to directories not in this list will
+ * result in a PublisherException. Files that do no fit the
+ * pattern will cause copy/merge to fail.
+ *
+ * @var array<string,string>
+ */
+ public $restrictions = [
+ ROOTPATH => '*',
+ FCPATH => '#\.(?css|js|map|htm?|xml|json|webmanifest|tff|eot|woff?|gif|jpe?g|tiff?|png|webp|bmp|ico|svg)$#i',
+ ];
+}

system/Config/Publisher.php

@@ -0,0 +1,42 @@
+<?php
+
+/**
+ * This file is part of the 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\Config;
+
+/**
+ * Publisher Configuration
+ *
+ * Defines basic security restrictions for the Publisher class
+ * to prevent abuse by injecting malicious files into a project.
+ */
+class Publisher extends BaseConfig
+{
+ /**
+ * A list of allowed destinations with a (pseudo-)regex
+ * of allowed files for each destination.
+ * Attempts to publish to directories not in this list will
+ * result in a PublisherException. Files that do no fit the
+ * pattern will cause copy/merge to fail.
+ *
+ * @var array<string,string>
+ */
+ public $restrictions = [
+ ROOTPATH => '*',
+ FCPATH => '#\.(?css|js|map|htm?|xml|json|webmanifest|tff|eot|woff?|gif|jpe?g|tiff?|png|webp|bmp|ico|svg)$#i',
+ ];
+
+ /**
+ * Disables Registrars to prevent modules from altering the restrictions.
+ */
+ final protected function registerProperties()
+ {
+ }
+}

system/Language/en/Publisher.php

@@ -11,9 +11,11 @@
 
 // Publisher language settings
 return [
- 'collision' => 'Publisher encountered an unexpected {0} while copying {1} to {2}.',
- 'expectedDirectory' => 'Publisher::{0} expects a valid directory.',
- 'expectedFile' => 'Publisher::{0} expects a valid file.',
+ 'collision' => 'Publisher encountered an unexpected {0} while copying {1} to {2}.',
+ 'expectedDirectory' => 'Publisher::{0} expects a valid directory.',
+ 'expectedFile' => 'Publisher::{0} expects a valid file.',
+ 'destinationNotAllowed' => 'Destination is not on the allowed list of Publisher directories: {0}',
+ 'fileNotAllowed' => '{0} fails the following restriction for {1}: {2}',
 
  // Publish Command
  'publishMissing' => 'No Publisher classes detected in {0} across all namespaces.',

system/Publisher/Exceptions/PublisherException.php

@@ -50,4 +50,26 @@ public static function forExpectedFile(string $caller)
  {
  return new static(lang('Publisher.expectedFile', [$caller]));
  }
+
+ /**
+ * Throws when given a destination that is not in the list of allowed directories.
+ *
+ * @param string $destination
+ */
+ public static function forDestinationNotAllowed(string $destination)
+ {
+ return new static(lang('Publisher.destinationNotAllowed', [$destination]));
+ }
+
+ /**
+ * Throws when a file fails to match the allowed pattern for its destination.
+ *
+ * @param string $file
+ * @param string $directory
+ * @param string $pattern
+ */
+ public static function forFileNotAllowed(string $file, string $directory, string $pattern)
+ {
+ return new static(lang('Publisher.fileNotAllowed', [$file, $directory, $pattern]));
+ }
 }

system/Publisher/Publisher.php

@@ -72,6 +72,14 @@ class Publisher
  */
  private $published = [];
 
+ /**
+ * List of allowed directories and their allowed files regex.
+ * Restrictions are intentionally private to prevent overriding.
+ *
+ * @var array<string,string>
+ */
+ private $restrictions;
+
  /**
  * Base path to use for the source.
  *
@@ -134,6 +142,8 @@ final public static function discover(string $directory = 'Publishers'): array
  * @param string $directory
  *
  * @return string
+ *
+ * @throws PublisherException
  */
  private static function resolveDirectory(string $directory): string
  {
@@ -152,6 +162,8 @@ private static function resolveDirectory(string $directory): string
  * @param string $file
  *
  * @return string
+ *
+ * @throws PublisherException
  */
  private static function resolveFile(string $file): string
  {
@@ -191,7 +203,7 @@ private static function filterFiles(array $files, string $directory): array
  *
  * @return string[]
  */
- private static function matchFiles(array $files, string $pattern)
+ private static function matchFiles(array $files, string $pattern): array
  {
  // Convert pseudo-regex into their true form
  if (@preg_match($pattern, null) === false) // @phpstan-ignore-line
@@ -236,49 +248,6 @@ private static function wipeDirectory(string $directory): void
  }
  }
 
- /**
- * Copies a file with directory creation and identical file awareness.
- * Intentionally allows errors.
- *
- * @param string $from
- * @param string $to
- * @param boolean $replace
- *
- * @return void
- *
- * @throws PublisherException For unresolvable collisions
- */
- private static function safeCopyFile(string $from, string $to, bool $replace): void
- {
- // Check for an existing file
- if (file_exists($to))
- {
- // If not replacing or if files are identical then consider successful
- if (! $replace || same_file($from, $to))
- {
- return;
- }
-
- // If it is a directory then do not try to remove it
- if (is_dir($to))
- {
- throw PublisherException::forCollision($from, $to);
- }
-
- // Try to remove anything else
- unlink($to);
- }
-
- // Make sure the directory exists
- if (! is_dir($directory = pathinfo($to, PATHINFO_DIRNAME)))
- {
- mkdir($directory, 0775, true);
- }
-
- // Allow copy() to throw errors
- copy($from, $to);
- }
-
  //--------------------------------------------------------------------
 
  /**
@@ -293,6 +262,20 @@ public function __construct(string $source = null, string $destination = null)
 
  $this->source = self::resolveDirectory($source ?? $this->source);
  $this->destination = self::resolveDirectory($destination ?? $this->destination);
+
+ // Restrictions are intentionally not injected to prevent overriding
+ $this->restrictions = config('Publisher')->restrictions;
+
+ // Make sure the destination is allowed
+ foreach (array_keys($this->restrictions) as $directory)
+ {
+ if (strpos($this->destination, $directory) === 0)
+ {
+ return;
+ }
+ }
+
+ throw PublisherException::forDestinationNotAllowed($this->destination);
  }
 
  /**
@@ -314,6 +297,8 @@ public function __destruct()
  * discovery.
  *
  * @return boolean
+ *
+ * @throws RuntimeException
  */
  public function publish(): bool
  {
@@ -683,7 +668,7 @@ final public function copy(bool $replace = true): bool
 
  try
  {
- self::safeCopyFile($file, $to, $replace);
+ $this->safeCopyFile($file, $to, $replace);
  $this->published[] = $to;
  }
  catch (Throwable $e)
@@ -707,7 +692,7 @@ final public function merge(bool $replace = true): bool
  {
  $this->errors = $this->published = [];
 
- // Get the file from source for special handling
+ // Get the files from source for special handling
  $sourced = self::filterFiles($this->getFiles(), $this->source);
 
  // Handle everything else with a flat copy
@@ -722,7 +707,7 @@ final public function merge(bool $replace = true): bool
 
  try
  {
- self::safeCopyFile($file, $to, $replace);
+ $this->safeCopyFile($file, $to, $replace);
  $this->published[] = $to;
  }
  catch (Throwable $e)
@@ -733,4 +718,56 @@ final public function merge(bool $replace = true): bool
 
  return $this->errors === [];
  }
+
+ /**
+ * Copies a file with directory creation and identical file awareness.
+ * Intentionally allows errors.
+ *
+ * @param string $from
+ * @param string $to
+ * @param boolean $replace
+ *
+ * @return void
+ *
+ * @throws PublisherException For collisions and restriction violations
+ */
+ private function safeCopyFile(string $from, string $to, bool $replace): void
+ {
+ // Verify this is an allowed file for its destination
+ foreach ($this->restrictions as $directory => $pattern)
+ {
+ if (strpos($to, $directory) === 0 && self::matchFiles([$to], $pattern) === [])
+ {
+ throw PublisherException::forFileNotAllowed($from, $directory, $pattern);
+ }
+ }
+
+ // Check for an existing file
+ if (file_exists($to))
+ {
+ // If not replacing or if files are identical then consider successful
+ if (! $replace || same_file($from, $to))
+ {
+ return;
+ }
+
+ // If it is a directory then do not try to remove it
+ if (is_dir($to))
+ {
+ throw PublisherException::forCollision($from, $to);
+ }
+
+ // Try to remove anything else
+ unlink($to);
+ }
+
+ // Make sure the directory exists
+ if (! is_dir($directory = pathinfo($to, PATHINFO_DIRNAME)))
+ {
+ mkdir($directory, 0775, true);
+ }
+
+ // Allow copy() to throw errors
+ copy($from, $to);
+ }
 }

tests/_support/Config/Registrar.php

@@ -113,4 +113,18 @@ public static function Database()
 
  return $config;
  }
+
+ /**
+ * Demonstrates Publisher security.
+ *
+ * @see PublisherRestrictionsTest::testRegistrarsNotAllowed()
+ *
+ * @return array
+ */
+ public static function Publisher()
+ {
+ return [
+ 'restrictions' => [SUPPORTPATH => '*'],
+ ];
+ }
 }

tests/system/Publisher/PublisherInputTest.php

@@ -1,4 +1,4 @@
-<?php namespace CodeIgniter\Pager;
+<?php
 
 use CodeIgniter\Publisher\Exceptions\PublisherException;
 use CodeIgniter\Publisher\Publisher;

tests/system/Publisher/PublisherOutputTest.php

@@ -1,4 +1,4 @@
-<?php namespace CodeIgniter\Pager;
+<?php
 
 use CodeIgniter\Publisher\Exceptions\PublisherException;
 use CodeIgniter\Publisher\Publisher;
@@ -68,6 +68,9 @@ protected function setUp(): void
  ];
 
  $this->root = vfsStream::setup('root', null, $this->structure);
+
+ // Add root to the list of allowed destinations
+ config('Publisher')->restrictions[$this->root->url()] = '*';
  }
 
  //--------------------------------------------------------------------