[Help Needed] Prevent injection of invalid entity ids for autocomplete

[javiereguiluz]

Symfony published a security issue in the UX autocomplete field: https://symfony.com/blog/cve-2023-41336-symfony-ux-autocomplete-prevent-injection-of-invalid-entity-ids-for-autocomplete-fields

That Symfony field was originally inspired by our autocomplete field: https://github.com/EasyCorp/EasyAdminBundle/blob/4.x/src/Form/EventListener/CrudAutocompleteSubscriber.php

I'm checking the patch (https://github.com/symfony/ux/commit/55a716bc47947ff8b9bfda8395762f4d5be8a6ba) but I'm struggling to fully understand if we need to make changes too.

Thanks for your help!

[dwd-akira]

I tried in my application and the injection is possible.

I have an autocomplete list of products, only enabled product are visible (#5792). But with a chrome plugin (web developer) i can convert select elements to text inputs and I can put any product ID even an invalid one (hacker case).

When the save is made, the product is loaded but without using the querybuilder.

Besides the querybuilder is never used to fill the list (in autocomplete context), it is createIndexQueryBuilder that is used (#5792)

[dwd-akira]

BUT with the patch, if i disable a product, the old quotes (with the disable product) can no longer be saved. The patch should only apply if the field is changed.

I think this patch is important for front office but not for back office.

[eminjk]

I think this patch is important for front office but not for back office.

I agree with this statement.

We have a similar situation in Action where you can use $actions->remove() so that the link does not appear, but one can still forge it and call it. Only $actions->disable() makes it impossible to run the action at all.

In my opinion, it would be enough to add the appropriate information in the documentation so that developers are aware that when saving, these data are not filtered. If they do not trust their users, they must check it themselves. The "backend" operates by its own rules and the default action is allow, while on the frontend the default action is disable.

Technically, solving this problem does not seem difficult - all that needs to be done is to add a boolean parameter that would be taken into account when validating entities. The problem I see concerns data inconsistency.

If there is already data in the database contradicting the restrictions we set in the field configuration, what then? Such an entity will be unsavable until it passes validation. Or maybe it's okay that old data are inconsistent with the new rule and we just want new entities to be subject to these restrictions.

Taking all of this into account, there are too many possibilities that would need to be addressed and this boils down to the developer preparing their custom solution depending on their needs. They just need to be aware that this is not automatically done.