Skip to content

Commit 15b77b1

Browse files
xabbuhxabbuh
committed
prevent hash collisions caused by reused object hashes
1 parent c175d67 commit 15b77b1

File tree

5 files changed

+60
-18
lines changed

5 files changed

+60
-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
}
@@ -133,18 +130,15 @@ public function validate($form, Constraint $formConstraint)
133130
foreach ($form->all() as $field) {
134131
if ($field->isSubmitted()) {
135132
$this->resolvedGroups[$field] = $groups;
136-
$fieldFormConstraint = new Form();
137-
$this->fieldFormConstraints[] = $fieldFormConstraint;
138133
$this->context->setNode($this->context->getValue(), $field, $this->context->getMetadata(), $this->context->getPropertyPath());
139-
$validator->atPath(sprintf('children[%s]', $field->getName()))->validate($field, $fieldFormConstraint);
134+
$validator->atPath(sprintf('children[%s]', $field->getName()))->validate($field, $formConstraint);
140135
}
141136
}
142137
}
143138

144139
if ($hasChildren && $form->isRoot()) {
145140
// destroy storage to avoid memory leaks
146141
$this->resolvedGroups = new \SplObjectStorage();
147-
$this->fieldFormConstraints = [];
148142
}
149143
} elseif (!$form->isSynchronized()) {
150144
$childrenSynchronized = true;
@@ -153,11 +147,8 @@ public function validate($form, Constraint $formConstraint)
153147
foreach ($form as $child) {
154148
if (!$child->isSynchronized()) {
155149
$childrenSynchronized = false;
156-
157-
$fieldFormConstraint = new Form();
158-
$this->fieldFormConstraints[] = $fieldFormConstraint;
159150
$this->context->setNode($this->context->getValue(), $child, $this->context->getMetadata(), $this->context->getPropertyPath());
160-
$validator->atPath(sprintf('children[%s]', $child->getName()))->validate($child, $fieldFormConstraint);
151+
$validator->atPath(sprintf('children[%s]', $child->getName()))->validate($child, $formConstraint);
161152
}
162153
}
163154

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
},
2727
"require-dev": {
2828
"doctrine/collections": "~1.0",
29-
"symfony/validator": "^3.4.44|^4.0.3",
29+
"symfony/validator": "^3.4.46|^4.4.15",
3030
"symfony/dependency-injection": "~3.3|~4.0",
3131
"symfony/config": "~2.7|~3.0|~4.0",
3232
"symfony/expression-language": "~3.4|~4.0",

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ class ExecutionContext implements ExecutionContextInterface
126126
* @var array
127127
*/
128128
private $initializedObjects;
129+
private $cachedObjectsRefs;
129130

130131
/**
131132
* Creates a new execution context.
@@ -148,6 +149,7 @@ public function __construct(ValidatorInterface $validator, $root, TranslatorInte
148149
$this->translator = $translator;
149150
$this->translationDomain = $translationDomain;
150151
$this->violations = new ConstraintViolationList();
152+
$this->cachedObjectsRefs = new \SplObjectStorage();
151153
}
152154

153155
/**
@@ -353,4 +355,20 @@ public function isObjectInitialized($cacheKey)
353355
{
354356
return isset($this->initializedObjects[$cacheKey]);
355357
}
358+
359+
/**
360+
* @internal
361+
*
362+
* @param object $object
363+
*
364+
* @return string
365+
*/
366+
public function generateCacheKey($object)
367+
{
368+
if (!isset($this->cachedObjectsRefs[$object])) {
369+
$this->cachedObjectsRefs[$object] = spl_object_hash($object);
370+
}
371+
372+
return $this->cachedObjectsRefs[$object];
373+
}
356374
}

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,25 @@ public function testValidatedConstraintsHashesDoNotCollide()
218218

219219
$this->assertCount(2, $this->validator->validate($entity, new TestConstraintHashesDoNotCollide()));
220220
}
221+
222+
public function testValidatedConstraintsHashesDoNotCollideWithSameConstraintValidatingDifferentProperties()
223+
{
224+
$value = new \stdClass();
225+
226+
$entity = new Entity();
227+
$entity->firstName = $value;
228+
$entity->setLastName($value);
229+
230+
$validator = $this->validator->startContext($entity);
231+
232+
$constraint = new IsNull();
233+
$validator->atPath('firstName')
234+
->validate($entity->firstName, $constraint);
235+
$validator->atPath('lastName')
236+
->validate($entity->getLastName(), $constraint);
237+
238+
$this->assertCount(2, $validator->getViolations());
239+
}
221240
}
222241

223242
final class TestConstraintHashesDoNotCollide extends Constraint

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

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public function validate($value, $constraints = null, $groups = null)
113113
$this->validateGenericNode(
114114
$value,
115115
$previousObject,
116-
\is_object($value) ? spl_object_hash($value) : null,
116+
\is_object($value) ? $this->generateCacheKey($value) : null,
117117
$metadata,
118118
$this->defaultPropertyPath,
119119
$groups,
@@ -181,7 +181,7 @@ public function validateProperty($object, $propertyName, $groups = null)
181181

182182
$propertyMetadatas = $classMetadata->getPropertyMetadata($propertyName);
183183
$groups = $groups ? $this->normalizeGroups($groups) : $this->defaultGroups;
184-
$cacheKey = spl_object_hash($object);
184+
$cacheKey = $this->generateCacheKey($object);
185185
$propertyPath = PropertyPath::append($this->defaultPropertyPath, $propertyName);
186186

187187
$previousValue = $this->context->getValue();
@@ -229,7 +229,7 @@ public function validatePropertyValue($objectOrClass, $propertyName, $value, $gr
229229
if (\is_object($objectOrClass)) {
230230
$object = $objectOrClass;
231231
$class = \get_class($object);
232-
$cacheKey = spl_object_hash($objectOrClass);
232+
$cacheKey = $this->generateCacheKey($objectOrClass);
233233
$propertyPath = PropertyPath::append($this->defaultPropertyPath, $propertyName);
234234
} else {
235235
// $objectOrClass contains a class name
@@ -323,7 +323,7 @@ private function validateObject($object, $propertyPath, array $groups, $traversa
323323

324324
$this->validateClassNode(
325325
$object,
326-
spl_object_hash($object),
326+
$this->generateCacheKey($object),
327327
$classMetadata,
328328
$propertyPath,
329329
$groups,
@@ -458,7 +458,7 @@ private function validateClassNode($object, $cacheKey, ClassMetadataInterface $m
458458
$defaultOverridden = false;
459459

460460
// Use the object hash for group sequences
461-
$groupHash = \is_object($group) ? spl_object_hash($group) : $group;
461+
$groupHash = \is_object($group) ? $this->generateCacheKey($group) : $group;
462462

463463
if ($context->isGroupValidated($cacheKey, $groupHash)) {
464464
// Skip this group when validating the properties and when
@@ -802,7 +802,7 @@ private function validateInGroup($value, $cacheKey, MetadataInterface $metadata,
802802
// Prevent duplicate validation of constraints, in the case
803803
// that constraints belong to multiple validated groups
804804
if (null !== $cacheKey) {
805-
$constraintHash = spl_object_hash($constraint);
805+
$constraintHash = $this->generateCacheKey($constraint);
806806
// instanceof Valid: In case of using a Valid constraint with many groups
807807
// it makes a reference object get validated by each group
808808
if ($constraint instanceof Composite || $constraint instanceof Valid) {
@@ -828,4 +828,18 @@ private function validateInGroup($value, $cacheKey, MetadataInterface $metadata,
828828
$validator->validate($value, $constraint);
829829
}
830830
}
831+
832+
/**
833+
* @param object $object
834+
*
835+
* @return string
836+
*/
837+
private function generateCacheKey($object)
838+
{
839+
if ($this->context instanceof ExecutionContext) {
840+
return $this->context->generateCacheKey($object).$this->context->getPropertyPath();
841+
}
842+
843+
return spl_object_hash($object).$this->context->getPropertyPath();
844+
}
831845
}

0 commit comments

Comments
 (0)