bolt: [BUG] - cannot use anymore TextType to save php serialized data in DB

I was previously using a TextType field to save in database a serialized array of php basic objects.

I know there is a best alternative to do that, ie, using doctrine object type but in my case I’m using the bolt/members extension and there is a feature to store some meta values to accounts. Those meta values are quite generic and store in database using a TextType.

After investigate the code, it seems it’s not really a bolt/members regression but a bolt/bolt I guess, because since that commit : https://github.com/bolt/bolt/commit/71c6c171435a28e49ffa36ea94e676c7a4283c4d

The sanitizer is called on all (string) FieldTypeBase persist() method :

if ($this instanceof SanitiserAwareInterface && is_string($value)) {
            $isWysiwyg = $this instanceof WysiwygAwareInterface;
            $value = $this->getSanitiser()->sanitise($value, $isWysiwyg);
}

Whereas before that commit, it was called on the HtmlType fields …

The problem is : a serialized string like

a:1:{i:0;O:51:"Bolt\Extension\DHuteau\MembersAddon\Form\Entity\Car":3:{s:8:"*model";s:9:"911 turbo";s:8:"*brand";s:7:"Porsche";s:13:"*identifier";s:10:"1234-vf-44";}}

is trimmed to something like

a:1:{i:0;O:51:"Bolt\Extension\DHuteau\MembersAddon\Form\Entity\Car"

And of course … it fails silently when i try to “unserialize()” because it just returns “false”.

Details

Relevant Bolt Version: [ 3.2 | master ] but i think since : https://github.com/bolt/bolt/commit/71c6c171435a28e49ffa36ea94e676c7a4283c4d

About this issue

Original URL
State: closed
Created 8 years ago
Comments: 31 (31 by maintainers)

Commits related to this issue

Fixed: Don't sanitise 'text' and 'textarea' type fields. Fixes #5789 — committed to bolt/bolt by bobdenotter 8 years ago

Most upvoted comments

is the main reason we moved away from serialisation in Bolt itself, as it’s a security risk.

Yeah, but this is about people saving whatever they want in fields that are designed to cope with them … if the fields aren’t rendered for display, there is a slightly different set of rules.

BTW, the above example from @dadaxr is an entity, which could be JSON encoded and rebuilt anyway 😜

GwendolenLynch on Sep 20, 2016

I’ve been thinking it over for the last couple of hours. So let’s step back.

Problem:

Certain field types should be sanitised, e.g.
- HTML
- Markdown
Certain fields are potential risks if un/sanitised, e.g.
- Text
- Textarea

No. 2 is only a problem if the field is used in the render context, and those fields can contain data not intended for such purpose … also text is the default type, and the fallback. it is a data loss problem to sanitise when we do.

RFC:

Sanitise fields in category 1. as we do now, but delay category 2. until render and cache the results.

GwendolenLynch on Sep 21, 2016

Oh, and this is slightly off topic, but this:

O:51:"Bolt\Extension\DHuteau\MembersAddon\Form\Entity\Car"

is the main reason we moved away from serialisation in Bolt itself, as it’s a security risk. A potential cracker can insert their own object with a __wakeup method, and the code will get executed on deserialisation. See: https://securitycafe.ro/2015/01/05/understanding-php-object-injection/

bobdenotter on Sep 20, 2016

Hmm, if it’s a type: text it might as well be a Title or somesuch, and we absolutely want to sanitise that. Two possible solutions that come to mind:

Before sanitising, make a guess if the data is JSON or serialized (as opposed to ‘text with HTML’) and don’t sanitise if it is.
Add an optional sanitise: false option to the field configuration.

bobdenotter on Sep 20, 2016