Bad JSON data breaks API and admin dashboard

[nikita-fuchs]

Hello,

It is possible to remotely break APIs if the content type has a JSON field and the create operation is permitted. This also breaks the admin UI and prevents administrators from removing the malicious record, accessing the DB directly is the only way to fix the incident.

Reproduction steps:

1. Create a datatype with a JSON field
2. Create a new record via direct API call or using the entity service

strapi.entityService.create(... { data: jsonField:{ "foo" }...

with a string (!) instead of an object for the JSON field.

This will lead to the string being stored directly in the DB table as strapi seemingly relies fully on the DB to perform validation. The problem is though that Postgres' JSONB type allows storing objects, strings, and other data. (see https://materialize.com/docs/sql/types/jsonb/#details )
Upon the call in step 2, the console will show a JSON parsing error, because Strapi is - for whatever reason - immediately reading the newly created record from the DB and expects the data to be a JSON object.

Now Strapi is defacto broken, because every time somebody calls the API endpoint that includes such a malformatted JSON record, the parsing fails and there is an error response.

If an admin tries to investigate the issue, he is hit with the same error as the record(s) cannot be read because of the parsing error.

I recommend implementing a 'safe mode' view for the admin panel, so users aren't stuck with a broken strapi instance if issues like these occur in the future.

Node.js version: v18.12.1
NPM version: v9.2.0
Strapi version: 4.5.6
Database: postgres
Operating system: OSX

[cpaczek]

issue is here: https://github.com/strapi/strapi/blob/main/packages/core/database/lib/fields/json.js

the issue is that the yup validation for the field doesn't require a JSON object and will accept a string and when strapi goes to pull that out of the database it will try and parse it and break everything. This could allow users to break the admin panel for anyone using a json field by just passing it a string.

[nikita-fuchs]

I don't know how the yup validators exactly work, but if all it needs is to check whether the passed data is actually an object, this should be doable pretty quickly, right?

[cpaczek]

Ya actually you would just need to validate the json in here: https://github.com/strapi/strapi/blob/main/packages/core/database/lib/fields/json.js also should probably do it on the yup end as this seems like its a last resort.

You can see an example here: https://github.com/strapi/strapi/blob/main/packages/core/database/lib/fields/number.js

[derrickmehaffy]

Marked as reproduced based on the evaluation of @cpaczek

[derrickmehaffy]

Notes:

Should only happen on PostgreSQL but possibly MySQL due to the native json functions in these databases.

[gu-stav]

@derrickmehaffy Do you think it makes sense to create a separate FE task based on that? I haven't tested the issue, but if the admin app crashes on the field value I'd also consider this is a bug.

[joshuaellis]

@derrickmehaffy Do you think it makes sense to create a separate FE task based on that? I haven't tested the issue, but if the admin app crashes on the field value I'd also consider this is a bug.

I'm not Derrick, but yes. I think the FE shouldn't break from a bad value in the CMS instead it probably needs to show a warning

[derrickmehaffy]

@derrickmehaffy Do you think it makes sense to create a separate FE task based on that? I haven't tested the issue, but if the admin app crashes on the field value I'd also consider this is a bug.

I'm not Derrick, but yes. I think the FE shouldn't break from a bad value in the CMS instead it probably needs to show a warning

Indeed I'd say that's a good idea