From 7bfc265a7ef0b7f8f55b69edb12a8552bfd50a8c Mon Sep 17 00:00:00 2001 From: Matthew Penner Date: Sat, 4 May 2024 16:06:13 -0600 Subject: [PATCH] api(remote): fix use of missing `node_id` field Fixes #5088 --- .../Backups/BackupRemoteUploadController.php | 19 +++++++++++++------ .../Remote/Backups/BackupStatusController.php | 9 ++++++++- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/app/Http/Controllers/Api/Remote/Backups/BackupRemoteUploadController.php b/app/Http/Controllers/Api/Remote/Backups/BackupRemoteUploadController.php index c3bf72662..15fe8d9bd 100644 --- a/app/Http/Controllers/Api/Remote/Backups/BackupRemoteUploadController.php +++ b/app/Http/Controllers/Api/Remote/Backups/BackupRemoteUploadController.php @@ -42,15 +42,22 @@ class BackupRemoteUploadController extends Controller throw new BadRequestHttpException('A non-empty "size" query parameter must be provided.'); } - /** @var \Pterodactyl\Models\Backup $backup */ - $backup = Backup::query() - ->where('node_id', $node->id) + /** @var \Pterodactyl\Models\Backup $model */ + $model = Backup::query() ->where('uuid', $backup) ->firstOrFail(); + // Check that the backup is "owned" by the node making the request. This avoids other nodes + // from messing with backups that they don't own. + /** @var \Pterodactyl\Models\Server $server */ + $server = $model->server; + if ($server->node_id !== $node->id) { + throw new HttpForbiddenException('You do not have permission to access that backup.'); + } + // Prevent backups that have already been completed from trying to // be uploaded again. - if (!is_null($backup->completed_at)) { + if (!is_null($model->completed_at)) { throw new ConflictHttpException('This backup is already in a completed state.'); } @@ -61,7 +68,7 @@ class BackupRemoteUploadController extends Controller } // The path where backup will be uploaded to - $path = sprintf('%s/%s.tar.gz', $backup->server->uuid, $backup->uuid); + $path = sprintf('%s/%s.tar.gz', $model->server->uuid, $model->uuid); // Get the S3 client $client = $adapter->getClient(); @@ -99,7 +106,7 @@ class BackupRemoteUploadController extends Controller } // Set the upload_id on the backup in the database. - $backup->update(['upload_id' => $params['UploadId']]); + $model->update(['upload_id' => $params['UploadId']]); return new JsonResponse([ 'parts' => $parts, diff --git a/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php b/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php index 75f039d28..7b30e0758 100644 --- a/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php +++ b/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php @@ -36,10 +36,17 @@ class BackupStatusController extends Controller /** @var \Pterodactyl\Models\Backup $model */ $model = Backup::query() - ->where('node_id', $node->id) ->where('uuid', $backup) ->firstOrFail(); + // Check that the backup is "owned" by the node making the request. This avoids other nodes + // from messing with backups that they don't own. + /** @var \Pterodactyl\Models\Server $server */ + $server = $model->server; + if ($server->node_id !== $node->id) { + throw new HttpForbiddenException('You do not have permission to access that backup.'); + } + if ($model->is_successful) { throw new BadRequestHttpException('Cannot update the status of a backup that is already marked as completed.'); }