From f7ecd895982ceb253dfc2b61f6389477ddaf9461 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Wed, 21 Oct 2026 17:09:21 +0530 Subject: [PATCH 1/4] fix(php-ini): disable session.upload_progress for new sites The session upload progress handler is not needed by WordPress or most apps; turning it off removes it as an attack surface (php2shell hardening). Apps that need it can set it back to On in custom.ini. --- templates/config/php-fpm/php.ini-56.mustache | 3 +++ templates/config/php-fpm/php.ini.mustache | 3 +++ 2 files changed, 6 insertions(+) diff --git a/templates/config/php-fpm/php.ini-56.mustache b/templates/config/php-fpm/php.ini-56.mustache index 91ea378..126c56e 100644 --- a/templates/config/php-fpm/php.ini-56.mustache +++ b/templates/config/php-fpm/php.ini-56.mustache @@ -3,5 +3,8 @@ upload_max_filesize = 100M post_max_size = 100M +; Session upload progress is off for security; set it to On only if your app needs it. +session.upload_progress.enabled = Off + [mail function] sendmail_path = /usr/sbin/sendmail -t -i -f no-reply@example.com diff --git a/templates/config/php-fpm/php.ini.mustache b/templates/config/php-fpm/php.ini.mustache index eb2436d..cce7ca7 100644 --- a/templates/config/php-fpm/php.ini.mustache +++ b/templates/config/php-fpm/php.ini.mustache @@ -3,5 +3,8 @@ upload_max_filesize = 100M post_max_size = 100M +; Session upload progress is off for security; set it to On only if your app needs it. +session.upload_progress.enabled = Off + [mail function] sendmail_path = /usr/bin/msmtp -t From 15b8fc32a951c4d1f7d6c4b52027dde57287d988 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Wed, 21 Oct 2026 17:09:21 +0530 Subject: [PATCH 2/4] fix(migration): disable session.upload_progress for existing php sites Adds session.upload_progress.enabled = Off to the custom.ini of every php site, enabled or disabled, unless it already sets it. Files are edited in place (owner, mode and inode kept), originals are backed up under EE_BACKUP_DIR and restored if the upgrade rolls back. Running sites get a graceful php-fpm reload after the image migration, skipped for php containers it recreated; disabled sites pick it up when enabled. Per-site problems are warnings so they don't block the upgrade. --- ...hp_disable_php_session_upload_progress.php | 360 ++++++++++++++++++ 1 file changed, 360 insertions(+) create mode 100644 migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php diff --git a/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php new file mode 100644 index 0000000..597a9d3 --- /dev/null +++ b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php @@ -0,0 +1,360 @@ + [ 'site' => Site, 'file' => string, 'backup' => string|null ] for every custom.ini changed by up(). */ + private $changed = []; + + /** @var array site_url => php container ID seen by up(), for running sites. */ + private $containers = []; + + /** @var string|null */ + private $backup_root; + + public function __construct() { + + parent::__construct(); + if ( $this->is_first_execution ) { + $this->skip_this_migration = true; + + return; + } + + $this->sites = array_filter( + Site::all() ?: [], + function ( $site ) { + return self::SITE_TYPE === $site->site_type; + } + ); + if ( empty( $this->sites ) ) { + $this->skip_this_migration = true; + } + } + + /** + * Add the setting to every site's custom.ini. + * + * @throws \Throwable + */ + public function up() { + + if ( $this->skip_this_migration ) { + EE::debug( 'Skipping disable-session-upload-progress migration as it is not needed.' ); + + return; + } + + $this->backup_root = EE_BACKUP_DIR . '/session-upload-progress-' . self::SITE_TYPE . '-' . date( 'Ymd-His' ); + if ( file_exists( $this->backup_root ) ) { + $this->backup_root .= '-' . getmypid(); + } + + try { + foreach ( $this->sites as $site ) { + $this->update_site( $site ); + } + } catch ( \Throwable $e ) { + $this->restore_files(); + throw $e; + } + + if ( empty( $this->changed ) ) { + return; + } + + EE::log( 'Disabled session.upload_progress in custom.ini of ' . count( $this->changed ) . ' ' . self::SITE_TYPE . ' site(s). The original files are in ' . $this->backup_root ); + + // The image migration may recreate php containers, which then read the new file anyway; reload only the others, once it is done. + if ( method_exists( '\EE\Migration\CustomContainerMigrations', 'keep_executed_migrations' ) ) { + EE::add_hook( 'after_docker_image_migration', [ $this, 'reload_unchanged_containers' ] ); + } else { + $this->reload_unchanged_containers(); + } + } + + /** + * Restore the original custom.ini files and reload the running sites. + */ + public function down() { + + if ( empty( $this->changed ) ) { + return; + } + + $sites = array_column( $this->changed, 'site' ); + $this->restore_files(); + + // A rolled-back image migration may have restarted them on the new file. + foreach ( $sites as $site ) { + if ( $site->site_enabled ) { + $id = $this->get_php_container( $site ); + if ( '' !== $id ) { + $this->reload_php( $site->site_url, $id ); + } + } + } + } + + /** + * Gracefully reload php-fpm of the sites whose php container is still the one up() saw. + */ + public function reload_unchanged_containers() { + + foreach ( $this->containers as $site_url => $id ) { + if ( ! isset( $this->changed[ $site_url ] ) ) { + continue; + } + if ( $this->get_php_container( $this->changed[ $site_url ]['site'] ) !== $id ) { + EE::debug( "$site_url: php container was recreated or stopped, no reload needed." ); + continue; + } + $this->reload_php( $site_url, $id ); + } + $this->containers = []; + } + + /** + * Return custom.ini content with the setting added, or null when it already sets it. + * + * @param string $content Current content. + * + * @return string|null + */ + public static function add_setting( string $content ) { + + $bom = ''; + if ( 0 === strpos( $content, "\xEF\xBB\xBF" ) ) { + $bom = "\xEF\xBB\xBF"; + $content = substr( $content, 3 ); + } + + // Directive names are case-sensitive, so only this spelling takes effect. + if ( preg_match( '/^[ \t]*session\.upload_progress\.enabled[ \t]*=/m', $content ) ) { + return null; + } + + $eol = false !== strpos( $content, "\r\n" ) ? "\r\n" : "\n"; + $lines = implode( $eol, self::SETTING_LINES ) . $eol; + + if ( '' === trim( $content ) ) { + return $bom . $lines; + } + + // Directives after a [PATH=] or [HOST=] section apply only there, so it goes before the first section. + if ( preg_match( '/^[ \t]*\[/m', $content, $matches, PREG_OFFSET_CAPTURE ) ) { + $pos = $matches[0][1]; + + return $bom . substr( $content, 0, $pos ) . $lines . $eol . substr( $content, $pos ); + } + + if ( "\n" !== substr( $content, -1 ) ) { + $content .= $eol; + } + + return $bom . $content . $eol . $lines; + } + + /** + * Add the setting to one site's custom.ini. Problems with one site are warnings, so they don't block the upgrade. + * + * @param Site $site Site to update. + */ + private function update_site( $site ) { + + $conf_dir = $site->site_fs_path . '/config/php/php/conf.d'; + $file = $conf_dir . '/custom.ini'; + $manual = "Add 'session.upload_progress.enabled = Off' to $file and run: ee site reload $site->site_url"; + + if ( is_link( $file ) || ( file_exists( $file ) && ! is_file( $file ) ) ) { + EE::warning( "Skipped disabling session.upload_progress for $site->site_url: custom.ini is not a regular file. $manual" ); + + return; + } + + $created = ! file_exists( $file ); + if ( $created && ! is_dir( $conf_dir ) ) { + EE::warning( "Skipped disabling session.upload_progress for $site->site_url: $conf_dir is missing. $manual" ); + + return; + } + + $content = $created ? '' : @file_get_contents( $file ); + if ( false === $content ) { + EE::warning( "Skipped disabling session.upload_progress for $site->site_url: could not read custom.ini. $manual" ); + + return; + } + + $new_content = self::add_setting( $content ); + if ( null === $new_content ) { + EE::debug( "$site->site_url: custom.ini already sets session.upload_progress.enabled, leaving it." ); + + return; + } + + $backup = null; + if ( ! $created ) { + $backup = $this->backup_root . '/' . $site->site_url . '/custom.ini'; + try { + $this->fs->mkdir( dirname( $backup ), 0700 ); + $this->fs->copy( $file, $backup, true ); + $this->fs->chmod( $backup, fileperms( $file ) & 0777 ); + $this->fs->touch( $backup, filemtime( $file ) ); + } catch ( \Throwable $e ) { + EE::warning( "Skipped disabling session.upload_progress for $site->site_url: could not back up custom.ini ({$e->getMessage()}). $manual" ); + + return; + } + } + + $this->changed[ $site->site_url ] = [ + 'site' => $site, + 'file' => $file, + 'backup' => $backup, + ]; + + // Written in place: keeps owner, mode and inode (macOS bind-mounts this single file). + if ( ! $this->write_in_place( $file, $new_content ) ) { + $this->restore_file( $site->site_url ); + EE::warning( "Could not disable session.upload_progress for $site->site_url: could not write custom.ini. $manual" ); + + return; + } + + if ( $created ) { + $this->match_owner( $file, $conf_dir ); + } + + $id = $site->site_enabled ? $this->get_php_container( $site ) : ''; + if ( '' !== $id ) { + $this->containers[ $site->site_url ] = $id; + } + EE::debug( "$site->site_url: disabled session.upload_progress in custom.ini." ); + } + + /** + * Restore every file changed by up(). + */ + private function restore_files() { + + foreach ( array_keys( $this->changed ) as $site_url ) { + $this->restore_file( $site_url ); + } + $this->containers = []; + + if ( ! empty( $this->backup_root ) && is_dir( $this->backup_root ) && ! ( new \FilesystemIterator( $this->backup_root ) )->valid() ) { + $this->fs->remove( $this->backup_root ); + } + } + + /** + * Restore one file changed by up(); its backup is removed once restored. + * + * @param string $site_url Site. + */ + private function restore_file( string $site_url ) { + + $entry = $this->changed[ $site_url ]; + unset( $this->changed[ $site_url ] ); + + if ( null === $entry['backup'] ) { + if ( ! @unlink( $entry['file'] ) && file_exists( $entry['file'] ) ) { + EE::warning( "Could not remove {$entry['file']} added by the session.upload_progress migration." ); + } + + return; + } + + $content = @file_get_contents( $entry['backup'] ); + // Skip the write when it never happened, e.g. on an immutable file. + if ( false === $content || ( @file_get_contents( $entry['file'] ) !== $content && ! $this->write_in_place( $entry['file'], $content ) ) ) { + EE::warning( "Could not restore {$entry['file']}; its original is {$entry['backup']}" ); + + return; + } + $this->fs->remove( dirname( $entry['backup'] ) ); + EE::debug( "$site_url: restored custom.ini." ); + } + + /** + * @param string $file File to overwrite without replacing it. + * @param string $content New content. + * + * @return bool Whether the file now holds exactly $content. + */ + private function write_in_place( string $file, string $content ) { + + $written = @file_put_contents( $file, $content ); + clearstatcache( true, $file ); + + return strlen( $content ) === $written && @file_get_contents( $file ) === $content; + } + + /** + * Give a new file the owner and group of its directory. + * + * @param string $file File. + * @param string $dir Directory. + */ + private function match_owner( string $file, string $dir ) { + + @chmod( $file, 0644 ); + @chown( $file, fileowner( $dir ) ); + @chgrp( $file, filegroup( $dir ) ); + } + + /** + * @param Site $site Site. + * + * @return string ID of the site's running php container, or '' when there is none. + */ + private function get_php_container( $site ) { + + $cwd = getcwd(); + if ( ! @chdir( $site->site_fs_path ) ) { + return ''; + } + $result = EE::launch( \EE_DOCKER::docker_compose_with_custom() . ' ps -q php' ); + if ( false !== $cwd ) { + chdir( $cwd ); + } + + return 0 === $result->return_code ? trim( $result->stdout ) : ''; + } + + /** + * Same graceful reload as `ee site reload`: php-fpm re-reads its ini files. + * + * @param string $site_url Site. + * @param string $id php container ID. + */ + private function reload_php( string $site_url, string $id ) { + + $result = EE::launch( 'docker exec --user root ' . escapeshellarg( $id ) . " sh -c 'kill -USR2 1'" ); + if ( 0 === $result->return_code ) { + EE::debug( "$site_url: reloaded php-fpm." ); + } else { + EE::warning( "Could not reload php-fpm of $site_url. Run: ee site reload $site_url" ); + } + } +} From 41126c870d04a6e2f15a6e7304b56b112c8814d0 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Wed, 21 Oct 2026 20:08:06 +0530 Subject: [PATCH 3/4] fix(migration): add a global setting when only [PATH=]/[HOST=] sections set it custom.ini is now read with PHP's own ini parser (same mode as php.ini) to find a global session.upload_progress.enabled, so a value set only inside a [PATH=] or [HOST=] section no longer counts. The new lines go at the very top of the file, where neither such a section nor a multi-line quoted value can capture them; for a file with a syntax error only the lines above it count, matching what PHP applies. A failed docker-compose ps is now a warning with the reload command instead of being treated as a stopped container. --- ...hp_disable_php_session_upload_progress.php | 75 +++++++++++++------ 1 file changed, 53 insertions(+), 22 deletions(-) diff --git a/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php index 597a9d3..97be2e6 100644 --- a/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php +++ b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php @@ -8,7 +8,7 @@ /** * Turns off session.upload_progress in the custom.ini of every PHP site, as hardening against php2shell (CVE-2026-109618, CVE-2026-109619). * - * A custom.ini that already sets it is left alone. Disabled sites pick it up on their next start. + * A custom.ini that already sets it outside [PATH=]/[HOST=] sections is left alone. Disabled sites pick it up on their next start. */ class DisablePhpSessionUploadProgress extends Base { @@ -108,7 +108,7 @@ public function down() { foreach ( $sites as $site ) { if ( $site->site_enabled ) { $id = $this->get_php_container( $site ); - if ( '' !== $id ) { + if ( null !== $id && '' !== $id ) { $this->reload_php( $site->site_url, $id ); } } @@ -124,8 +124,11 @@ public function reload_unchanged_containers() { if ( ! isset( $this->changed[ $site_url ] ) ) { continue; } - if ( $this->get_php_container( $this->changed[ $site_url ]['site'] ) !== $id ) { - EE::debug( "$site_url: php container was recreated or stopped, no reload needed." ); + $now = $this->get_php_container( $this->changed[ $site_url ]['site'] ); + if ( $now !== $id ) { + if ( null !== $now ) { + EE::debug( "$site_url: php container was recreated or stopped, no reload needed." ); + } continue; } $this->reload_php( $site_url, $id ); @@ -134,7 +137,7 @@ public function reload_unchanged_containers() { } /** - * Return custom.ini content with the setting added, or null when it already sets it. + * Return custom.ini content with the setting added, or null when it already sets it globally. * * @param string $content Current content. * @@ -148,8 +151,7 @@ public static function add_setting( string $content ) { $content = substr( $content, 3 ); } - // Directive names are case-sensitive, so only this spelling takes effect. - if ( preg_match( '/^[ \t]*session\.upload_progress\.enabled[ \t]*=/m', $content ) ) { + if ( self::sets_globally( $content ) ) { return null; } @@ -160,18 +162,38 @@ public static function add_setting( string $content ) { return $bom . $lines; } - // Directives after a [PATH=] or [HOST=] section apply only there, so it goes before the first section. - if ( preg_match( '/^[ \t]*\[/m', $content, $matches, PREG_OFFSET_CAPTURE ) ) { - $pos = $matches[0][1]; + // At the very top, so neither a [PATH=]/[HOST=] section nor a multi-line quoted value can capture it. + return $bom . $lines . $eol . $content; + } + + /** + * Whether the content sets the directive outside [PATH=] and [HOST=] sections, read in the same mode as php.ini. + * + * @param string $content ini content without BOM. + * + * @return bool + */ + public static function sets_globally( string $content ) { - return $bom . substr( $content, 0, $pos ) . $lines . $eol . substr( $content, $pos ); + $ini = @parse_ini_string( $content, true, INI_SCANNER_NORMAL ); + // PHP applies the lines above a syntax error, so only that part counts. + for ( $lines = preg_split( '/(?<=\n)/', $content ), $n = count( $lines ) - 1; false === $ini && $n > 0; $n-- ) { + $ini = @parse_ini_string( implode( '', array_slice( $lines, 0, $n ) ), true, INI_SCANNER_NORMAL ); + } + if ( false === $ini ) { + return false; } - if ( "\n" !== substr( $content, -1 ) ) { - $content .= $eol; + foreach ( $ini as $key => $value ) { + if ( 'session.upload_progress.enabled' === $key && ! is_array( $value ) ) { + return true; + } + if ( is_array( $value ) && array_key_exists( 'session.upload_progress.enabled', $value ) && ! preg_match( '/^(PATH|HOST)=/i', (string) $key ) ) { + return true; + } } - return $bom . $content . $eol . $lines; + return false; } /** @@ -212,6 +234,12 @@ private function update_site( $site ) { return; } + if ( ! self::sets_globally( preg_replace( '/^\xEF\xBB\xBF/', '', $new_content ) ) ) { + EE::warning( "Skipped disabling session.upload_progress for $site->site_url: could not add it to custom.ini safely. $manual" ); + + return; + } + $backup = null; if ( ! $created ) { $backup = $this->backup_root . '/' . $site->site_url . '/custom.ini'; @@ -246,7 +274,7 @@ private function update_site( $site ) { } $id = $site->site_enabled ? $this->get_php_container( $site ) : ''; - if ( '' !== $id ) { + if ( null !== $id && '' !== $id ) { $this->containers[ $site->site_url ] = $id; } EE::debug( "$site->site_url: disabled session.upload_progress in custom.ini." ); @@ -326,20 +354,23 @@ private function match_owner( string $file, string $dir ) { /** * @param Site $site Site. * - * @return string ID of the site's running php container, or '' when there is none. + * @return string|null ID of the site's running php container, '' when none runs, null when the lookup failed (already warned). */ private function get_php_container( $site ) { - $cwd = getcwd(); - if ( ! @chdir( $site->site_fs_path ) ) { - return ''; - } - $result = EE::launch( \EE_DOCKER::docker_compose_with_custom() . ' ps -q php' ); + $cwd = getcwd(); + $result = @chdir( $site->site_fs_path ) ? EE::launch( \EE_DOCKER::docker_compose_with_custom() . ' ps -q php' ) : null; if ( false !== $cwd ) { chdir( $cwd ); } - return 0 === $result->return_code ? trim( $result->stdout ) : ''; + if ( null === $result || 0 !== $result->return_code ) { + EE::warning( "Could not check the php container of $site->site_url, so php-fpm may still run with the old session.upload_progress setting. Fix the error, then run: ee site reload $site->site_url" ); + + return null; + } + + return trim( $result->stdout ); } /** From ec39de2d734941fb470a378d9b7d42c9905c07b8 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Wed, 21 Oct 2026 20:33:44 +0530 Subject: [PATCH 4/4] fix(migration): treat every entry after a PATH or HOST section as scoped PHP treats any section whose name starts with PATH or HOST (case-insensitive, spaces allowed before the =) as scoped, and entries under a later ordinary header stay in that scope. Only the part before the first such header now counts as global, so a scoped-only setting no longer stops the global Off from being added. --- ...hp_disable_php_session_upload_progress.php | 38 +++++++++++++++---- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php index 97be2e6..24442ac 100644 --- a/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php +++ b/migrations/container/20261021120100_site-type-php_disable_php_session_upload_progress.php @@ -8,7 +8,7 @@ /** * Turns off session.upload_progress in the custom.ini of every PHP site, as hardening against php2shell (CVE-2026-109618, CVE-2026-109619). * - * A custom.ini that already sets it outside [PATH=]/[HOST=] sections is left alone. Disabled sites pick it up on their next start. + * A custom.ini that already sets it globally (before any [PATH]/[HOST] section) is left alone. Disabled sites pick it up on their next start. */ class DisablePhpSessionUploadProgress extends Base { @@ -167,7 +167,7 @@ public static function add_setting( string $content ) { } /** - * Whether the content sets the directive outside [PATH=] and [HOST=] sections, read in the same mode as php.ini. + * Whether the content sets the directive globally, read in the same mode as php.ini. * * @param string $content ini content without BOM. * @@ -175,9 +175,10 @@ public static function add_setting( string $content ) { */ public static function sets_globally( string $content ) { - $ini = @parse_ini_string( $content, true, INI_SCANNER_NORMAL ); + $global = self::global_part( $content ); + $ini = @parse_ini_string( $global, true, INI_SCANNER_NORMAL ); // PHP applies the lines above a syntax error, so only that part counts. - for ( $lines = preg_split( '/(?<=\n)/', $content ), $n = count( $lines ) - 1; false === $ini && $n > 0; $n-- ) { + for ( $lines = preg_split( '/(?<=\n)/', $global ), $n = count( $lines ) - 1; false === $ini && $n > 0; $n-- ) { $ini = @parse_ini_string( implode( '', array_slice( $lines, 0, $n ) ), true, INI_SCANNER_NORMAL ); } if ( false === $ini ) { @@ -185,10 +186,7 @@ public static function sets_globally( string $content ) { } foreach ( $ini as $key => $value ) { - if ( 'session.upload_progress.enabled' === $key && ! is_array( $value ) ) { - return true; - } - if ( is_array( $value ) && array_key_exists( 'session.upload_progress.enabled', $value ) && ! preg_match( '/^(PATH|HOST)=/i', (string) $key ) ) { + if ( is_array( $value ) ? array_key_exists( 'session.upload_progress.enabled', $value ) : 'session.upload_progress.enabled' === $key ) { return true; } } @@ -196,6 +194,30 @@ public static function sets_globally( string $content ) { return false; } + /** + * The part of the content before the first section whose name starts with PATH or HOST. + * + * PHP scopes every entry after such a header to it, even under later ordinary headers. + * + * @param string $content ini content without BOM. + * + * @return string + */ + private static function global_part( string $content ) { + + if ( preg_match_all( '/^[ \t]*\[(?:PATH|HOST)[^\r\n]*$/mi', $content, $matches, PREG_OFFSET_CAPTURE ) ) { + foreach ( $matches[0] as $match ) { + // Text inside a multi-line quoted value doesn't parse as the last section. + $ini = @parse_ini_string( substr( $content, 0, $match[1] + strlen( $match[0] ) ), true, INI_SCANNER_NORMAL ); + if ( false !== $ini && is_array( end( $ini ) ) && preg_match( '/^(PATH|HOST)/i', (string) key( $ini ) ) ) { + return substr( $content, 0, $match[1] ); + } + } + } + + return $content; + } + /** * Add the setting to one site's custom.ini. Problems with one site are warnings, so they don't block the upgrade. *