Connect CMS: Improper Authorization in the My Page Profile Update Feature Allows Modification of Arbitrary User Information

@@ -12,7 +12,6 @@
 use App\Plugins\Mypage\MypagePluginBase;
 use App\Rules\CustomValiLoginIdAndPasswordDoNotMatch;
 use App\Rules\CustomValiUserEmailUnique;
-use App\User;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Hash;
 use Illuminate\Support\Facades\Validator;
@@ -62,7 +61,6 @@ public function index($request, $id = null)
             'themes'                  => $request->themes,
             "function"                => __FUNCTION__,
             "plugin_name"             => "profile",
-            "id"                      => $user->id,
             "user"                    => $user,
             "users_columns"           => $users_columns,
             "users_columns_id_select" => $users_columns_id_select,
@@ -75,9 +73,11 @@ public function index($request, $id = null)
     /**
      * 更新
      */
-    public function update($request, $id)
+    public function update($request, $id = null)
     {
-        $user = User::where('id', $id)->first();
+        // URLのidではなく、ログインユーザー自身のみを更新対象にする。
+        $user = Auth::user();
+        $user_id = $user->id;
 
         // ユーザーのカラム
         $users_columns_all = UsersTool::getUsersColumns($user->columns_set_id);
@@ -100,11 +100,11 @@ public function update($request, $id)
                 $base_rules = ['required', 'string', 'max:255'];
                 $validator_array['column']['name'] = UsersTool::getDefaultColumnAdditionalRules($base_rules, $users_column);
             } elseif ($users_column->column_type == UserColumnType::login_id) {
-                $base_rules = ['required', 'max:255', Rule::unique('users', 'userid')->ignore($id)];
+                $base_rules = ['required', 'max:255', Rule::unique('users', 'userid')->ignore($user_id)];
                 $validator_array['column']['userid'] = UsersTool::getDefaultColumnAdditionalRules($base_rules, $users_column);
             } elseif ($users_column->column_type == UserColumnType::user_email) {
                 // $validator_array['column']['email'] = ['nullable', 'email', 'max:255', Rule::unique('users')->ignore($id)];
-                $base_rules = ['email', 'max:255', new CustomValiUserEmailUnique($request->columns_set_id, $id)];
+                $base_rules = ['email', 'max:255', new CustomValiUserEmailUnique($user->columns_set_id, $user_id)];
                 if ($users_column->required) {
                     array_unshift($base_rules, 'required');
                 } else {
@@ -146,7 +146,7 @@ function ($attribute, $value, $fail) {
                 // チェックしない
             } else {
                 // バリデータールールをセット
-                $validator_array = UsersTool::getValidatorRule($validator_array, $users_column, $user->columns_set_id, $id);
+                $validator_array = UsersTool::getValidatorRule($validator_array, $users_column, $user->columns_set_id, $user_id);
             }
         }
 

@@ -16,7 +16,7 @@
 {{-- 登録後メッセージ表示 --}}
 @include('plugins.common.flash_message')
 
-<form action="{{url('/')}}/mypage/profile/update/{{$id}}" class="form-horizontal" method="POST" name="form_profile">
+<form action="{{url('/')}}/mypage/profile/update" class="form-horizontal" method="POST" name="form_profile">
     {{ csrf_field() }}
 
     @foreach($users_columns as $column)

@@ -0,0 +1,80 @@
+<?php
+
+namespace Tests\Feature\Mypage;
+
+use App\User;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Tests\TestCase;
+
+/**
+ * @runTestsInSeparateProcesses
+ * @preserveGlobalState disabled
+ */
+class ProfileMypageUpdateTest extends TestCase
+{
+    use RefreshDatabase;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->seed();
+    }
+
+    /**
+     * URL引数のIDを指定しても、ログインユーザー以外のプロフィールは更新できない。
+     */
+    public function testProfileUpdatePathIdCannotUpdateAnotherUser(): void
+    {
+        $attacker = User::factory()->create([
+            'name' => 'attacker',
+            'userid' => 'attacker-userid',
+            'email' => 'attacker@example.com',
+            'columns_set_id' => 1,
+        ]);
+
+        $victim = User::factory()->create([
+            'name' => 'victim',
+            'userid' => 'victim-userid',
+            'email' => 'victim@example.com',
+            'columns_set_id' => 1,
+        ]);
+
+        $response = $this->actingAs($attacker)->post("/mypage/profile/update/{$victim->id}", [
+            'name' => $attacker->name,
+            'userid' => $attacker->userid,
+            'email' => 'attacker-updated@example.com',
+        ]);
+
+        $response->assertStatus(302);
+        $response->assertRedirect(url('/mypage/profile'));
+        $response->assertSessionHas('flash_message', '更新しました。');
+
+        $this->assertSame('attacker-updated@example.com', $attacker->fresh()->email);
+        $this->assertSame('victim@example.com', $victim->fresh()->email);
+    }
+
+    /**
+     * フォーム送信先（IDなし）でもログインユーザーのプロフィールを更新できる。
+     */
+    public function testProfileUpdateWithoutPathIdUpdatesLoggedInUser(): void
+    {
+        $user = User::factory()->create([
+            'name' => 'self-user',
+            'userid' => 'self-userid',
+            'email' => 'self@example.com',
+            'columns_set_id' => 1,
+        ]);
+
+        $response = $this->actingAs($user)->post('/mypage/profile/update', [
+            'name' => $user->name,
+            'userid' => $user->userid,
+            'email' => 'self-updated@example.com',
+        ]);
+
+        $response->assertStatus(302);
+        $response->assertRedirect(url('/mypage/profile'));
+        $response->assertSessionHas('flash_message', '更新しました。');
+
+        $this->assertSame('self-updated@example.com', $user->fresh()->email);
+    }
+}