From 4271cf802283cd7dd7d72f3295e82c0c62eac5d1 Mon Sep 17 00:00:00 2001 From: Thibaut Cholley Date: Fri, 19 Jun 2026 09:36:51 +0200 Subject: [PATCH 1/2] fix: resolve union in types --- .../TypeConfusion/UnionCollectionTarget.php | 31 +++++ .../TypeConfusionUnionCollectionTest.php | 111 ++++++++++++++++++ 2 files changed, 142 insertions(+) create mode 100644 tests/Fixtures/TestBundle/ApiResource/TypeConfusion/UnionCollectionTarget.php create mode 100644 tests/Functional/Security/TypeConfusionUnionCollectionTest.php diff --git a/tests/Fixtures/TestBundle/ApiResource/TypeConfusion/UnionCollectionTarget.php b/tests/Fixtures/TestBundle/ApiResource/TypeConfusion/UnionCollectionTarget.php new file mode 100644 index 00000000000..1af02cc6dae --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/TypeConfusion/UnionCollectionTarget.php @@ -0,0 +1,31 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\TypeConfusion; + +use ApiPlatform\Metadata\Post; + +#[Post(uriTemplate: '/type-confusion/union-collection-targets{._format}')] +class UnionCollectionTarget +{ + public ?string $name = null; + + /** + * PHPDoc-only union array: each element may be either Foo or Bar. + * Mirrors the real-world "@var PharmacyRevenueFile[]|MerchFile[]" pattern + * that regressed under the GHSA-9rjg-x2p2-h68h security fix. + * + * @var Foo[]|Bar[] + */ + public array $attachments = []; +} \ No newline at end of file diff --git a/tests/Functional/Security/TypeConfusionUnionCollectionTest.php b/tests/Functional/Security/TypeConfusionUnionCollectionTest.php new file mode 100644 index 00000000000..29fed777925 --- /dev/null +++ b/tests/Functional/Security/TypeConfusionUnionCollectionTest.php @@ -0,0 +1,111 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Tests\Functional\Security; + +use ApiPlatform\Symfony\Bundle\Test\ApiTestCase; +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\TypeConfusion\Bar; +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\TypeConfusion\Foo; +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\TypeConfusion\UnionCollectionTarget; +use ApiPlatform\Tests\SetupClassResourcesTrait; + +/** + * Regression test for the side-effect of the GHSA-9rjg-x2p2-h68h security fix. + * + * The fix added an is_a guard in AbstractItemNormalizer::getResourceFromIri() to prevent + * CWE-843 type confusion. However, for a property typed as a union of collections + * (e.g. "@var Foo[]|Bar[]"), the denormalization loop iterates over each possible type + * one at a time (first tries Foo[], then Bar[]). The is_a check rejects any item that + * does not match the single type being currently tried, so a mixed [Foo, Bar] collection + * always fails: Foo[] fails on the Bar item, Bar[] fails on the Foo item. + * + * Before the fix, both iterations returned successfully (no type guard), so mixed + * collections were accepted and Symfony's validator would catch genuinely wrong types + * at the individual item path (e.g. "attachments[2]: must be Foo|Bar"). + * + * After the fix, the per-item NotNormalizableValueException bubbles through the union + * fallback logic and is re-wrapped at the collection level, producing a less informative + * error ("attachments: must be array|array") and — worse — rejecting + * previously valid mixed-type collections entirely. + */ +final class TypeConfusionUnionCollectionTest extends ApiTestCase +{ + use SetupClassResourcesTrait; + + protected static ?bool $alwaysBootKernel = false; + + /** + * @return class-string[] + */ + public static function getResources(): array + { + return [Foo::class, Bar::class, UnionCollectionTarget::class]; + } + + public function testHomogeneousFooCollectionIsAccepted(): void + { + $response = self::createClient()->request('POST', '/type-confusion/union-collection-targets', [ + 'headers' => ['Content-Type' => 'application/ld+json', 'Accept' => 'application/ld+json'], + 'json' => [ + 'name' => 'all-foos', + 'attachments' => ['/type-confusion/foos/1', '/type-confusion/foos/2'], + ], + ]); + + $this->assertResponseStatusCodeSame(201); + } + + public function testHomogeneousBarCollectionIsAccepted(): void + { + $response = self::createClient()->request('POST', '/type-confusion/union-collection-targets', [ + 'headers' => ['Content-Type' => 'application/ld+json', 'Accept' => 'application/ld+json'], + 'json' => [ + 'name' => 'all-bars', + 'attachments' => ['/type-confusion/bars/1', '/type-confusion/bars/2'], + ], + ]); + + $this->assertResponseStatusCodeSame(201); + } + + /** + * A @var Foo[]|Bar[] collection must accept a mix of Foo and Bar IRIs. + * + * This test demonstrates the regression introduced by GHSA-9rjg-x2p2-h68h: + * the is_a guard in getResourceFromIri() causes AbstractItemNormalizer to reject + * any mixed [Foo, Bar] payload because neither single-type iteration can satisfy + * all items, even though each individual item is a valid union member. + */ + public function testMixedUnionCollectionIsAccepted(): void + { + $response = self::createClient()->request('POST', '/type-confusion/union-collection-targets', [ + 'headers' => ['Content-Type' => 'application/ld+json', 'Accept' => 'application/ld+json'], + 'json' => [ + 'name' => 'mixed', + 'attachments' => [ + '/type-confusion/foos/1', + '/type-confusion/foos/2', + '/type-confusion/bars/1', + ], + ], + ]); + + $this->assertResponseStatusCodeSame( + 201, + \sprintf( + 'Regression from GHSA-9rjg-x2p2-h68h: mixed Foo[]|Bar[] collection was rejected. Body: %s', + $response->getContent(false) + ) + ); + } +} \ No newline at end of file From 817f45ec5f831654d92379f615b70d441f67023b Mon Sep 17 00:00:00 2001 From: Thibaut Cholley Date: Fri, 19 Jun 2026 10:13:29 +0200 Subject: [PATCH 2/2] wip --- src/Serializer/AbstractItemNormalizer.php | 40 +++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index 75361cecea0..05ef2118094 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -764,8 +764,12 @@ private function getResourceFromIri(string $data, array $context, string $resour try { $item = $this->iriConverter->getResourceFromIri($data, $context + ['fetch_data' => true]); - // Type-confusion guard: declared relation class must match the IRI's resource. - if (!is_a($item, $resourceClass)) { + // Type-confusion guard (CWE-843 / GHSA-9rjg-x2p2-h68h): the resolved + // resource must be an instance of the declared class. For union-typed + // properties (e.g. Foo[]|Bar[]) every resource class in the union is + // accepted; a genuinely foreign type is still rejected. + $allowedClasses = $context['_api_union_resource_classes'] ?? [$resourceClass]; + if (!\array_reduce($allowedClasses, static fn (bool $ok, string $cls): bool => $ok || \is_a($item, $cls), false)) { throw new NotNormalizableValueException(\sprintf('The iri "%s" does not reference the correct resource.', $data)); } @@ -1191,6 +1195,38 @@ private function createAndValidateAttributeValue(string $attribute, mixed $value $isMultipleTypes = \count($types) > 1; $denormalizationException = null; + // Build the full set of resource classes across all union types so that + // getResourceFromIri()'s is_a guard can accept any valid union member. + // Without this, a Foo[]|Bar[] property rejects mixed collections because + // the guard only sees the single class being tried in each iteration. + unset($context['_api_union_resource_classes']); + if ($isMultipleTypes) { + $unionResourceClasses = []; + foreach ($types as $ut) { + if ($ut instanceof CollectionType) { + $cvt = $ut->getCollectionValueType(); + if ($cvt instanceof ObjectType && $this->resourceClassResolver->isResourceClass($cvt->getClassName())) { + $unionResourceClasses[] = $cvt->getClassName(); + } + } elseif ($ut instanceof ObjectType && $this->resourceClassResolver->isResourceClass($ut->getClassName())) { + $unionResourceClasses[] = $ut->getClassName(); + } elseif ($ut instanceof LegacyType) { + if ($ut->isCollection()) { + foreach ($ut->getCollectionValueTypes() as $cvt) { + if (null !== ($cn = $cvt->getClassName()) && $this->resourceClassResolver->isResourceClass($cn)) { + $unionResourceClasses[] = $cn; + } + } + } elseif (null !== ($cn = $ut->getClassName()) && $this->resourceClassResolver->isResourceClass($cn)) { + $unionResourceClasses[] = $cn; + } + } + } + if ($unionResourceClasses) { + $context['_api_union_resource_classes'] = array_unique($unionResourceClasses); + } + } + foreach ($types as $t) { if ($type instanceof Type) { $isNullable = $type->isNullable();