diff --git a/source/packages/com_mokojoombackup/src/Controller/AjaxController.php b/source/packages/com_mokojoombackup/src/Controller/AjaxController.php index 9e5c637..e08da9f 100644 --- a/source/packages/com_mokojoombackup/src/Controller/AjaxController.php +++ b/source/packages/com_mokojoombackup/src/Controller/AjaxController.php @@ -80,8 +80,24 @@ class AjaxController extends BaseController return; } - $path = $this->input->getString('path', JPATH_ROOT); - $path = realpath($path) ?: $path; + $requestPath = $this->input->getString('path', JPATH_ROOT); + $path = realpath($requestPath) ?: $requestPath; + + // Security: restrict browsing to site root and current user's home + $jRoot = realpath(JPATH_ROOT); + $homeDir = getenv('HOME') ?: (getenv('USERPROFILE') ?: ''); + $allowed = false; + + if ($jRoot !== false && strpos($path, $jRoot) === 0) { + $allowed = true; + } elseif ($homeDir !== '' && strpos($path, $homeDir) === 0) { + $allowed = true; + } + + if (!$allowed) { + $this->sendJson(['error' => true, 'message' => 'Access denied: path outside allowed directories']); + return; + } if (!is_dir($path)) { $this->sendJson(['error' => true, 'message' => 'Directory not found: ' . $path]); diff --git a/source/packages/com_mokojoombackup/src/Engine/BackupEngine.php b/source/packages/com_mokojoombackup/src/Engine/BackupEngine.php index 43d5d91..62aa3eb 100644 --- a/source/packages/com_mokojoombackup/src/Engine/BackupEngine.php +++ b/source/packages/com_mokojoombackup/src/Engine/BackupEngine.php @@ -67,7 +67,9 @@ class BackupEngine $this->backupDir = $this->resolveBackupDir($resolver->resolve($configuredDir)); if (!is_dir($this->backupDir)) { - mkdir($this->backupDir, 0755, true); + if (!mkdir($this->backupDir, 0755, true)) { + return ['success' => false, 'message' => 'Cannot create backup directory: ' . $this->backupDir, 'record_id' => 0]; + } } // Create backup record @@ -155,14 +157,22 @@ class BackupEngine $filesCount = count($filesToBackup); $this->log('Backing up ' . $filesCount . ' files'); + $skippedFiles = 0; + foreach ($filesToBackup as $relativePath) { $fullPath = JPATH_ROOT . '/' . $relativePath; if (is_file($fullPath) && is_readable($fullPath)) { $archiver->addFile($fullPath, $relativePath); + } else { + $skippedFiles++; } } + if ($skippedFiles > 0) { + $this->log('WARNING: ' . $skippedFiles . ' files skipped (not readable or missing)'); + } + $this->log('Files added to archive'); // Build manifest for full/differential backups (used by future differentials) @@ -239,7 +249,9 @@ class BackupEngine // Write log file alongside the archive $logContent = implode("\n", $this->log); $logPath = preg_replace('/\.(zip|tar\.gz)$/i', '.log', $archivePath); - @file_put_contents($logPath, $logContent); + if (@file_put_contents($logPath, $logContent) === false) { + error_log('MokoJoomBackup: Could not write log file: ' . $logPath); + } // Final record update $update = (object) [ @@ -493,7 +505,8 @@ class BackupEngine $app->getDispatcher()->dispatch('onMokoJoomBackupAfterRun', $event); } catch (\Throwable $e) { - // Never let a listener failure break the backup result + // Never let a listener failure break the backup result, but log it + error_log('MokoJoomBackup: onAfterRun listener error: ' . $e->getMessage()); } } diff --git a/source/packages/com_mokojoombackup/src/Engine/MokoRestore.php b/source/packages/com_mokojoombackup/src/Engine/MokoRestore.php index e17020b..4781699 100644 --- a/source/packages/com_mokojoombackup/src/Engine/MokoRestore.php +++ b/source/packages/com_mokojoombackup/src/Engine/MokoRestore.php @@ -373,7 +373,7 @@ function actionDatabase(array $data): array $pdo->exec('SET FOREIGN_KEY_CHECKS = 1'); return [ - 'success' => true, + 'success' => ($statements > 0 || $errors === 0), 'message' => "Executed {$statements} statements" . ($errors ? " ({$errors} warnings)" : ''), 'statements' => $statements, 'errors' => $errors, @@ -625,9 +625,20 @@ function actionCleanup(): array function getDbConnection(array $data): PDO { $host = $data['db_host'] ?? 'localhost'; +// Validate db_prefix to prevent SQL injection $prefix = $data['db_prefix'] ?? 'moko_'; if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_]{0,20}$/', $prefix)) { throw new RuntimeException('Invalid table prefix format'); } $name = $data['db_name'] ?? ''; +// Validate db_prefix to prevent SQL injection $prefix = $data['db_prefix'] ?? 'moko_'; if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_]{0,20}$/', $prefix)) { throw new RuntimeException('Invalid table prefix format'); } $user = $data['db_user'] ?? ''; +// Validate db_prefix to prevent SQL injection $prefix = $data['db_prefix'] ?? 'moko_'; if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_]{0,20}$/', $prefix)) { throw new RuntimeException('Invalid table prefix format'); } $pass = $data['db_pass'] ?? ''; +// Validate db_prefix to prevent SQL injection $prefix = $data['db_prefix'] ?? 'moko_'; if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_]{0,20}$/', $prefix)) { throw new RuntimeException('Invalid table prefix format'); } + + // Validate db_prefix to prevent SQL injection + $prefix = $data['db_prefix'] ?? 'moko_'; +// Validate db_prefix to prevent SQL injection $prefix = $data['db_prefix'] ?? 'moko_'; if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_]{0,20}$/', $prefix)) { throw new RuntimeException('Invalid table prefix format'); } + if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_]{0,20}$\/', $prefix)) { + throw new RuntimeException('Invalid table prefix format'); + } return new PDO( "mysql:host={$host};dbname={$name};charset=utf8mb4", diff --git a/source/packages/com_mokojoombackup/src/Engine/NotificationSender.php b/source/packages/com_mokojoombackup/src/Engine/NotificationSender.php index b26f7c0..fc2d96a 100644 --- a/source/packages/com_mokojoombackup/src/Engine/NotificationSender.php +++ b/source/packages/com_mokojoombackup/src/Engine/NotificationSender.php @@ -172,6 +172,8 @@ class NotificationSender return $db->loadColumn() ?: []; } catch (\Throwable $e) { + error_log('MokoJoomBackup: Could not resolve user group emails: ' . $e->getMessage()); + return []; } } diff --git a/source/packages/com_mokojoombackup/src/Engine/SteppedBackupEngine.php b/source/packages/com_mokojoombackup/src/Engine/SteppedBackupEngine.php index be900d5..e54b1b6 100644 --- a/source/packages/com_mokojoombackup/src/Engine/SteppedBackupEngine.php +++ b/source/packages/com_mokojoombackup/src/Engine/SteppedBackupEngine.php @@ -65,7 +65,9 @@ class SteppedBackupEngine $backupDir = $this->resolveBackupDir($resolver->resolve($session->backupDir)); if (!is_dir($backupDir)) { - mkdir($backupDir, 0755, true); + if (!mkdir($backupDir, 0755, true)) { + return ['error' => true, 'message' => 'Cannot create backup directory: ' . $backupDir]; + } } $now = date('Y-m-d H:i:s'); @@ -232,11 +234,15 @@ class SteppedBackupEngine . "-- Prefix: " . $db->getPrefix() . "\n\n" . "SET SQL_MODE = \"NO_AUTO_VALUE_ON_ZERO\";\n" . "SET time_zone = \"+00:00\";\n\n"; - file_put_contents($sqlFile, $header); + if (file_put_contents($sqlFile, $header) === false) { + throw new \RuntimeException('Cannot write SQL dump: ' . $sqlFile); + } $flags = FILE_APPEND; } - file_put_contents($sqlFile, $sql, $flags); + if (file_put_contents($sqlFile, $sql, $flags) === false) { + throw new \RuntimeException('Cannot write SQL dump: ' . $sqlFile); + } $session->dbSize += strlen($sql); $session->tableIndex++; @@ -369,6 +375,7 @@ class SteppedBackupEngine $uploader = match ($session->remoteStorage) { 'ftp' => new FtpUploader($profile), 'google_drive' => new GoogleDriveUploader($profile), + 's3' => new S3Uploader($profile), default => throw new \InvalidArgumentException('Unknown storage: ' . $session->remoteStorage), }; @@ -414,7 +421,9 @@ class SteppedBackupEngine // Write log file alongside the archive $logPath = preg_replace('/\.(zip|tar\.gz)$/i', '.log', $session->archivePath); - @file_put_contents($logPath, $logContent); + if (@file_put_contents($logPath, $logContent) === false) { + error_log('MokoJoomBackup: Could not write log file: ' . $logPath); + } $update = (object) [ 'id' => $session->recordId, diff --git a/source/packages/com_mokojoombackup/src/Engine/SteppedSession.php b/source/packages/com_mokojoombackup/src/Engine/SteppedSession.php index 2e0255b..7f83d80 100644 --- a/source/packages/com_mokojoombackup/src/Engine/SteppedSession.php +++ b/source/packages/com_mokojoombackup/src/Engine/SteppedSession.php @@ -65,7 +65,9 @@ class SteppedSession $dir = JPATH_ROOT . '/tmp/mokojoombackup-sessions'; if (!is_dir($dir)) { - mkdir($dir, 0755, true); + if (!mkdir($dir, 0755, true)) { + throw new \RuntimeException('Cannot create session directory: ' . $dir); + } } return $dir; @@ -124,7 +126,9 @@ class SteppedSession public function save(): void { $path = self::getSessionPath($this->sessionId); - file_put_contents($path, json_encode(get_object_vars($this), JSON_PRETTY_PRINT)); + if (file_put_contents($path, json_encode(get_object_vars($this), JSON_PRETTY_PRINT)) === false) { + throw new \RuntimeException('Cannot save backup session: ' . $path); + } } /** diff --git a/source/packages/plg_system_mokojoombackup/src/Extension/MokoJoomBackup.php b/source/packages/plg_system_mokojoombackup/src/Extension/MokoJoomBackup.php index 0387fd5..e459e3f 100644 --- a/source/packages/plg_system_mokojoombackup/src/Extension/MokoJoomBackup.php +++ b/source/packages/plg_system_mokojoombackup/src/Extension/MokoJoomBackup.php @@ -151,7 +151,9 @@ final class MokoJoomBackup extends CMSPlugin implements SubscriberInterface foreach ($expired as $record) { if (!empty($record->absolute_path) && is_file($record->absolute_path)) { - @unlink($record->absolute_path); + if (!@unlink($record->absolute_path)) { + continue; // Don't delete DB record if file can't be removed + } } $db->setQuery( @@ -182,7 +184,9 @@ final class MokoJoomBackup extends CMSPlugin implements SubscriberInterface foreach ($oldest as $record) { if (!empty($record->absolute_path) && is_file($record->absolute_path)) { - @unlink($record->absolute_path); + if (!@unlink($record->absolute_path)) { + continue; // Do not delete DB record if file cannot be removed + } } $db->setQuery( diff --git a/source/script.php b/source/script.php index 96244fb..b074313 100644 --- a/source/script.php +++ b/source/script.php @@ -101,7 +101,7 @@ class Pkg_MokoJoomBackupInstallerScript $this->savedDownloadKey = $key; } } catch (\Throwable $e) { - // Not critical + error_log('MokoJoomBackup: Could not save download key: ' . $e->getMessage()); } } @@ -242,7 +242,7 @@ class Pkg_MokoJoomBackupInstallerScript $db->execute(); } } catch (\Throwable $e) { - // Not critical + error_log('MokoJoomBackup: Could not restore download key: ' . $e->getMessage()); } } @@ -278,6 +278,8 @@ class Pkg_MokoJoomBackupInstallerScript 'warning' ); } - catch (\Throwable $e) {} + catch (\Throwable $e) { + error_log('MokoJoomBackup: License key check failed: ' . $e->getMessage()); + } } }