fix(audit L1): Schleifen-Rechteprüfung auf dem bereinigten Wert (sanitize_text_field-Umgehung) und mit sanitize_key-Lesart von enable/queryType

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Jan-Niclas Loosen
2026-10-20 00:11:11 +02:00
co-authored by Claude Sonnet 5.5
parent bd80ab1394
commit 71c2e03c9b
4 changed files with 59 additions and 9 deletions
+4 -2
View File
@@ -145,10 +145,12 @@ final class AttrWriter
{ {
foreach ($normalized as $breakpoint => $states) { foreach ($normalized as $breakpoint => $states) {
foreach ($states as $state => $value) { foreach ($states as $state => $value) {
$clean = $this->cleaner->clean($module, $path, $value);
if ($breakpoint === 'desktop' && $state === 'value') { if ($breakpoint === 'desktop' && $state === 'value') {
$this->cleaner->guardLoop($path, $value); // The cleaned value: sanitizers turn " on" or "user<b></b>_roles" into what Divi acts on.
$this->cleaner->guardLoop($path, $clean);
} }
$normalized[$breakpoint][$state] = $this->cleaner->clean($module, $path, $value); $normalized[$breakpoint][$state] = $clean;
} }
} }
@@ -180,12 +180,15 @@ final class ValueCleaner
if ($path !== 'module.advanced.loop' || ! is_array($desktopValue)) { if ($path !== 'module.advanced.loop' || ! is_array($desktopValue)) {
return; 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; return;
} }
$queryType = $desktopValue['queryType'] ?? 'post_types'; $queryType = array_key_exists('queryType', $desktopValue) ? $key($desktopValue['queryType']) : 'post_types';
$queryType = is_string($queryType) ? preg_replace('/[^a-z0-9_\-]/', '', strtolower($queryType)) : '';
if (($queryType === 'user_roles' || $queryType === 'users') && ! $this->sanitizer->canListUsers()) { if (($queryType === 'user_roles' || $queryType === 'users') && ! $this->sanitizer->canListUsers()) {
throw new InvalidArgumentException( throw new InvalidArgumentException(
'Eine Benutzer-Schleife listet Konten und E-Mail-Adressen und braucht das Recht list_users; ' 'Eine Benutzer-Schleife listet Konten und E-Mail-Adressen und braucht das Recht list_users; '
+8 -4
View File
@@ -50,12 +50,14 @@ final class DiviWriterFixture
public static function composition( public static function composition(
DesignSystemReader $design, DesignSystemReader $design,
bool $unfiltered = false, bool $unfiltered = false,
?DynamicContentMap $registry = null ?DynamicContentMap $registry = null,
bool $listUsers = true,
bool $readPrivate = true
): DiviComposition { ): DiviComposition {
return new DiviComposition( return new DiviComposition(
self::modules(), self::modules(),
$design, $design,
ComposeFixture::sanitizer($unfiltered), ComposeFixture::sanitizer($unfiltered, $listUsers, $readPrivate),
ComposeFixture::media(), ComposeFixture::media(),
$registry ?? new DynamicContentMap() $registry ?? new DynamicContentMap()
); );
@@ -106,7 +108,9 @@ final class DiviWriterFixture
?GlobalEdits $globals = null, ?GlobalEdits $globals = null,
?ThemeBuilderEdits $themeBuilder = null, ?ThemeBuilderEdits $themeBuilder = null,
?SimilarSections $similar = null, ?SimilarSections $similar = null,
?DynamicContentMap $registry = null ?DynamicContentMap $registry = null,
bool $listUsers = true,
bool $readPrivate = true
): DiviWriter { ): DiviWriter {
$design = self::designReader(); $design = self::designReader();
@@ -115,7 +119,7 @@ final class DiviWriterFixture
self::saver($saved, $failures), self::saver($saved, $failures),
self::log(), self::log(),
$design, $design,
self::composition($design, $unfiltered, $registry), self::composition($design, $unfiltered, $registry, $listUsers, $readPrivate),
$globals, $globals,
$themeBuilder, $themeBuilder,
$similar $similar
@@ -439,4 +439,45 @@ final class DiviLoopWriteTest extends DiviWriteAbilityTestCase
self::assertStringContainsString('kann Daten eines anderen Objekts zeigen', $text); self::assertStringContainsString('kann Daten eines anderen Objekts zeigen', $text);
self::assertStringNotContainsString('zeigt sie dort leer', $text); self::assertStringNotContainsString('zeigt sie dort leer', $text);
} }
/** @return array<string, array{array<string, mixed>, string}> */
public static function disguisedLoops(): array
{
return [
'enable with a space' => [['enable' => ' on', 'queryType' => 'user_roles'], 'list_users'],
'enable with a tag' => [['enable' => 'o<x>n', 'queryType' => 'user_roles'], 'list_users'],
'queryType with empty tags' => [['enable' => 'on', 'queryType' => 'user<b></b>_roles'], 'list_users'],
'any via tag' => [
['enable' => 'on', 'queryType' => 'post_types', 'subTypes' => [['value' => 'att<x>achment']]],
'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",
* "o<x>n" or "user<b></b>_roles" into the values Divi acts on. Through insert and update.
*
* @dataProvider disguisedLoops
* @param array<string, mixed> $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);
}
} }