Security: Prevent room ID enumeration

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Security header Referrer-Policy ([#2519])
 - Docs: HTTP Strict Transport Security (HSTS) recommendations ([#2519])
 - Virus scan results to metrics ([#2304])
+- Rate limiting to prevent Room-ID enumeration attacks ([#2518])
 
 ### Changed
 
@@ -572,6 +573,7 @@ You can find the changelog for older versions there [here](https://github.com/TH
 [#2480]: https://github.com/THM-Health/PILOS/pull/2480
 [#2496]: https://github.com/THM-Health/PILOS/issues/2496
 [#2497]: https://github.com/THM-Health/PILOS/pull/2497
+[#2518]: https://github.com/THM-Health/PILOS/pull/2518
 [#2519]: https://github.com/THM-Health/PILOS/pull/2519
 [unreleased]: https://github.com/THM-Health/PILOS/compare/v4.7.1...develop
 [v3.0.0]: https://github.com/THM-Health/PILOS/releases/tag/v3.0.0

app/Providers/RouteServiceProvider.php

@@ -2,12 +2,14 @@
 
 namespace App\Providers;
 
+use App\Models\Room;
 use App\Models\User;
 use Illuminate\Cache\RateLimiting\Limit;
 use Illuminate\Foundation\Support\Providers\RouteServiceProvider as ServiceProvider;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\RateLimiter;
 use Illuminate\Support\Facades\Route;
+use Response;
 
 class RouteServiceProvider extends ServiceProvider
 {
@@ -70,5 +72,20 @@ protected function configureRateLimiting(): void
             // If the user is not editing himself, no rate limit (use the default rate limit, see api rate limit)
             return Limit::none();
         });
+
+        RateLimiter::for('room-enumeration', function (Request $request) {
+            return Limit::perMinute(10)
+                ->by($request->user()?->id ?: $request->ip())
+                ->after(function (\Symfony\Component\HttpFoundation\Response $response) use ($request) {
+                    // If the response is not a 404, do not count this request
+                    if ($response->getStatusCode() !== 404) {
+                        return false;
+                    }
+
+                    // Only count the request if the route parameter 'room' was not resolved to a Room model
+                    // Prevent counting requests that are valid and return a 404 for other reasons
+                    return ! ($request->route('room') instanceof Room);
+                });
+        });
     }
 }

routes/api.php

@@ -73,63 +73,67 @@
 
         Route::get('rooms', [RoomController::class, 'index'])->name('rooms.index');
         Route::post('rooms', [RoomController::class, 'store'])->name('rooms.store');
-        Route::post('rooms/{room}/favorites', [RoomController::class, 'addToFavorites'])->name('rooms.favorites.add');
-        Route::delete('rooms/{room}/favorites', [RoomController::class, 'deleteFromFavorites'])->name('rooms.favorites.delete');
-        Route::put('rooms/{room}', [RoomController::class, 'update'])->name('rooms.update');
-        Route::delete('rooms/{room}', [RoomController::class, 'destroy'])->name('rooms.destroy');
 
-        Route::get('rooms/{room}/settings', [RoomController::class, 'getSettings'])->name('rooms.settings');
+        Route::middleware('throttle:room-enumeration')->group(function () {
+            Route::post('rooms/{room}/favorites', [RoomController::class, 'addToFavorites'])->name('rooms.favorites.add');
+            Route::delete('rooms/{room}/favorites', [RoomController::class, 'deleteFromFavorites'])->name('rooms.favorites.delete');
+            Route::put('rooms/{room}', [RoomController::class, 'update'])->name('rooms.update');
+            Route::delete('rooms/{room}', [RoomController::class, 'destroy'])->name('rooms.destroy');
+
+            Route::get('rooms/{room}/settings', [RoomController::class, 'getSettings'])->name('rooms.settings');
+
+            Route::put('rooms/{room}/description', [RoomController::class, 'updateDescription'])->name('rooms.description.update')->middleware('can:update,room');
+
+            Route::post('rooms/{room}/transfer', [RoomController::class, 'transferOwnership'])->name('rooms.transfer')->middleware('can:transfer,room');
+
+            // Membership user self add/remove
+            Route::post('rooms/{room}/membership', [RoomMemberController::class, 'join'])->name('rooms.membership.join');
+            Route::delete('rooms/{room}/membership', [RoomMemberController::class, 'leave'])->name('rooms.membership.leave');
+
+            // Membership users for mass update & delete
+            Route::post('rooms/{room}/member/bulk', [RoomMemberController::class, 'bulkImport'])->name('rooms.member.bulkImport')->middleware('can:manageMembers,room');
+            Route::put('rooms/{room}/member/bulk', [RoomMemberController::class, 'bulkUpdate'])->name('rooms.member.bulkUpdate')->middleware('can:manageMembers,room');
+            Route::delete('rooms/{room}/member/bulk', [RoomMemberController::class, 'bulkDestroy'])->name('rooms.member.bulkDestroy')->middleware('can:manageMembers,room');
+
+            // Membership operations by room owner
+            Route::get('rooms/{room}/member', [RoomMemberController::class, 'index'])->name('rooms.member.get')->middleware('can:viewMembers,room');
+            Route::post('rooms/{room}/member', [RoomMemberController::class, 'store'])->name('rooms.member.add')->middleware('can:manageMembers,room');
+            Route::put('rooms/{room}/member/{user}', [RoomMemberController::class, 'update'])->name('rooms.member.update')->middleware('can:manageMembers,room');
+            Route::delete('rooms/{room}/member/{user}', [RoomMemberController::class, 'destroy'])->name('rooms.member.destroy')->middleware('can:manageMembers,room');
+
+            // Recording operations
+            Route::middleware('can:manageRecordings,room')->scopeBindings()->group(function () {
+                Route::put('rooms/{room}/recordings/{recording}', [RecordingController::class, 'update'])->name('rooms.recordings.update');
+                Route::delete('rooms/{room}/recordings/{recording}', [RecordingController::class, 'destroy'])->name('rooms.recordings.destroy');
+            });
+
+            // Streaming operations
+            Route::middleware('can:viewStreaming,room')->scopeBindings()->group(function () {
+                Route::get('rooms/{room}/streaming/config', [RoomStreamingController::class, 'getConfig'])->name('rooms.streaming.config.get');
+                Route::get('rooms/{room}/streaming/status', [RoomStreamingController::class, 'status'])->name('rooms.streaming.status');
+            });
+
+            Route::middleware('can:manageStreaming,room')->scopeBindings()->group(function () {
+                Route::put('rooms/{room}/streaming/config', [RoomStreamingController::class, 'updateConfig'])->name('rooms.streaming.config.update');
+                Route::post('rooms/{room}/streaming/start', [RoomStreamingController::class, 'start'])->name('rooms.streaming.start');
+                Route::post('rooms/{room}/streaming/stop', [RoomStreamingController::class, 'stop'])->name('rooms.streaming.stop');
+                Route::post('rooms/{room}/streaming/pause', [RoomStreamingController::class, 'pause'])->name('rooms.streaming.pause');
+                Route::post('rooms/{room}/streaming/resume', [RoomStreamingController::class, 'resume'])->name('rooms.streaming.resume');
+            });
+
+            // Personalized room tokens
+            Route::get('rooms/{room}/tokens', [RoomTokenController::class, 'index'])->name('rooms.tokens.get')->middleware('can:viewTokens,room');
+            Route::post('rooms/{room}/tokens', [RoomTokenController::class, 'store'])->name('rooms.tokens.add')->middleware('can:manageTokens,room');
+            Route::put('rooms/{room}/tokens/{token}', [RoomTokenController::class, 'update'])->name('rooms.tokens.update')->middleware('can:manageTokens,room');
+            Route::delete('rooms/{room}/tokens/{token}', [RoomTokenController::class, 'destroy'])->name('rooms.tokens.destroy')->middleware('can:manageTokens,room');
+
+            // File operations
+            Route::middleware('can:manageFiles,room')->scopeBindings()->group(function () {
+                Route::post('rooms/{room}/files', [RoomFileController::class, 'store'])->name('rooms.files.add');
+                Route::put('rooms/{room}/files/{file}', [RoomFileController::class, 'update'])->name('rooms.files.update');
+                Route::delete('rooms/{room}/files/{file}', [RoomFileController::class, 'destroy'])->name('rooms.files.destroy');
+            });
 
-        Route::put('rooms/{room}/description', [RoomController::class, 'updateDescription'])->name('rooms.description.update')->middleware('can:update,room');
-
-        Route::post('rooms/{room}/transfer', [RoomController::class, 'transferOwnership'])->name('rooms.transfer')->middleware('can:transfer,room');
-
-        // Membership user self add/remove
-        Route::post('rooms/{room}/membership', [RoomMemberController::class, 'join'])->name('rooms.membership.join');
-        Route::delete('rooms/{room}/membership', [RoomMemberController::class, 'leave'])->name('rooms.membership.leave');
-
-        // Membership users for mass update & delete
-        Route::post('rooms/{room}/member/bulk', [RoomMemberController::class, 'bulkImport'])->name('rooms.member.bulkImport')->middleware('can:manageMembers,room');
-        Route::put('rooms/{room}/member/bulk', [RoomMemberController::class, 'bulkUpdate'])->name('rooms.member.bulkUpdate')->middleware('can:manageMembers,room');
-        Route::delete('rooms/{room}/member/bulk', [RoomMemberController::class, 'bulkDestroy'])->name('rooms.member.bulkDestroy')->middleware('can:manageMembers,room');
-
-        // Membership operations by room owner
-        Route::get('rooms/{room}/member', [RoomMemberController::class, 'index'])->name('rooms.member.get')->middleware('can:viewMembers,room');
-        Route::post('rooms/{room}/member', [RoomMemberController::class, 'store'])->name('rooms.member.add')->middleware('can:manageMembers,room');
-        Route::put('rooms/{room}/member/{user}', [RoomMemberController::class, 'update'])->name('rooms.member.update')->middleware('can:manageMembers,room');
-        Route::delete('rooms/{room}/member/{user}', [RoomMemberController::class, 'destroy'])->name('rooms.member.destroy')->middleware('can:manageMembers,room');
-
-        // Recording operations
-        Route::middleware('can:manageRecordings,room')->scopeBindings()->group(function () {
-            Route::put('rooms/{room}/recordings/{recording}', [RecordingController::class, 'update'])->name('rooms.recordings.update');
-            Route::delete('rooms/{room}/recordings/{recording}', [RecordingController::class, 'destroy'])->name('rooms.recordings.destroy');
-        });
-
-        // Streaming operations
-        Route::middleware('can:viewStreaming,room')->scopeBindings()->group(function () {
-            Route::get('rooms/{room}/streaming/config', [RoomStreamingController::class, 'getConfig'])->name('rooms.streaming.config.get');
-            Route::get('rooms/{room}/streaming/status', [RoomStreamingController::class, 'status'])->name('rooms.streaming.status');
-        });
-
-        Route::middleware('can:manageStreaming,room')->scopeBindings()->group(function () {
-            Route::put('rooms/{room}/streaming/config', [RoomStreamingController::class, 'updateConfig'])->name('rooms.streaming.config.update');
-            Route::post('rooms/{room}/streaming/start', [RoomStreamingController::class, 'start'])->name('rooms.streaming.start');
-            Route::post('rooms/{room}/streaming/stop', [RoomStreamingController::class, 'stop'])->name('rooms.streaming.stop');
-            Route::post('rooms/{room}/streaming/pause', [RoomStreamingController::class, 'pause'])->name('rooms.streaming.pause');
-            Route::post('rooms/{room}/streaming/resume', [RoomStreamingController::class, 'resume'])->name('rooms.streaming.resume');
-        });
-
-        // Personalized room tokens
-        Route::get('rooms/{room}/tokens', [RoomTokenController::class, 'index'])->name('rooms.tokens.get')->middleware('can:viewTokens,room');
-        Route::post('rooms/{room}/tokens', [RoomTokenController::class, 'store'])->name('rooms.tokens.add')->middleware('can:manageTokens,room');
-        Route::put('rooms/{room}/tokens/{token}', [RoomTokenController::class, 'update'])->name('rooms.tokens.update')->middleware('can:manageTokens,room');
-        Route::delete('rooms/{room}/tokens/{token}', [RoomTokenController::class, 'destroy'])->name('rooms.tokens.destroy')->middleware('can:manageTokens,room');
-
-        // File operations
-        Route::middleware('can:manageFiles,room')->scopeBindings()->group(function () {
-            Route::post('rooms/{room}/files', [RoomFileController::class, 'store'])->name('rooms.files.add');
-            Route::put('rooms/{room}/files/{file}', [RoomFileController::class, 'update'])->name('rooms.files.update');
-            Route::delete('rooms/{room}/files/{file}', [RoomFileController::class, 'destroy'])->name('rooms.files.destroy');
         });
 
         Route::get('users/search', [UserController::class, 'search'])->name('users.search');
@@ -156,28 +160,31 @@
         Route::get('meetings/{meeting}/attendance', [MeetingController::class, 'attendance'])->name('meetings.attendance');
         Route::get('meetings/{meeting}/stats', [MeetingController::class, 'stats'])->name('meetings.stats');
         Route::get('meetings', [MeetingController::class, 'index'])->name('meetings.index');
-        Route::get('rooms/{room}/meetings', [RoomController::class, 'meetings'])->name('rooms.meetings');
+
+        Route::get('rooms/{room}/meetings', [RoomController::class, 'meetings'])->middleware('throttle:room-enumeration')->name('rooms.meetings');
 
         Route::get('getTimezones', function () {
             return response()->json(['data' => timezone_identifiers_list()]);
         });
     });
 
-    Route::get('rooms/{room}', [RoomController::class, 'show'])->name('rooms.show')->middleware('room.authenticate:true');
+    Route::middleware('throttle:room-enumeration')->group(function () {
+        Route::get('rooms/{room}', [RoomController::class, 'show'])->name('rooms.show')->middleware('room.authenticate:true');
 
-    Route::middleware('room.authenticate')->scopeBindings()->group(function () {
-        Route::options('rooms/{room}/start', [RoomController::class, 'getStartRequirements'])->name('rooms.start-requirements')->middleware('can:start,room');
-        Route::options('rooms/{room}/join', [RoomController::class, 'getJoinRequirements'])->name('rooms.join-requirements');
+        Route::middleware('room.authenticate')->scopeBindings()->group(function () {
+            Route::options('rooms/{room}/start', [RoomController::class, 'getStartRequirements'])->name('rooms.start-requirements')->middleware('can:start,room');
+            Route::options('rooms/{room}/join', [RoomController::class, 'getJoinRequirements'])->name('rooms.join-requirements');
 
-        Route::post('rooms/{room}/start', [RoomController::class, 'start'])->name('rooms.start')->middleware('can:start,room');
-        Route::post('rooms/{room}/join', [RoomController::class, 'join'])->name('rooms.join');
-        Route::get('rooms/{room}/files', [RoomFileController::class, 'index'])->name('rooms.files.get');
-        Route::get('rooms/{room}/files/{file}', [RoomFileController::class, 'show'])->name('rooms.files.show')->middleware('can:downloadFile,room,file');
-        Route::get('rooms/{room}/recordings', [RecordingController::class, 'index'])->name('rooms.recordings.index');
-        Route::get('rooms/{room}/recordings/{recording}/formats/{format}', [RecordingFormatController::class, 'show'])->name('rooms.recordings.formats.show')->middleware('can:viewRecordingFormat,room,format');
+            Route::post('rooms/{room}/start', [RoomController::class, 'start'])->name('rooms.start')->middleware('can:start,room');
+            Route::post('rooms/{room}/join', [RoomController::class, 'join'])->name('rooms.join');
+            Route::get('rooms/{room}/files', [RoomFileController::class, 'index'])->name('rooms.files.get');
+            Route::get('rooms/{room}/files/{file}', [RoomFileController::class, 'show'])->name('rooms.files.show')->middleware('can:downloadFile,room,file');
+            Route::get('rooms/{room}/recordings', [RecordingController::class, 'index'])->name('rooms.recordings.index');
+            Route::get('rooms/{room}/recordings/{recording}/formats/{format}', [RecordingFormatController::class, 'show'])->name('rooms.recordings.formats.show')->middleware('can:viewRecordingFormat,room,format');
 
-    });
+        });
 
+    });
     Route::get('meetings/{meeting}/endCallback', [MeetingController::class, 'endMeetingCallback'])->name('meetings.endcallback');
 });
 

tests/Backend/Feature/api/v1/Room/RoomTest.php

@@ -416,6 +416,59 @@ public function test_transfer_room()
             ->assertJsonValidationErrors(['user']);
     }
 
+    /**
+     * Test room enumeration attack prevention using a rate limit
+     */
+    public function test_room_404_rate_limit()
+    {
+        $room = Room::factory()->create();
+
+        // Guest trying to access non-existing rooms
+        for ($i = 0; $i < 10; $i++) {
+            $this->getJson(route('api.v1.rooms.show', ['room' => 999999]))
+                ->assertNotFound();
+        }
+        // Check route is now rate limited
+        $this->getJson(route('api.v1.rooms.show', ['room' => 999999]))
+            ->assertStatus(429);
+        // Check other room routes are also rate limited
+        $this->getJson(route('api.v1.rooms.files.show', ['room' => 999999, 'file' => 999999]))
+            ->assertStatus(429);
+
+        // User trying to access non-existing rooms
+        // The rate limit is per user or IP for unauthenticated users, so the user should have its own limit
+        $this->actingAs($room->owner);
+        for ($i = 0; $i < 10; $i++) {
+            $this->getJson(route('api.v1.rooms.show', ['room' => 999999]))
+                ->assertNotFound();
+        }
+        // Check route is now rate limited
+        $this->getJson(route('api.v1.rooms.show', ['room' => 999999]))
+            ->assertStatus(429);
+        // Check other room routes are also rate limited
+        $this->getJson(route('api.v1.rooms.files.show', ['room' => 999999, 'file' => 999999]))
+            ->assertStatus(429);
+
+        // Time travel 1 minute to reset rate limit
+        $this->travel(1)->minutes();
+        $this->getJson(route('api.v1.rooms.show', ['room' => 999999]))
+            ->assertNotFound();
+
+        $this->actingAsGuest();
+        $this->getJson(route('api.v1.rooms.show', ['room' => 999999]))
+            ->assertNotFound();
+
+        // Time travel 1 minute to reset rate limit
+        $this->travel(1)->minutes();
+
+        // Check calling routes for an existing room that also result in a 404
+        // due to other reasons than the room not existing are not so strictly rate limited
+        for ($i = 0; $i < 50; $i++) {
+            $this->actingAs($room->owner)->getJson(route('api.v1.rooms.files.show', ['room' => $room, 'file' => 999999]))
+                ->assertNotFound();
+        }
+    }
+
     /**
      * Test if guests can access room
      */