Problem/Motivation

If the Drupal\Core\Site\Settings class in injected and stored on any object instance that is then serialized, such as a form. All of the settings data will also be serialized. This is problematic as you could have stale settings data stored in lots of place, as well as actually storing this data in other places besides settings.php.

Proposed resolution

Explore this, look into feasibility of custom serializable behaviour for this class?

Remaining tasks

Everything.

User interface changes

None

API changes

?

Comments

damiankloip’s picture

damiankloip commented
Title: Don't allow settings values from Settings being serialized » Don't allow settings values from Drupal\Core\Site\Settings to be serialized
sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Title: Don't allow settings values from Drupal\Core\Site\Settings to be serialized » Injected Settings may be serialized + unserialized later, not reflecting current settings.php values
Priority: Major » Critical
Issue tags: +Security
Related issues: +#2207585: Find a new OO home for drupal_get_hash_salt(), +#2199795: Make the Settings class prevent serialization of actual settings, +#2251113: Use container parameters instead of settings

This is a security issue.

Even if the answer is a formal policy change of "You have to rebuild the container AND flush all caches after changing settings.php."

olli’s picture

olli commented

Do we need something similar for $databases #2252033: Don't serialize database connection info?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

I think that #2199795: Make the Settings class prevent serialization of actual settings is proper fix for this. We need to prevent a serialisation of Settings using stale values and that issue achieves this.

dawehner’s picture

dawehner
Primary language German
commented

I wonder whether moving most of these parameters into the container causes an issue here? I would argue that the situation
is much better because you actually inject the "raw" value in there.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Closed (duplicate)
Issue tags: +Needs issue summary update

Marking duplicate of #2251795: Injected Settings may be serialized + unserialized later, not reflecting current settings.php values #2199795: Make the Settings class prevent serialization of actual settings.

I think container parameters are OK and less of a problem - it's obvious you have to do a container rebuild if you change a container parameter.