From 71c2e03c9b35d27bd2c1301daca4ce8682a163b9 Mon Sep 17 00:00:00 2001 From: Jan-Niclas Loosen Date: Tue, 20 Oct 2026 00:11:11 +0200 Subject: [PATCH] =?UTF-8?q?fix(audit=20L1):=20Schleifen-Rechtepr=C3=BCfung?= =?UTF-8?q?=20auf=20dem=20bereinigten=20Wert=20(sanitize=5Ftext=5Ffield-Um?= =?UTF-8?q?gehung)=20und=20mit=20sanitize=5Fkey-Lesart=20von=20enable/quer?= =?UTF-8?q?yType?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 5.5 --- src/Integrations/Divi/Compose/AttrWriter.php | 6 ++- .../Divi/Compose/ValueCleaner.php | 9 ++-- tests/Support/DiviWriterFixture.php | 12 ++++-- .../Divi/Pages/DiviLoopWriteTest.php | 41 +++++++++++++++++++ 4 files changed, 59 insertions(+), 9 deletions(-) diff --git a/src/Integrations/Divi/Compose/AttrWriter.php b/src/Integrations/Divi/Compose/AttrWriter.php index dd11f94..4d73e76 100644 --- a/src/Integrations/Divi/Compose/AttrWriter.php +++ b/src/Integrations/Divi/Compose/AttrWriter.php @@ -145,10 +145,12 @@ final class AttrWriter { foreach ($normalized as $breakpoint => $states) { foreach ($states as $state => $value) { + $clean = $this->cleaner->clean($module, $path, $value); if ($breakpoint === 'desktop' && $state === 'value') { - $this->cleaner->guardLoop($path, $value); + // The cleaned value: sanitizers turn " on" or "user_roles" into what Divi acts on. + $this->cleaner->guardLoop($path, $clean); } - $normalized[$breakpoint][$state] = $this->cleaner->clean($module, $path, $value); + $normalized[$breakpoint][$state] = $clean; } } diff --git a/src/Integrations/Divi/Compose/ValueCleaner.php b/src/Integrations/Divi/Compose/ValueCleaner.php index fef16e9..99c6921 100644 --- a/src/Integrations/Divi/Compose/ValueCleaner.php +++ b/src/Integrations/Divi/Compose/ValueCleaner.php @@ -180,12 +180,15 @@ final class ValueCleaner if ($path !== 'module.advanced.loop' || ! is_array($desktopValue)) { return; } - if (($desktopValue['enable'] ?? null) !== 'on') { + // Divi reads enable and queryType through sanitize_key() (LoopUtils.php:725-731): stored values count so. + $key = static fn (mixed $value): string => is_string($value) + ? (string) preg_replace('/[^a-z0-9_\-]/', '', strtolower($value)) + : ''; + if ($key($desktopValue['enable'] ?? null) !== 'on') { return; } - $queryType = $desktopValue['queryType'] ?? 'post_types'; - $queryType = is_string($queryType) ? preg_replace('/[^a-z0-9_\-]/', '', strtolower($queryType)) : ''; + $queryType = array_key_exists('queryType', $desktopValue) ? $key($desktopValue['queryType']) : 'post_types'; if (($queryType === 'user_roles' || $queryType === 'users') && ! $this->sanitizer->canListUsers()) { throw new InvalidArgumentException( 'Eine Benutzer-Schleife listet Konten und E-Mail-Adressen und braucht das Recht list_users; ' diff --git a/tests/Support/DiviWriterFixture.php b/tests/Support/DiviWriterFixture.php index 750a6fd..dfac59b 100644 --- a/tests/Support/DiviWriterFixture.php +++ b/tests/Support/DiviWriterFixture.php @@ -50,12 +50,14 @@ final class DiviWriterFixture public static function composition( DesignSystemReader $design, bool $unfiltered = false, - ?DynamicContentMap $registry = null + ?DynamicContentMap $registry = null, + bool $listUsers = true, + bool $readPrivate = true ): DiviComposition { return new DiviComposition( self::modules(), $design, - ComposeFixture::sanitizer($unfiltered), + ComposeFixture::sanitizer($unfiltered, $listUsers, $readPrivate), ComposeFixture::media(), $registry ?? new DynamicContentMap() ); @@ -106,7 +108,9 @@ final class DiviWriterFixture ?GlobalEdits $globals = null, ?ThemeBuilderEdits $themeBuilder = null, ?SimilarSections $similar = null, - ?DynamicContentMap $registry = null + ?DynamicContentMap $registry = null, + bool $listUsers = true, + bool $readPrivate = true ): DiviWriter { $design = self::designReader(); @@ -115,7 +119,7 @@ final class DiviWriterFixture self::saver($saved, $failures), self::log(), $design, - self::composition($design, $unfiltered, $registry), + self::composition($design, $unfiltered, $registry, $listUsers, $readPrivate), $globals, $themeBuilder, $similar diff --git a/tests/Unit/Integrations/Divi/Pages/DiviLoopWriteTest.php b/tests/Unit/Integrations/Divi/Pages/DiviLoopWriteTest.php index 72fab8b..84e8653 100644 --- a/tests/Unit/Integrations/Divi/Pages/DiviLoopWriteTest.php +++ b/tests/Unit/Integrations/Divi/Pages/DiviLoopWriteTest.php @@ -439,4 +439,45 @@ final class DiviLoopWriteTest extends DiviWriteAbilityTestCase self::assertStringContainsString('kann Daten eines anderen Objekts zeigen', $text); self::assertStringNotContainsString('zeigt sie dort leer', $text); } + + /** @return array, string}> */ + public static function disguisedLoops(): array + { + return [ + 'enable with a space' => [['enable' => ' on', 'queryType' => 'user_roles'], 'list_users'], + 'enable with a tag' => [['enable' => 'on', 'queryType' => 'user_roles'], 'list_users'], + 'queryType with empty tags' => [['enable' => 'on', 'queryType' => 'user_roles'], 'list_users'], + 'any via tag' => [ + ['enable' => 'on', 'queryType' => 'post_types', 'subTypes' => [['value' => 'attachment']]], + 'read_private_posts', + ], + 'empty subTypes' => [['enable' => 'on', 'queryType' => 'post_types', 'subTypes' => []], 'read_private_posts'], + ]; + } + + /** + * Audit L1 (re-review): the check runs on the value the sanitizer leaves – sanitize_text_field turns " on", + * "on" or "user_roles" into the values Divi acts on. Through insert and update. + * + * @dataProvider disguisedLoops + * @param array $loop + */ + public function testDisguisedLoopsAreRefusedWithoutTheRights(array $loop, string $right): void + { + $registry = self::registry(); + $this->writer = DiviWriterFixture::writer( + $this->saved, + registry: new DynamicContentMap(static fn (int $postId): array => $registry), + listUsers: false, + readPrivate: false + ); + $attrs = ['module.advanced.loop' => ['desktop' => $loop]]; + + $inserted = $this->insertInto('s0.r0.c2', ['nodes' => [['type' => 'text', 'attrs' => $attrs, 'content' => 'x']]]); + self::assertStringContainsString($right, self::problems($inserted)); + + $updated = $this->changeAll([['id' => 's0.r0.c2', 'attrs' => $attrs]]); + self::assertStringContainsString($right, self::problems($updated)); + self::assertSame([], $this->saved); + } }