Merge branch 'develop' into ts3-query-fix

Author: DaneEveritt

CHANGELOG.md

@@ -3,6 +3,9 @@ This file is a running track of new features and fixes to each version of the pa
 
 This project follows [Semantic Versioning](http://semver.org) guidelines.
 
+## v1.2.2
+* **[security]** Fixes authentication bypass allowing a user to take control of specific server actions such as executing schedules, rotating database passwords, and viewing or deleting a backup.
+
 ## v1.2.1
 ### Fixed
 * Fixes URL-encoding of filenames when working in the filemanager to fix issues when moving, renaming, or deleting files.

app/Helpers/Utilities.php

@@ -42,13 +42,14 @@ public static function randomStringWithSpecialCharacters(int $length = 16): stri
      * @param string $minute
      * @param string $hour
      * @param string $dayOfMonth
+     * @param string $month
      * @param string $dayOfWeek
      * @return \Carbon\Carbon
      */
-    public static function getScheduleNextRunDate(string $minute, string $hour, string $dayOfMonth, string $dayOfWeek)
+    public static function getScheduleNextRunDate(string $minute, string $hour, string $dayOfMonth, string $month, string $dayOfWeek)
     {
         return Carbon::instance(CronExpression::factory(
-            sprintf('%s %s %s * %s', $minute, $hour, $dayOfMonth, $dayOfWeek)
+            sprintf('%s %s %s %s %s', $minute, $hour, $dayOfMonth, $month, $dayOfWeek)
         )->getNextRunDate());
     }
 

app/Http/Controllers/Api/Client/Servers/ScheduleController.php

@@ -84,6 +84,7 @@ public function store(StoreScheduleRequest $request, Server $server)
             'server_id' => $server->id,
             'name' => $request->input('name'),
             'cron_day_of_week' => $request->input('day_of_week'),
+            'cron_month' => $request->input('month'),
             'cron_day_of_month' => $request->input('day_of_month'),
             'cron_hour' => $request->input('hour'),
             'cron_minute' => $request->input('minute'),
@@ -136,6 +137,7 @@ public function update(UpdateScheduleRequest $request, Server $server, Schedule
         $data = [
             'name' => $request->input('name'),
             'cron_day_of_week' => $request->input('day_of_week'),
+            'cron_month' => $request->input('month'),
             'cron_day_of_month' => $request->input('day_of_month'),
             'cron_hour' => $request->input('hour'),
             'cron_minute' => $request->input('minute'),
@@ -211,6 +213,7 @@ protected function getNextRunAt(Request $request): Carbon
                 $request->input('minute'),
                 $request->input('hour'),
                 $request->input('day_of_month'),
+                $request->input('month'),
                 $request->input('day_of_week')
             );
         } catch (Exception $exception) {

app/Http/Middleware/Api/Client/Server/AllocationBelongsToServer.php

@@ -0,0 +1,92 @@
+<?php
+
+namespace Pterodactyl\Http\Middleware\Api\Client\Server;
+
+use Closure;
+use Illuminate\Http\Request;
+use Pterodactyl\Models\Task;
+use Pterodactyl\Models\User;
+use InvalidArgumentException;
+use Pterodactyl\Models\Server;
+use Pterodactyl\Models\Backup;
+use Pterodactyl\Models\Subuser;
+use Pterodactyl\Models\Schedule;
+use Pterodactyl\Models\Database;
+use Pterodactyl\Models\Allocation;
+use Illuminate\Database\Eloquent\Model;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
+
+class ResourceBelongsToServer
+{
+    /**
+     * Looks at the request parameters to determine if the given resource belongs
+     * to the requested server. If not, a 404 error will be returned to the caller.
+     *
+     * This is critical to ensuring that all subsequent logic is using exactly the
+     * server that is expected, and that we're not accessing a resource completely
+     * unrelated to the server provided in the request.
+     *
+     * @param \Illuminate\Http\Request $request
+     * @param \Closure $next
+     * @return mixed
+     */
+    public function handle(Request $request, Closure $next)
+    {
+        $params = $request->route()->parameters();
+        if (is_null($params) || ! $params['server'] instanceof Server) {
+            throw new InvalidArgumentException('This middleware cannot be used in a context that is missing a server in the parameters.');
+        }
+
+        /** @var \Pterodactyl\Models\Server $server */
+        $server = $request->route()->parameter('server');
+        $exception = new NotFoundHttpException('The requested resource was not found for this server.');
+        foreach ($params as $key => $model) {
+            // Specifically skip the server, we're just trying to see if all of the
+            // other resources are assigned to this server. Also skip anything that
+            // is not currently a Model instance since those will just end up being
+            // a 404 down the road.
+            if ($key === 'server' || ! $model instanceof Model) {
+                continue;
+            }
+
+            switch (get_class($model)) {
+                // All of these models use "server_id" as the field key for the server
+                // they are assigned to, so the logic is identical for them all.
+                case Allocation::class:
+                case Backup::class:
+                case Database::class:
+                case Schedule::class:
+                case Subuser::class:
+                    if ($model->server_id !== $server->id) {
+                        throw $exception;
+                    }
+                    break;
+                // Regular users are a special case here as we need to make sure they're
+                // currently assigned as a subuser on the server.
+                case User::class:
+                    $subuser = $server->subusers()->where('user_id', $model->id)->first();
+                    if (is_null($subuser)) {
+                        throw $exception;
+                    }
+                    // This is a special case to avoid an additional query being triggered
+                    // in the underlying logic.
+                    $request->attributes->set('subuser', $subuser);
+                    break;
+                // Tasks are special since they're (currently) the only item in the API
+                // that requires something in addition to the server in order to be accessed.
+                case Task::class:
+                    $schedule = $request->route()->parameter('schedule');
+                    if ($model->schedule_id !== $schedule->id || $schedule->server_id !== $server->id) {
+                        throw $exception;
+                    }
+                    break;
+                default:
+                    // Don't return a 404 here since we want to make sure no one relies
+                    // on this middleware in a context in which it will not work. Fail safe.
+                    throw new InvalidArgumentException('There is no handler configured for a resource of this type: ' . get_class($model));
+            }
+        }
+
+        return $next($request);
+    }
+}

app/Http/Middleware/Api/Client/Server/ResourceBelongsToServer.php

@@ -12,6 +12,7 @@
  * @property int $server_id
  * @property string $name
  * @property string $cron_day_of_week
+ * @property string $cron_month
  * @property string $cron_day_of_month
  * @property string $cron_hour
  * @property string $cron_minute
@@ -58,6 +59,7 @@ class Schedule extends Model
         'server_id',
         'name',
         'cron_day_of_week',
+        'cron_month',
         'cron_day_of_month',
         'cron_hour',
         'cron_minute',
@@ -93,6 +95,7 @@ class Schedule extends Model
     protected $attributes = [
         'name' => null,
         'cron_day_of_week' => '*',
+        'cron_month' => '*',
         'cron_day_of_month' => '*',
         'cron_hour' => '*',
         'cron_minute' => '*',
@@ -107,6 +110,7 @@ class Schedule extends Model
         'server_id' => 'required|exists:servers,id',
         'name' => 'required|string|max:191',
         'cron_day_of_week' => 'required|string',
+        'cron_month' => 'required|string',
         'cron_day_of_month' => 'required|string',
         'cron_hour' => 'required|string',
         'cron_minute' => 'required|string',
@@ -123,7 +127,7 @@ class Schedule extends Model
      */
     public function getNextRunDate()
     {
-        $formatted = sprintf('%s %s %s * %s', $this->cron_minute, $this->cron_hour, $this->cron_day_of_month, $this->cron_day_of_week);
+        $formatted = sprintf('%s %s %s %s %s', $this->cron_minute, $this->cron_hour, $this->cron_day_of_month, $this->cron_month, $this->cron_day_of_week);
 
         return CarbonImmutable::createFromTimestamp(
             CronExpression::factory($formatted)->getNextRunDate()->getTimestamp()

app/Http/Middleware/Api/Client/Server/SubuserBelongsToServer.php

@@ -40,6 +40,7 @@ public function transform(Schedule $model)
             'cron' => [
                 'day_of_week' => $model->cron_day_of_week,
                 'day_of_month' => $model->cron_day_of_month,
+                'month' => $model->cron_month,
                 'hour' => $model->cron_hour,
                 'minute' => $model->cron_minute,
             ],

app/Models/Schedule.php

@@ -7,6 +7,8 @@
 use Pterodactyl\Models\Node;
 use Faker\Generator as Faker;
 use Pterodactyl\Models\ApiKey;
+use Pterodactyl\Models\Backup;
+use Pterodactyl\Models\Permission;
 
 /** @var \Illuminate\Database\Eloquent\Factory $factory */
 /*
@@ -134,7 +136,9 @@
 });
 
 $factory->define(Pterodactyl\Models\Subuser::class, function (Faker $faker) {
-    return [];
+    return [
+        'permissions' => [Permission::ACTION_WEBSOCKET_CONNECT],
+    ];
 });
 
 $factory->define(Pterodactyl\Models\Allocation::class, function (Faker $faker) {
@@ -161,7 +165,7 @@
         'database' => str_random(10),
         'username' => str_random(10),
         'remote' => '%',
-        'password' => $password ?: bcrypt('test123'),
+        'password' => $password ?: encrypt('test123'),
         'created_at' => Carbon::now()->toDateTimeString(),
         'updated_at' => Carbon::now()->toDateTimeString(),
     ];
@@ -196,3 +200,12 @@
         'updated_at' => Carbon::now()->toDateTimeString(),
     ];
 });
+
+$factory->define(Pterodactyl\Models\Backup::class, function (Faker $faker) {
+    return [
+        'uuid' => Uuid::uuid4()->toString(),
+        'is_successful' => true,
+        'name' => $faker->sentence,
+        'disk' => Backup::ADAPTER_WINGS,
+    ];
+});

app/Transformers/Api/Client/ScheduleTransformer.php

@@ -0,0 +1,32 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+class AddCronMonth extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Schema::table('schedules', function (Blueprint $table) {
+            $table->string('cron_month')->after('cron_day_of_week');
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        Schema::table('schedules', function (Blueprint $table) {
+            $table->dropColumn('cron_month');
+        });
+    }
+}