8000 bug #38387 [Validator] prevent hash collisions caused by reused objec… · symfony/symfony@c72f853 · GitHub
[go: up one dir, main page]
Skip to content

Commit c72f853

Browse files
xabbuhxabbuh
committed
bug #38387 [Validator] prevent hash collisions caused by reused object hashes (fancyweb, xabbuh)
This PR was merged into the 4.4 branch. Discussion ---------- [Validator] prevent hash collisions caused by reused object hashes | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #36415 | License | MIT | Doc PR | Commits ------- 8dd1a6e prevent hash collisions caused by reused object hashes 9645fa3 [Validator][RecursiveContextualValidator] Prevent validated hash collisions
2 parents 3834d76 + 8dd1a6e commit c72f853

File tree

5 files changed

+111
-18
lines changed

5 files changed

+111
-18
lines changed

‎src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
class FormValidator extends ConstraintValidator
2626
{
2727
private $resolvedGroups;
28-
private $fieldFormConstraints;
2928

3029
/**
3130
* {@inheritdoc}
@@ -68,7 +67,6 @@ public function validate($form, Constraint $formConstraint)
6867

6968
if ($hasChildren && $form->isRoot()) {
7069
$this->resolvedGroups = new \SplObjectStorage();
71-
$this->fieldFormConstraints = [];
7270
}
7371

7472
if ($groups instanceof GroupSequence) {
@@ -93,7 +91,6 @@ public function validate($form, Constraint $formConstraint)
9391
$this->resolvedGroups[$field] = (array) $group;
9492
$fieldFormConstraint = new Form();
9593
$fieldFormConstraint->groups = $group;
96-
$this->fieldFormConstraints[] = $fieldFormConstraint;
9794
$this->context->setNode($this->context->getValue(), $field, $this->context->getMetadata(), $this->context->getPropertyPath());
9895
$validator->atPath(sprintf('children[%s]', $field->getName()))->validate($field, $fieldFormConstraint, $group);
9996
}
@@ -139,18 +136,15 @@ public function validate($form, Constraint $formConstraint)
139136
foreach ($form->all() as $field) {
140137
if ($field->isSubmitted()) {
141138
$this->resolvedGroups[$field] = $groups;
142-
$fieldFormConstraint = new Form();
143-
$this->fieldFormConstraints[] = $fieldFormConstraint;
144139
$this->context->setNode($this->context->getValue(), $field, $this->context->getMetadata(), $this->context->getPropertyPath());
145-
$validator->atPath(sprintf('children[%s]', $field->getName()))->validate($field, $fieldFormConstraint);
140+
$validator->atPath(sprintf('children[%s]', $field->getName()))->validate($field, $formConstraint);
146141
}
147142
}
148143
}
149144

150145
if ($hasChildren && $form->isRoot()) {
151146
// destroy storage to avoid memory leaks
152147
$this->resolvedGroups = new \SplObjectStorage();
153-
$this->fieldFormConstraints = [];
154148
}
155149
} elseif (!$form->isSynchronized()) {
156150
$childrenSynchronized = true;
@@ -159,11 +153,8 @@ public function validate($form, Constraint $formConstraint)
159153
foreach ($form as $child) {
160154
if (!$child->isSynchronized()) {
161155
$childrenSynchronized = false;
162-
163-
$fieldFormConstraint = new Form();
164-
$this->fieldFormConstraints[] = $fieldFormConstraint;
165156
$this->context->setNode($this->context->getValue(), $child, $this->context->getMetadata(), $this->context->getPropertyPath());
166-
$validator->atPath(sprintf('children[%s]', $child->getName()))->validate($child, $fieldFormConstraint);
157+
$validator->atPath(sprintf('children[%s]', $child->getName()))->validate($child, $formConstraint);
167158
}
168159
}
169160

‎src/Symfony/Component/Form/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
},
2828
"require-dev": {
2929
"doctrine/collections": "~1.0",
30-
"symfony/validator": "^3.4.44|^4.3.4|^5.0",
30+
"symfony/validator": "^4.4.17|^5.1.9",
3131
"symfony/dependency-injection": "^3.4|^4.0|^5.0",
3232
"symfony/expression-language": "^3.4|^4.0|^5.0",
3333
"symfony/config": "^3.4|^4.0|^5.0",

‎src/Symfony/Component/Validator/Context/ExecutionContext.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ class ExecutionContext implements ExecutionContextInterface
129129
* @var array
130130
*/
131131
private $initializedObjects;
132+
private $cachedObjectsRefs;
132133

133134
/**
134135
* Creates a new execution context.
@@ -153,6 +154,7 @@ public function __construct(ValidatorInterface $validator, $root, $translator, s
153154
$this->translator = $translator;
154155
$this->translationDomain = $translationDomain;
155156
$this->violations = new ConstraintViolationList();
157+
$this->cachedObjectsRefs = new \SplObjectStorage();
156158
}
157159

158160
/**
@@ -358,4 +360,20 @@ public function isObjectInitialized($cacheKey): bool
358360
{
359361
return isset($this->initializedObjects[$cacheKey]);
360362
}
363+
364+
/**
365+
* @internal
366+
*
367+
* @param object $object
368+
*
369+
* @return string
370+
*/
371+
public function generateCacheKey($object)
372+
{
373+
if (!isset($this->cachedObjectsRefs[$object])) {
374+
$this->cachedObjectsRefs[$object] = spl_object_hash($object);
375+
}
376+
377+
return $this->cachedObjectsRefs[$object];
378+
}
361379
}

‎src/Symfony/Component/Validator/Tests/Validator/RecursiveValidatorTest.php

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,14 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Translation\IdentityTranslator;
16+
use Symfony\Component\Validator\Constraint;
1617
use Symfony\Component\Validator\Constraints\All;
1718
use Symfony\Component\Validator\Constraints\Callback;
1819
use Symfony\Component\Validator\Constraints\Collection;
1920
use Symfony\Component\Validator\Constraints\Expression;
2021
use Symfony\Component\Validator\Constraints\GroupSequence;
22+
use Symfony\Component\Validator\Constraints\IsFalse;
23+
use Symfony\Component\Validator\Constraints\IsNull;
2124
use Symfony\Component\Validator\Constraints\IsTrue;
2225
use Symfony\Component\Validator\Constraints\Length;
2326
use Symfony\Component\Validator\Constraints\NotBlank;
@@ -26,6 +29,7 @@
2629
use Symfony\Component\Validator\Constraints\Required;
2730
use Symfony\Component\Validator\Constraints\Traverse;
2831
use Symfony\Component\Validator\Constraints\Valid;
32+
use Symfony\Component\Validator\ConstraintValidator;
2933
use Symfony\Component\Validator\ConstraintValidatorFactory;
3034
use Symfony\Component\Validator\ConstraintViolationInterface;
3135
use Symfony\Component\Validator\Context\ExecutionContextFactory;
@@ -2135,4 +2139,66 @@ public function testOptionalConstraintIsIgnored()
21352139

21362140
$this->assertCount(0, $violations);
21372141
}
2142+
2143+
public function testValidatedConstraintsHashesDoNotCollide()
2144+
{
2145+
$metadata = new ClassMetadata(Entity::class);
2146+
$metadata->addPropertyConstraint('initialized', new NotNull(['groups' => 'should_pass']));
2147+
$metadata->addPropertyConstraint('initialized', new IsNull(['groups' => 'should_fail']));
2148+
2149+
$this->metadataFactory->addMetadata($metadata);
2150+
2151+
$entity = new Entity();
2152+
$entity->data = new \stdClass();
2153+
2154+
$this->assertCount(2, $this->validator->validate($entity, new TestConstraintHashesDoNotCollide()));
2155+
}
2156+
2157+
public function testValidatedConstraintsHashesDoNotCollideWithSameConstraintValidatingDifferentProperties()
2158+
{
2159+
$value = new \stdClass();
2160+
2161+
$entity = new Entity();
2162+
$entity->firstName = $value;
2163+
$entity->setLastName($value);
2164+
2165+
$validator = $this->validator->startContext($entity);
2166+
2167+
$constraint = new IsNull();
2168+
$validator->atPath('firstName')
2169+
->validate($entity->firstName, $constraint);
2170+
$validator->atPath('lastName')
2171+
->validate($entity->getLastName(), $constraint);
2172+
2173+
$this->assertCount(2, $validator->getViolations());
2174+
}
2175+
}
2176+
2177+
final class TestConstraintHashesDoNotCollide extends Constraint
2178+
{
2179+
}
2180+
2181+
final class TestConstraintHashesDoNotCollideValidator extends ConstraintValidator
2182+
{
2183+
/**
2184+
* {@inheritdoc}
2185+
*/
2186+
public function validate($value, Constraint $constraint)
2187+
{
2188+
if (!$value instanceof Entity) {
2189+
throw new \LogicException();
2190+
}
2191+
2192+
$this->context->getValidator()
2193+
->inContext($this->context)
2194+
->atPath('data')
2195+
->validate($value, new NotNull())
2196+
->validate($value, new NotNull())
2197+
->validate($value, new IsFalse());
2198+
2199+
$this->context->getValidator()
2200+
->inContext($this->context)
2201+
->validate($value, null, new GroupSequence(['should_pass']))
2202+
->validate($value, null, new GroupSequence(['should_fail']));
2203+
}
21382204
}

‎src/Symfony/Component/Validator/Validator/RecursiveContextualValidator.php

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public function validate($value, $constraints = null, $groups = null)
108108
$this->validateGenericNode(
109109
$value,
110110
$previousObject,
111-
\is_object($value) ? spl_object_hash($value) : null,
111+
\is_object($value) ? $this->generateCacheKey($value) : null,
112112
$metadata,
113113
$this->defaultPropertyPath,
114114
$groups,
@@ -176,7 +176,7 @@ public function validateProperty($object, $propertyName, $groups = null)
176176

177177
$propertyMetadatas = $classMetadata->getPropertyMetadata($propertyName);
178178
$groups = $groups ? $this->normalizeGroups($groups) : $this->defaultGroups;
179-
$cacheKey = spl_object_hash($object);
179+
$cacheKey = $this->generateCacheKey($object);
180180
$propertyPath = PropertyPath::append($this->defaultPropertyPath, $propertyName);
181181

182182
$previousValue = $this->context->getValue();
@@ -224,7 +224,7 @@ public function validatePropertyValue($objectOrClass, $propertyName, $value, $gr
224224
if (\is_object($objectOrClass)) {
225225
$object = $objectOrClass;
226226
$class = \get_class($object);
227-
$cacheKey = spl_object_hash($objectOrClass);
227+
$cacheKey = $this->generateCacheKey($objectOrClass);
228228
$propertyPath = PropertyPath::append($this->defaultPropertyPath, $propertyName);
229229
} else {
230230
// $objectOrClass contains a class name
@@ -313,7 +313,7 @@ private function validateObject($object, string $propertyPath, array $groups, in
313313

314314
$this->validateClassNode(
315315
$object,
316-
spl_object_hash($object),
316+
$this->generateCacheKey($object),
317317
$classMetadata,
318318
$propertyPath,
319319
$groups,
@@ -429,7 +429,7 @@ private function validateClassNode($object, ?string $cacheKey, ClassMetadataInte
429429
$defaultOverridden = false;
430430

431431
// Use the object hash for group sequences
432-
$groupHash = \is_object($group) ? spl_object_hash($group) : $group;
432+
$groupHash = \is_object($group) ? $this->generateCacheKey($group, true) : $group;
433433

434434
if ($context->isGroupValidated($cacheKey, $groupHash)) {
435435
// Skip this group when validating the properties and when
@@ -740,7 +740,7 @@ private function validateInGroup($value, ?string $cacheKey, MetadataInterface $m
740740
// Prevent duplicate validation of constraints, in the case
741741
// that constraints belong to multiple validated groups
742742
if (null !== $cacheKey) {
743-
$constraintHash = spl_object_hash($constraint);
743+
$constraintHash = $this->generateCacheKey($constraint, true);
744744
// instanceof Valid: In case of using a Valid constraint with many groups
745745
// it makes a reference object get validated by each group
746746
if ($constraint instanceof Composite || $constraint instanceof Valid) {
@@ -772,4 +772,22 @@ private function validateInGroup($value, ?string $cacheKey, MetadataInterface $m
772772
}
773773
}
774774
}
775+
776+
/**
777+
* @param object $object
778+
*/
779+
private function generateCacheKey($object, bool $dependsOnPropertyPath = false): string
780+
{
781+
if ($this->context instanceof ExecutionContext) {
782+
$cacheKey = $this->context->generateCacheKey($object);
783+
} else {
784+
$cacheKey = spl_object_hash($object);
785+
}
786+
787+
if ($dependsOnPropertyPath) {
788+
$cacheKey .= $this->context->getPropertyPath();
789+
}
790+
791+
return $cacheKey;
792+
}
775793
}

0 commit comments

Comments
 (0)
0