- Issue created by @BramDriesen
- 🇺🇸United States benjifisher Boston area
The title of this issue refers to "Field sql storage", and the proposed resolution mentions
Drupal\Core\Field\FieldItemInterface::schema()
. But 📌 Allow field types to control how properties are mapped to and from storage Needs work refers to\Drupal\Core\Entity\Sql\SqlContentEntityStorage
, and I think that is where the actual calls tounserialize()
are. - 🇮🇳India kalpanajaiswal
I'm still relatively new to contributing to core, but I’ve been trying to understand this issue and the potential risks around unserialize() and serialized field storage.
Please feel free to correct or improve them:1. Add a Warning in FieldItem Schema
Since 'serialize' => TRUE can be risky, we could document something like the following in FieldItemInterface::schema():When using 'serialize' => TRUE, the FieldItem class must ensure only safe values (such as arrays or scalars) are stored. Arbitrary PHP objects should not be serialized.
2. Use allowed_classes in unserialize()
Wherever unserialize() is used (for example, in SqlContentEntityStorage::mapFromStorageRecords()), it might be safer to include:unserialize($data, ['allowed_classes' => false]);
Alternatively, we could use a limited allowlist of known-safe classes to prevent PHP object injection vulnerabilities.
3. Deprecate Unrestricted unserialize() Usage
We could consider marking all usages of unserialize() without the allowed_classes parameter as @deprecated, and encourage the use of a safer wrapper function or utility.4. Add a PHPCS Rule
A PHPCS rule could help identify unsafe uses of unserialize() during development—similar to how we detect unsafe render arrays.5. Audit Use of 'serialize' => TRUE in Core and Contrib
It might be worth reviewing all field types in core and contrib that use 'serialize' => TRUE, to ensure they are not accepting object instances via setValue() or unsafe API data.