Originally reported to the Drupal security team by
@bradjones1 →
on 23 August 2022 (#177392). Assuming 1) this is related to the JSON:API component, and 2) it affects the latest version
---
(Original post)
I wrote up a "typical" report below, but per some initial conversations with @bbrala (JSON:API maintainer) and @neclimdul on Slack (responding from a PM request on #security-team) it sounds like this needs some mulling over.
I think this could either be:
- "Just" a bug/oversight, fixed in public as an add-on to
https://www.drupal.org/project/drupal/issues/3304654 →
- Considered to be a security issue in JSON:API as it performs entity validation but in a way that doesn't provide an environment consistent with "typical" entity validation, which would be performed on save;
- Reveals a bit of an inconsistent entity validation API and this just "is what it is" or is a prompt for a more broad cleanup/clarification in core. Thanks to json:api doing more entity validation than most places in core, I've started to do mandatory validation way more often, and have discovered that calling $entity->validate() prior to save, as is the case here, can be frought with some gotchas. I suppose if you squint and consider this a big deal, it could be a more general core vulnerability.
The question comes down to, what really is a vulnerability? Bjorn and I chewed on this a bit and there are sparing few examples in core of validators that do require the original entity. The only one I could find of note is in the translation API, and if the original isn't there, it just takes care of loading it, within the validator. This is a duplication of logic, but right now there's no contract on the part of entity validation that says "you will get an original entity if this is not new."
I'd be happy for this to not be a security issue, but it smells stinky enough I figured I'd ask.
---
Drupal does not consistently provide a fully-initialized context for validating entities and their fields.
You can see this vulnerability by:
1. Enabling json:api module
2. As a user with permission to edit fields on a content entity, send a PATCH request for an entity.
3. Given the field has a validation constraint that requires the presence of the original/unchanged entity for operation, the validator will not have such context available.
4. Validate the entity by calling $entity->validate().
5. Any further validation that might otherwise be performed during save (with the original entity available) will not occur, as ContentEntityBase::validate() will have already set $entity->validated to TRUE.
Problem/Motivation
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Contributors
- bradjones1
- bbrala
- Wim Leers
- Berdir
- catch
Coordination:
- DamienMcKenna