From 4d1a7e66373713f560eca9b447674c4308b447b9 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 4 Aug 2021 22:20:43 -0700 Subject: [PATCH] Improve client API route model binding and prevent accidental route access without valid model binds --- .../Api/Client/Servers/ScheduleController.php | 4 - app/Http/Kernel.php | 2 + .../Client/SubstituteClientApiBindings.php | 103 ++++++++++++------ .../Middleware/Api/PreventUnboundModels.php | 49 +++++++++ 4 files changed, 119 insertions(+), 39 deletions(-) create mode 100644 app/Http/Middleware/Api/PreventUnboundModels.php diff --git a/app/Http/Controllers/Api/Client/Servers/ScheduleController.php b/app/Http/Controllers/Api/Client/Servers/ScheduleController.php index 9d5e229e2..be41317cd 100644 --- a/app/Http/Controllers/Api/Client/Servers/ScheduleController.php +++ b/app/Http/Controllers/Api/Client/Servers/ScheduleController.php @@ -88,10 +88,6 @@ class ScheduleController extends ClientApiController */ public function view(ViewScheduleRequest $request, Server $server, Schedule $schedule): array { - if ($schedule->server_id !== $server->id) { - throw new NotFoundHttpException(); - } - $schedule->loadMissing('tasks'); return $this->fractal->item($schedule) diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index bd48fa5ec..3eb6c2405 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -21,6 +21,7 @@ use Illuminate\View\Middleware\ShareErrorsFromSession; use Pterodactyl\Http\Middleware\MaintenanceMiddleware; use Pterodactyl\Http\Middleware\RedirectIfAuthenticated; use Illuminate\Auth\Middleware\AuthenticateWithBasicAuth; +use Pterodactyl\Http\Middleware\Api\PreventUnboundModels; use Pterodactyl\Http\Middleware\Api\ApiSubstituteBindings; use Illuminate\Foundation\Http\Middleware\ValidatePostSize; use Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse; @@ -76,6 +77,7 @@ class Kernel extends HttpKernel EnsureFrontendRequestsAreStateful::class, 'auth:sanctum', SubstituteClientApiBindings::class, + PreventUnboundModels::class, // This is perhaps a little backwards with the Client API, but logically you'd be unable // to create/get an API key without first enabling 2FA on the account, so I suppose in the // end it makes sense. diff --git a/app/Http/Middleware/Api/Client/SubstituteClientApiBindings.php b/app/Http/Middleware/Api/Client/SubstituteClientApiBindings.php index 31590f28a..a5ea1c786 100644 --- a/app/Http/Middleware/Api/Client/SubstituteClientApiBindings.php +++ b/app/Http/Middleware/Api/Client/SubstituteClientApiBindings.php @@ -3,60 +3,93 @@ namespace Pterodactyl\Http\Middleware\Api\Client; use Closure; -use Pterodactyl\Models\User; -use Pterodactyl\Models\Backup; -use Pterodactyl\Models\Database; +use Illuminate\Support\Str; +use Illuminate\Routing\Route; +use Pterodactyl\Models\Server; use Illuminate\Container\Container; +use Illuminate\Database\Query\JoinClause; +use Illuminate\Contracts\Routing\Registrar; use Pterodactyl\Contracts\Extensions\HashidsInterface; -use Pterodactyl\Http\Middleware\Api\ApiSubstituteBindings; -use Pterodactyl\Exceptions\Repository\RecordNotFoundException; -use Pterodactyl\Contracts\Repository\ServerRepositoryInterface; +use Illuminate\Database\Eloquent\ModelNotFoundException; -class SubstituteClientApiBindings extends ApiSubstituteBindings +class SubstituteClientApiBindings { + private Registrar $router; + + public function __construct(Registrar $router) + { + $this->router = $router; + } + /** - * Perform substitution of route parameters without triggering - * a 404 error if a model is not found. - * - * @param \Illuminate\Http\Request $request + * Perform substitution of route parameters for the Client API. * + * @param \Illuminate\Http\Request + * @param \Closure $next * @return mixed */ public function handle($request, Closure $next) { - // Override default behavior of the model binding to use a specific table - // column rather than the default 'id'. - $this->router->bind('server', function ($value) use ($request) { - try { - $column = 'uuidShort'; - if (preg_match('/^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i', $value)) { - $column = 'uuid'; - } - - return Container::getInstance()->make(ServerRepositoryInterface::class)->findFirstWhere([ - [$column, '=', $value], - ]); - } catch (RecordNotFoundException $ex) { - $request->attributes->set('is_missing_model', true); - - return null; - } + $this->router->bind('server', function ($value) { + return Server::query()->where(Str::length($value) === 8 ? 'uuidShort' : 'uuid', $value)->firstOrFail(); }); - $this->router->bind('database', function ($value) { + $this->router->bind('allocation', function ($value, $route) { + return $this->server($route)->allocations()->where('id', $value)->firstOrFail(); + }); + + $this->router->bind('schedule', function ($value, $route) { + return $this->server($route)->schedule()->where('id', $value)->firstOrFail(); + }); + + $this->router->bind('database', function ($value, $route) { $id = Container::getInstance()->make(HashidsInterface::class)->decodeFirst($value); - return Database::query()->where('id', $id)->firstOrFail(); + return $this->server($route)->where('id', $id)->firstOrFail(); }); - $this->router->bind('backup', function ($value) { - return Backup::query()->where('uuid', $value)->firstOrFail(); + $this->router->bind('backup', function ($value, $route) { + return $this->server($route)->backups()->where('uuid', $value)->firstOrFail(); }); - $this->router->bind('user', function ($value) { - return User::query()->where('uuid', $value)->firstOrFail(); + $this->router->bind('user', function ($value, $route) { + // TODO: is this actually a valid binding for users on the server? + return $this->server($route)->subusers() + ->join('users', function (JoinClause $join) { + $join->on('subusers.user_id', 'users.id') + ->where('subusers.server_id', 'servers.id'); + }) + ->where('users.uuid', $value) + ->firstOrFail(); }); - return parent::handle($request, $next); + try { + /** @var \Illuminate\Routing\Route $route */ + $this->router->substituteBindings($route = $request->route()); + } catch (ModelNotFoundException $exception) { + if (isset($route) && $route->getMissing()) { + $route->getMissing()($request); + } + + throw $exception; + } + + return $next($request); + } + + /** + * Plucks the server model off the route. If no server model is present a + * ModelNotFound exception will be thrown. + * + * @throws \Illuminate\Database\Eloquent\ModelNotFoundException + */ + private function server(Route $route): Server + { + $server = $route->parameter('server'); + if (!$server instanceof Server) { + throw (new ModelNotFoundException())->setModel(Server::class, [$route->parameter('server')]); + } + + return $server; } } diff --git a/app/Http/Middleware/Api/PreventUnboundModels.php b/app/Http/Middleware/Api/PreventUnboundModels.php new file mode 100644 index 000000000..ff3cd402e --- /dev/null +++ b/app/Http/Middleware/Api/PreventUnboundModels.php @@ -0,0 +1,49 @@ +route(); + + $parameters = $route->signatureParameters(UrlRoutable::class); + for ($i = 0; $i < count($route->parameters()); $i++) { + $class = Reflector::getParameterClassName($parameters[$i + 1]); + + // Skip anything that isn't explicitly requested as a model. + if (is_null($class) || !is_subclass_of($class, Model::class)) { + continue; + } + + if (!array_values($route->parameters())[$i] instanceof $class) { + throw new BindingResolutionException( + sprintf( + 'No parameter binding has been defined for model [%s] using route parameter key "%s".', + $class, + array_keys($route->parameters())[$i] + ) + ); + } + } + + return $next($request); + } +}