Fix: Priviledge Escalation

app/Console/Commands/UserCreate.php

@@ -101,7 +101,6 @@ public function handle(DiscourseService $discourseService): void
                 'name' => $name,
                 'email' => $email,
                 'password' => Hash::make($password),
-                'role' => $role,
                 'recovery' => substr(bin2hex(openssl_random_pseudo_bytes(32)), 0, 24),
                 'recovery_expires' => strftime('%Y-%m-%d %X', time() + (24 * 60 * 60)),
                 'calendar_hash' => Str::random(15),
@@ -118,6 +117,9 @@ public function handle(DiscourseService $discourseService): void
 
             if ($user)
             {
+                $user->role = $role;
+                $user->save();
+
                 $this->info("User created {$user->name} (#{$user->id})");
                 $this->info("Role: {$this->getRoleName($user->role)}");
 

app/Http/Controllers/Auth/RegisterController.php

@@ -5,6 +5,7 @@
 use Illuminate\Routing\Controllers\HasMiddleware;
 use Illuminate\Routing\Controllers\Middleware;
 use App\Http\Controllers\Controller;
+use App\Models\Role;
 use App\Models\User;
 use Illuminate\Foundation\Auth\RegistersUsers;
 use Illuminate\Support\Facades\Hash;
@@ -62,11 +63,13 @@ protected function create(array $data): User
                                 'name' => $data['name'],
                                 'email' => $data['email'],
                                 'password' => Hash::make($data['password']),
-            'role' => 4,
             'recovery' => substr(bin2hex(openssl_random_pseudo_bytes(32)), 0, 24),
             'recovery_expires' => strftime('%Y-%m-%d %X', time() + (24 * 60 * 60)),
                             ]);
 
+        $user->role = Role::RESTARTER;
+        $user->save();
+
         Session::createSession($user->id);
 
         return $user;

app/Http/Controllers/UserController.php

@@ -149,10 +149,9 @@ public function postProfileInfoEdit(Request $request, App\Helpers\Geocoder $geoc
             return redirect()->back()->withErrors($validator)->withInput();
         }
 
-        if ($request->input('id') !== null) {
-            $id = $request->input('id');
-        } else {
-            $id = Auth::id();
+        $id = $request->input('id', Auth::id());
+        if ($id != Auth::id() && !Auth::user()->hasRole('Administrator')) {
+            abort(403);
         }
 
         User::find($id)->update([
@@ -188,10 +187,9 @@ public function postProfileInfoEdit(Request $request, App\Helpers\Geocoder $geoc
 
     public function postProfilePasswordEdit(Request $request): RedirectResponse
     {
-        if ($request->input('id') !== null) {
-            $id = $request->input('id');
-        } else {
-            $id = Auth::id();
+        $id = $request->input('id', Auth::id());
+        if ($id != Auth::id() && !Auth::user()->hasRole('Administrator')) {
+            abort(403);
         }
 
         $user = User::find($id);
@@ -346,10 +344,9 @@ public function postProfileTagsEdit(Request $request): RedirectResponse
 
     public function postProfilePictureEdit(Request $request): RedirectResponse
     {
-        if ($request->input('id') !== null) {
-            $id = $request->input('id');
-        } else {
-            $id = Auth::id();
+        $id = $request->input('id', Auth::id());
+        if ($id != Auth::id() && !Auth::user()->hasRole('Administrator')) {
+            abort(403);
         }
 
         if (isset($_FILES) && ! empty($_FILES)) {
@@ -364,20 +361,19 @@ public function postProfilePictureEdit(Request $request): RedirectResponse
 
     public function postAdminEdit(Request $request): RedirectResponse
     {
-        if ($request->input('id') !== null) {
-            $user_id = $request->input('id');
-        } else {
-            $user_id = Auth::id();
+        if (!Auth::user()->hasRole('Administrator')) {
+            abort(403);
         }
 
+        $user_id = $request->input('id', Auth::id());
+
         $user = User::find($user_id);
 
         $oldRole = $user->role;
 
         // Set role for User
-        $user->update([
-            'role' => $request->input('user_role'),
-        ]);
+        $user->role = $request->input('user_role');
+        $user->save();
 
         // If we are demoting from NetworkCoordinator, remove them from the list of coordinators for
         // any networks they are currently coordinating.
@@ -823,34 +819,30 @@ public function edit($id, Request $request)
             $Groups = new Group;
             $Groups = $Groups->findAll();
 
-            $data = $request->post();
-
             if (! Fixometer::hasRole($User->find($id), 'Administrator')) {
-                $sent_groups = $data['groups'];
+                $sent_groups = $request->input('groups');
             }
 
+            $data = $request->only([
+                'name', 'email', 'location', 'age', 'gender', 'country_code',
+                'biography', 'language', 'newsletter', 'invites',
+            ]);
+
             $error = false;
             // check for email in use
             $editingUser = $User->find($id);
             if ($editingUser->email !== $data['email'] && ! $User->checkEmail($data['email'])) {
                 $error['email'] = 'The email you entered is already in use in our database. Please use another one.';
             }
 
-            if (! empty($data['new-password'])) {
-                if ($data['new-password'] !== $data['password-confirm']) {
+            if (! empty($request->input('new-password'))) {
+                if ($request->input('new-password') !== $request->input('password-confirm')) {
                     $error['password'] = 'The passwords are not identical!';
                 } else {
-                    $data['password'] = Hash::make($data['new-password']);
+                    $data['password'] = Hash::make($request->input('new-password'));
                 }
             }
 
-            unset($data['new-password']);
-            unset($data['password-confirm']);
-
-            unset($data['groups']);
-            unset($data['profile']);
-            unset($data['id']);
-
             if (! is_array($error)) {
                 $u = $User->find($id)->update($data);
 
@@ -1015,7 +1007,6 @@ public function postRegister(Request $request, $hash = null): RedirectResponse
                 'name' => $request->input('name'),
                 'email' => $request->input('email'),
                 'password' => Hash::make($request->input('password')),
-                'role' => $role,
                 'recovery' => substr(bin2hex(openssl_random_pseudo_bytes(32)), 0, 24),
                 'recovery_expires' => strftime('%Y-%m-%d %X', time() + (24 * 60 * 60)),
                 'country_code' => $request->input('country'),
@@ -1025,6 +1016,9 @@ public function postRegister(Request $request, $hash = null): RedirectResponse
                 'calendar_hash' => Str::random(15),
                 'username' => '',
             ]);
+
+            $user->role = $role;
+            $user->save();
         }
 
         $user->generateAndSetUsername();

app/Models/User.php

@@ -40,8 +40,11 @@ class User extends Authenticatable implements Auditable, HasLocalePreference
      *
      * @var array
      */
+    // Note: `role` and `api_token` are intentionally excluded from $fillable.
+    // They must be set via direct assignment ($user->role = X; $user->save())
+    // to prevent privilege escalation via mass assignment (security: C2/M1).
     protected $fillable = [
-        'name', 'email', 'password', 'role', 'recovery', 'recovery_expires', 'language', 'repair_network', 'location', 'age', 'gender', 'country_code', 'newsletter', 'invites', 'biography', 'consent_future_data', 'consent_past_data', 'consent_gdpr', 'number_of_logins', 'latitude', 'longitude', 'last_login_at', 'api_token', 'access_group_tag_id', 'calendar_hash', 'repairdir_role', 'mediawiki', 'username', 'external_user_id', 'external_username',
+        'name', 'email', 'password', 'recovery', 'recovery_expires', 'language', 'repair_network', 'location', 'age', 'gender', 'country_code', 'newsletter', 'invites', 'biography', 'consent_future_data', 'consent_past_data', 'consent_gdpr', 'number_of_logins', 'latitude', 'longitude', 'last_login_at', 'access_group_tag_id', 'calendar_hash', 'repairdir_role', 'mediawiki', 'username', 'external_user_id', 'external_username',
     ];
 
     /**

tests/Feature/Users/PrivilegeEscalationTest.php

@@ -0,0 +1,179 @@
+<?php
+
+namespace Tests\Feature;
+
+use App\Models\Role;
+use App\Models\User;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Tests\TestCase;
+
+class PrivilegeEscalationTest extends TestCase
+{
+    use RefreshDatabase;
+
+    /**
+     * C1: Restarter cannot edit another user's profile info.
+     */
+    public function test_restarter_cannot_edit_another_users_info(): void
+    {
+        $attacker = User::factory()->restarter()->create();
+        $victim = User::factory()->restarter()->create();
+
+        $this->actingAs($attacker);
+
+        $response = $this->post('/profile/edit-info', [
+            'id' => $victim->id,
+            'email' => $victim->email,
+            'name' => 'Hacked Name',
+            'country' => $victim->country_code,
+            'townCity' => $victim->location,
+            'age' => $victim->age,
+            'gender' => $victim->gender,
+            'biography' => 'hacked',
+        ]);
+
+        $response->assertStatus(403);
+        $this->assertNotEquals('Hacked Name', $victim->fresh()->name);
+    }
+
+    /**
+     * C1: Restarter cannot change another user's password.
+     */
+    public function test_restarter_cannot_change_another_users_password(): void
+    {
+        $attacker = User::factory()->restarter()->create();
+        $victim = User::factory()->restarter()->create();
+
+        $this->actingAs($attacker);
+
+        $response = $this->post('/profile/edit-password', [
+            'id' => $victim->id,
+            'current-password' => 'secret',
+            'new-password' => 'hacked123',
+            'new-password-repeat' => 'hacked123',
+        ]);
+
+        $response->assertStatus(403);
+    }
+
+    /**
+     * C1: Restarter cannot change another user's photo.
+     */
+    public function test_restarter_cannot_change_another_users_photo(): void
+    {
+        $attacker = User::factory()->restarter()->create();
+        $victim = User::factory()->restarter()->create();
+
+        $this->actingAs($attacker);
+
+        $response = $this->post('/profile/edit-photo', [
+            'id' => $victim->id,
+        ]);
+
+        $response->assertStatus(403);
+    }
+
+    /**
+     * C1: Restarter cannot access admin edit settings.
+     */
+    public function test_restarter_cannot_access_admin_edit_settings(): void
+    {
+        $attacker = User::factory()->restarter()->create();
+
+        $this->actingAs($attacker);
+
+        $response = $this->post('/profile/edit-admin-settings', [
+            'id' => $attacker->id,
+            'user_role' => Role::ROOT,
+        ]);
+
+        $response->assertStatus(403);
+        $this->assertEquals(Role::RESTARTER, $attacker->fresh()->role);
+    }
+
+    /**
+     * C1 positive: Admin CAN change role via admin edit settings.
+     */
+    public function test_admin_can_change_role_via_admin_edit(): void
+    {
+        $admin = User::factory()->administrator()->create();
+        $user = User::factory()->restarter()->create();
+
+        $this->actingAs($admin);
+
+        $response = $this->post('/profile/edit-admin-settings', [
+            'id' => $user->id,
+            'user_role' => Role::HOST,
+            'assigned_groups' => [],
+            'preferences' => [],
+            'permissions' => [],
+        ]);
+
+        $response->assertSessionHas('message');
+        $this->assertEquals(Role::HOST, $user->fresh()->role);
+    }
+
+    /**
+     * C1 positive: Admin CAN edit another user's info.
+     */
+    public function test_admin_can_edit_another_users_info(): void
+    {
+        $admin = User::factory()->administrator()->create();
+        $user = User::factory()->restarter()->create();
+
+        $this->actingAs($admin);
+
+        $response = $this->post('/profile/edit-info', [
+            'id' => $user->id,
+            'email' => $user->email,
+            'name' => 'Updated By Admin',
+            'country' => $user->country_code,
+            'townCity' => $user->location,
+            'age' => $user->age,
+            'gender' => $user->gender,
+            'biography' => $user->biography,
+        ]);
+
+        $response->assertRedirect();
+        $this->assertEquals('Updated By Admin', $user->fresh()->name);
+    }
+
+    /**
+     * C2/M1: User cannot escalate role via /user/edit endpoint.
+     */
+    public function test_user_cannot_escalate_role_via_edit_endpoint(): void
+    {
+        $admin = User::factory()->administrator()->create();
+        $user = User::factory()->restarter()->create();
+
+        $this->actingAs($admin);
+
+        $response = $this->post("/user/edit/{$user->id}", [
+            'name' => $user->name,
+            'email' => $user->email,
+            'role' => Role::ROOT,
+        ]);
+
+        $this->assertNotEquals(Role::ROOT, $user->fresh()->role);
+    }
+
+    /**
+     * C2/M1: User cannot overwrite api_token via /user/edit endpoint.
+     */
+    public function test_user_cannot_overwrite_api_token_via_edit_endpoint(): void
+    {
+        $admin = User::factory()->administrator()->create();
+        $user = User::factory()->restarter()->create();
+        $originalToken = $user->api_token;
+
+        $this->actingAs($admin);
+
+        $response = $this->post("/user/edit/{$user->id}", [
+            'name' => $user->name,
+            'email' => $user->email,
+            'api_token' => 'stolen_token_value',
+        ]);
+
+        $this->assertEquals($originalToken, $user->fresh()->api_token);
+    }
+}