From d6a9b5a5e9f7f5f0aa99ed129cda1a315a83f37f Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Fri, 26 Dec 2025 14:57:59 +0530 Subject: [PATCH 1/4] security(backup): add escapeshellarg() to all rclone shell commands Add proper shell argument escaping to prevent command injection in rclone operations: - rclone size --json (line 995) - rclone lsf --dirs-only (line 1172) - rclone copy in rclone_download() (line 1251) - rclone copy in rclone_upload() (line 1280) - rclone lsf after upload (line 1292) Note: rclone purge commands in cleanup_old_backups() and rollback_failed_backup() were already properly escaped. This is a defense-in-depth measure as paths are derived from site_url in the database, but protects against potential command injection if malicious data were to be inserted. --- src/helper/Site_Backup_Restore.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/helper/Site_Backup_Restore.php b/src/helper/Site_Backup_Restore.php index 9576e344..1860e42d 100644 --- a/src/helper/Site_Backup_Restore.php +++ b/src/helper/Site_Backup_Restore.php @@ -992,7 +992,7 @@ private function pre_restore_check() { $this->pre_backup_restore_checks(); $remote_path = $this->get_remote_path( false ); - $command = sprintf( 'rclone size --json %s', $remote_path ); + $command = sprintf( 'rclone size --json %s', escapeshellarg( $remote_path ) ); $output = EE::launch( $command ); if ( $output->return_code ) { @@ -1169,7 +1169,7 @@ private function list_remote_backups( $return = false ) { $remote_path = $this->get_rclone_config_path(); // Get remote path without creating a new timestamped folder - $command = sprintf( 'rclone lsf --dirs-only %s', $remote_path ); // List only directories + $command = sprintf( 'rclone lsf --dirs-only %s', escapeshellarg( $remote_path ) ); // List only directories $output = EE::launch( $command ); if ( $output->return_code !== 0 && ! $return ) { @@ -1248,7 +1248,7 @@ private function get_remote_path( $upload = true ) { private function rclone_download( $path ) { $cpu_cores = intval( EE::launch( 'nproc' )->stdout ); $multi_threads = min( intval( $cpu_cores ) * 2, 32 ); - $command = sprintf( "rclone copy -P --multi-thread-streams %d %s %s", $multi_threads, $this->get_remote_path( false ), $path ); + $command = sprintf( "rclone copy -P --multi-thread-streams %d %s %s", $multi_threads, escapeshellarg( $this->get_remote_path( false ) ), escapeshellarg( $path ) ); $output = EE::launch( $command ); if ( $output->return_code ) { @@ -1277,7 +1277,7 @@ private function rclone_upload( $path ) { $s3_flag = ' --s3-chunk-size=64M --s3-upload-concurrency ' . min( intval( $cpu_cores ) * 2, 32 ); } - $command = sprintf( "rclone copy -P %s --transfers %d --checkers %d --buffer-size %s %s %s", $s3_flag, $transfers, $transfers, $buffer_size, $path, $this->get_remote_path() ); + $command = sprintf( "rclone copy -P %s --transfers %d --checkers %d --buffer-size %s %s %s", $s3_flag, $transfers, $transfers, $buffer_size, escapeshellarg( $path ), escapeshellarg( $this->get_remote_path() ) ); $output = EE::launch( $command ); if ( $output->return_code ) { @@ -1289,7 +1289,7 @@ private function rclone_upload( $path ) { EE::error( 'Error uploading backup to remote storage.' ); } else { - $command = sprintf( 'rclone lsf %s', $this->get_remote_path( false ) ); + $command = sprintf( 'rclone lsf %s', escapeshellarg( $this->get_remote_path( false ) ) ); $output = EE::launch( $command ); $remote_path = $output->stdout; EE::success( 'Backup uploaded to remote storage. Remote path: ' . $remote_path ); From 3ebcd6f60867b7fe7dbf74beafdf62d354c2dbe1 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Fri, 26 Dec 2025 15:08:03 +0530 Subject: [PATCH 2/4] feat(backup): add global flock-based lock to serialize concurrent backups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement flock()-based serialization to ensure only one backup runs at a time. This prevents OOM killer (exit code 137) when multiple backups are triggered simultaneously by EasyDash, as each backup calculates available RAM independently. Problem: When 5 backups trigger at once, each checks available RAM (e.g., 8GB) and allocates buffers accordingly. Total memory = 5 × 8GB = 40GB → OOM killer. Solution: Global lock ensures backups run sequentially. Each backup gets accurate RAM reading and full system resources. Implementation details: - Uses PHP flock() for atomic, race-condition-free locking - Waiting backups poll every 30 seconds with status messages - Maximum wait time of 2 hours before timeout (error code 5001) - Lock file stores current backup site URL and PID for debugging - Shutdown handler ensures lock release on any exit (error, crash, kill) - release_global_backup_lock() is idempotent (safe to call multiple times) Lock file location: EE_ROOT_DIR/services/backup-global.lock Error codes: - 5001: Timeout waiting for another backup to complete - 5002: Cannot create backup lock file --- src/helper/Site_Backup_Restore.php | 96 ++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/src/helper/Site_Backup_Restore.php b/src/helper/Site_Backup_Restore.php index 1860e42d..7d9ac8d3 100644 --- a/src/helper/Site_Backup_Restore.php +++ b/src/helper/Site_Backup_Restore.php @@ -41,6 +41,9 @@ class Site_Backup_Restore { private $dash_error_type = 'unknown'; private $dash_error_code = 0; + // Global backup lock handle for serializing backups + private $global_backup_lock_handle = null; + public function __construct() { $this->fs = new Filesystem(); } @@ -97,6 +100,12 @@ public function backup( $args, $assoc_args = [] ) { register_shutdown_function( [ $this, 'dash_shutdown_handler' ] ); } + // Acquire global lock to serialize backups (prevents OOM from concurrent backups) + $this->acquire_global_backup_lock(); + + // Register shutdown handler to release lock on any exit (error, crash, etc.) + register_shutdown_function( [ $this, 'release_global_backup_lock' ] ); + $this->pre_backup_check(); $backup_dir = EE_BACKUP_DIR . '/' . $this->site_data['site_url']; @@ -146,6 +155,9 @@ public function backup( $args, $assoc_args = [] ) { } } + // Release global backup lock (also released by shutdown handler as safety net) + $this->release_global_backup_lock(); + delem_log( 'site backup end' ); } @@ -1603,4 +1615,88 @@ private function sanitize_count( $value ) { return intval( $value ); } + + /** + * Acquire a global backup lock to ensure only one backup runs at a time. + * Uses flock() for atomic, race-condition-free locking. + * + * This prevents multiple concurrent backups from exhausting system resources + * (RAM, CPU, disk I/O, network bandwidth) when triggered simultaneously. + * + * Note: flock() may not work reliably on NFS or other network filesystems. + * EE_ROOT_DIR should be on a local filesystem for proper lock behavior. + * + * @return void + */ + private function acquire_global_backup_lock() { + $lock_file = EE_ROOT_DIR . '/services/backup-global.lock'; + $max_wait = 86400; // 24 hours max wait + $waited = 0; + $interval = 60; // Check every 60 seconds + + // Ensure services directory exists + $services_dir = dirname( $lock_file ); + if ( ! $this->fs->exists( $services_dir ) ) { + $this->fs->mkdir( $services_dir ); + } + + // Open file handle (creates if doesn't exist) + $this->global_backup_lock_handle = fopen( $lock_file, 'c+' ); + + if ( ! $this->global_backup_lock_handle ) { + $this->capture_error( + 'Cannot create backup lock file', + self::ERROR_TYPE_FILESYSTEM, + 5002 + ); + EE::error( 'Cannot create backup lock file.' ); + } + + // Try to acquire exclusive lock (non-blocking first to log status) + while ( ! flock( $this->global_backup_lock_handle, LOCK_EX | LOCK_NB ) ) { + if ( $waited >= $max_wait ) { + fclose( $this->global_backup_lock_handle ); + $this->global_backup_lock_handle = null; + $this->capture_error( + 'Timeout waiting for another backup to complete', + self::ERROR_TYPE_LOCK, + 5001 + ); + EE::error( 'Timeout waiting for another backup. Try again later.' ); + } + + // Read who has the lock + rewind( $this->global_backup_lock_handle ); + $lock_info = stream_get_contents( $this->global_backup_lock_handle ); + + EE::log( sprintf( 'Another backup in progress (%s). Waiting... (%d/%d sec)', + trim( $lock_info ) ?: 'unknown', $waited, $max_wait ) ); + + sleep( $interval ); + $waited += $interval; + } + + // Got the lock! Write our info + ftruncate( $this->global_backup_lock_handle, 0 ); + rewind( $this->global_backup_lock_handle ); + fwrite( $this->global_backup_lock_handle, $this->site_data['site_url'] . ' (PID: ' . getmypid() . ')' ); + fflush( $this->global_backup_lock_handle ); + + EE::debug( 'Acquired global backup lock for: ' . $this->site_data['site_url'] ); + } + + /** + * Release the global backup lock. + * Safe to call multiple times (idempotent). + * + * @return void + */ + public function release_global_backup_lock() { + if ( $this->global_backup_lock_handle ) { + flock( $this->global_backup_lock_handle, LOCK_UN ); + fclose( $this->global_backup_lock_handle ); + $this->global_backup_lock_handle = null; + EE::debug( 'Released global backup lock' ); + } + } } From 9f2ed001aab1e8695cbb7b27ca03c8974982fbf1 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Fri, 26 Dec 2025 15:20:30 +0530 Subject: [PATCH 3/4] refactor(backup): move lock file to EE_BACKUP_DIR Move backup-global.lock from EE_ROOT_DIR/services/ to EE_BACKUP_DIR/ to keep all backup-related files in one location. Lock file location: EE_BACKUP_DIR/backup-global.lock --- src/helper/Site_Backup_Restore.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/helper/Site_Backup_Restore.php b/src/helper/Site_Backup_Restore.php index 7d9ac8d3..11d5353f 100644 --- a/src/helper/Site_Backup_Restore.php +++ b/src/helper/Site_Backup_Restore.php @@ -1624,20 +1624,19 @@ private function sanitize_count( $value ) { * (RAM, CPU, disk I/O, network bandwidth) when triggered simultaneously. * * Note: flock() may not work reliably on NFS or other network filesystems. - * EE_ROOT_DIR should be on a local filesystem for proper lock behavior. + * EE_BACKUP_DIR should be on a local filesystem for proper lock behavior. * * @return void */ private function acquire_global_backup_lock() { - $lock_file = EE_ROOT_DIR . '/services/backup-global.lock'; + $lock_file = EE_BACKUP_DIR . '/backup-global.lock'; $max_wait = 86400; // 24 hours max wait $waited = 0; $interval = 60; // Check every 60 seconds - // Ensure services directory exists - $services_dir = dirname( $lock_file ); - if ( ! $this->fs->exists( $services_dir ) ) { - $this->fs->mkdir( $services_dir ); + // Ensure backup directory exists + if ( ! $this->fs->exists( EE_BACKUP_DIR ) ) { + $this->fs->mkdir( EE_BACKUP_DIR ); } // Open file handle (creates if doesn't exist) From de5495206c8d10698495241eacbdf316b9154ec4 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Fri, 26 Dec 2025 15:30:20 +0530 Subject: [PATCH 4/4] fix(backup): use unique error code 5003 for lock timeout Error code 5001 was already used for PHP Fatal Errors in dash_shutdown_handler. Changed lock timeout to 5003 to avoid collision. --- src/helper/Site_Backup_Restore.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helper/Site_Backup_Restore.php b/src/helper/Site_Backup_Restore.php index 11d5353f..54bea922 100644 --- a/src/helper/Site_Backup_Restore.php +++ b/src/helper/Site_Backup_Restore.php @@ -1659,7 +1659,7 @@ private function acquire_global_backup_lock() { $this->capture_error( 'Timeout waiting for another backup to complete', self::ERROR_TYPE_LOCK, - 5001 + 5003 ); EE::error( 'Timeout waiting for another backup. Try again later.' ); }