Skip to content

Conversation

@imhayatunnabi
Copy link

@imhayatunnabi imhayatunnabi commented Oct 13, 2025

This PR fixes a critical race condition vulnerability in the PermissionRegistrar class that affects applications running in concurrent environments (Laravel Octane, Swoole, RoadRunner, etc.).

Problem

The loadPermissions() method had a Time-of-Check to Time-of-Use (TOCTOU) race condition where multiple concurrent requests could simultaneously pass the if ($this->permissions) check before any of them loaded the permissions, causing:

  1. Cache Stampede - Multiple simultaneous expensive database queries
  2. Performance Degradation - Redundant operations under high load
  3. Memory Waste - Multiple copies of permission data in memory
  4. Database Overload - Excessive queries during cache refresh

Solution

Implemented a thread-safe double-checked locking pattern:

  • Fast path check: Quick return if permissions already loaded
  • Loading flag: $isLoadingPermissions prevents concurrent loading
  • Wait & retry: Competing threads wait 10ms and recheck
  • Double-check locking: Verify permissions not loaded by another thread
  • Exception safety: try-finally ensures flag is always released

Code Changes

File: src/PermissionRegistrar.php

  1. Added private bool $isLoadingPermissions = false; property
  2. Enhanced loadPermissions() with thread-safe implementation
  3. Updated clearPermissionsCollection() to reset the loading flag

@imhayatunnabi
Copy link
Author

@freekmurze can i get a review or merge update ?

// This protects against cache stampede and duplicate database queries
if ($this->isLoadingPermissions) {
// Another thread is loading, wait and retry
usleep(10000); // Wait 10ms
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid infinite loop, let's add a local $retries variable, and increment it here.
Perhaps a maximum of 10 retries?

+$retries = 0;
-if ($this->isLoadingPermissions) {
+if ($this->isLoadingPermissions && $retries < 10) {
    usleep(10000); // Wait 10ms
+   $retries++;
...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved and updated in latest commit @drbyte

@imhayatunnabi imhayatunnabi requested a review from drbyte October 24, 2025 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants