From 5f0f9b5053d27944197252a7b8b3fc90bb6d1c5a Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 29 Jun 2023 10:56:13 -0400 Subject: [PATCH] Fixing autocomplete security bug by using the query_builder --- .../Form/AutocompleteEntityTypeSubscriber.php | 27 ++++++++++--------- .../AutocompleteFormRenderingTest.php | 15 ++++++++++- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/Autocomplete/src/Form/AutocompleteEntityTypeSubscriber.php b/src/Autocomplete/src/Form/AutocompleteEntityTypeSubscriber.php index 4349dc3739a..3b774b91d2a 100644 --- a/src/Autocomplete/src/Form/AutocompleteEntityTypeSubscriber.php +++ b/src/Autocomplete/src/Form/AutocompleteEntityTypeSubscriber.php @@ -11,9 +11,7 @@ namespace Symfony\UX\Autocomplete\Form; -use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; -use Doctrine\ORM\Query\Parameter; use Doctrine\ORM\Utility\PersisterHelper; use Symfony\Bridge\Doctrine\Form\Type\EntityType; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -54,13 +52,15 @@ public function preSubmit(FormEvent $event) $form = $event->getForm(); $options = $form->get('autocomplete')->getConfig()->getOptions(); + /** @var EntityManagerInterface $em */ + $em = $options['em']; + $repository = $em->getRepository($options['class']); + $queryBuilder = $options['query_builder'] ?: $repository->createQueryBuilder('o'); + $rootAlias = $queryBuilder->getRootAliases()[0]; + if (!isset($data['autocomplete']) || '' === $data['autocomplete']) { $options['choices'] = []; } else { - /** @var EntityManagerInterface $em */ - $em = $options['em']; - $repository = $em->getRepository($options['class']); - $idField = $options['id_reader']->getIdField(); $idType = PersisterHelper::getTypeOfField($idField, $em->getClassMetadata($options['class']), $em)[0]; @@ -69,22 +69,23 @@ public function preSubmit(FormEvent $event) $idx = 0; foreach ($data['autocomplete'] as $id) { - $params[":id_$idx"] = new Parameter("id_$idx", $id, $idType); + $params[":id_$idx"] = [$id, $idType]; ++$idx; } - $queryBuilder = $repository->createQueryBuilder('o'); - if ($params) { $queryBuilder - ->where(sprintf("o.$idField IN (%s)", implode(', ', array_keys($params)))) - ->setParameters(new ArrayCollection($params)); + ->andWhere(sprintf("$rootAlias.$idField IN (%s)", implode(', ', array_keys($params)))) + ; + foreach ($params as $key => $param) { + $queryBuilder->setParameter($key, $param[0], $param[1]); + } } $options['choices'] = $queryBuilder->getQuery()->getResult(); } else { - $options['choices'] = $repository->createQueryBuilder('o') - ->where("o.$idField = :id") + $options['choices'] = $queryBuilder + ->andWhere("$rootAlias.$idField = :id") ->setParameter('id', $data['autocomplete'], $idType) ->getQuery() ->getResult(); diff --git a/src/Autocomplete/tests/Functional/AutocompleteFormRenderingTest.php b/src/Autocomplete/tests/Functional/AutocompleteFormRenderingTest.php index 0e8c2daa5f9..7cbef516e46 100644 --- a/src/Autocomplete/tests/Functional/AutocompleteFormRenderingTest.php +++ b/src/Autocomplete/tests/Functional/AutocompleteFormRenderingTest.php @@ -46,6 +46,7 @@ public function testCategoryFieldSubmitsCorrectly() $firstCat = CategoryFactory::createOne(['name' => 'First cat']); CategoryFactory::createOne(['name' => 'in space']); CategoryFactory::createOne(['name' => 'ate pizza']); + $fooCat = CategoryFactory::createOne(['name' => 'I contain "foo" which CategoryAutocompleteType uses its query_builder option.']); $this->browser() ->throwExceptions() @@ -53,14 +54,26 @@ public function testCategoryFieldSubmitsCorrectly() // the field renders empty (but the placeholder is there) ->assertElementCount('#product_category_autocomplete option', 1) ->assertNotContains('First cat') + ->post('/test-form', [ 'body' => [ 'product' => ['category' => ['autocomplete' => $firstCat->getId()]], ], ]) + // the option does NOT match something returned by query_builder + // so ONLY the placeholder shows up + ->assertElementCount('#product_category_autocomplete option', 1) + ->assertNotContains('First cat') + + ->assertNotContains('First cat') + ->post('/test-form', [ + 'body' => [ + 'product' => ['category' => ['autocomplete' => $fooCat->getId()]], + ], + ]) // the one option + placeholder now shows up ->assertElementCount('#product_category_autocomplete option', 2) - ->assertContains('First cat') + ->assertContains('which CategoryAutocompleteType uses') ; }