Synopsis

This has been discovered by tsala, marycooch and sarjona during the tests of our community sites upgrades to moodle 3.6. What we saw was that one user's digital maturity declaration during the signup (is GDPR minor / is not GDPR minor) affected other user's choice too, in their own other browser / session.

We were able to reproduce it with a test script (reproduction steps below) and trace it down to a bug in the cache_session cache loader. However, this bug does not expose when a default session store is used for MUC session caches. To reproduce and experience it, the site has to use an alternative store - such as redis in our case.

Steps to reproduce

The overall idea of reproducing is: (1) Set up your Moodle to use Redis as a store for MUC session caches. (2) Set a value in the session cache for one guest / anonymous user's session (3) As a different guest / anonymous user, check that the value set by the first user, is not present in your session cache.

1. Configure your Moodle site to use a Redis instance as a default store for session mode caches:
   1. Go to Site administration > Plugins > Caching > Configuration
   2. Scroll down to the section "Stores used when no mapping is present"
   3. Click the "Edit mappings" links
   4. For the "Session" caches, select the Redis store to be used.
2. Put the attached script tsess.php to the root of the site
3. Go to the site main page and either stay logged off, or log in as a guest
4. Visit the tsess.php script by typing its URL to your browser
5. The script will report that "flag has not been not set" and will set it to a new value matching your current PHP session id.
6. From now on, when reloading the script, you should see "flag found" and the value set during the first visit.
7. Open a different browser or an incognito window and go to the site front page
8. Again make sure that you are either logged off, or logged in as a guest - same as in the first case
9. Again visit the tsess.php script by typing its URL to your browser

• Expected behaviour: You must be considered as a different anonymous / guest user with your own session. The tsess.php must again report "flag has not been not set" and set it to a new value
• Actual behaviour: The script reports "flag found" even during the first visit and shows the other user's session id

Reason

I was able to trace the bug down to the cache_session cache loader. It is not a bug in the redis cache store, but it exposes with redis for a reason.

To understand what's going on here, one has to know that when we have a MUC session cache instance $cache, then calling

$cache->set($key, $data);

actually executes something like (pseudocoded):

$cache->get_store()->set($lowlevelkey, $data);

where the $lowlevelkey is a calculated value that makes the given $key unique for each user session. The data are then stored in the associated cache store using this lowlevelkey - which may look like

u0_p3icg24cl0vrhei4h01tqnnv4s_6f4542e197848cf52cfac9187b7154425c067cae

where "u0" indicates the user id (here it is not logged in user id=0, guest users would have typically "u1" and so on), followed by the PHP session identifier "p3icg24cl0vrhei4h01tqnnv4s" and finally followed by an SHA1 hash based on the original $key and the cache definition.

The problem is, because of the bug, that the second part (session id) is not always set. So the low level key can be calculated as if the session id was null and the value would be like

u0__6f4542e197848cf52cfac9187b7154425c067cae

and so two different guests would use the same key.

Now, this does not usually expose as a problem if the PHP's $_SESSION is used as the storage for MUC session caches. Because each user will have its own PHP session implicitly.

But if an alternative store such as redis is used, all users use the same storage. Which makes the "session id" part of the low level key mandatory and because it is missing, two different guest browsers end up with using the same low level key.

Solution

The fix for this is quite straightforward. We need to fix the cache_session class so that it always takes the current session_id() into account.