fix(security): Divi-markup refusal sees content as storage will (ruling S52)

Control characters removed by kses (wp_kses_no_null: C0, \\0 sequences)
or ControlChars (DEL) could hide a divi/* delimiter or an et_pb shortcode
from the raw-input check. The shared DiviMarkup helper checks the raw
and the normalized form and now guards every raw-content path:
content-create/-update content and media-update description (attachment
post_content). MediaImporter logs import failures with scheme, host,
port and path only; uninstall.php lists the removed user meta.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7
This commit is contained in:
2026-09-26 19:50:31 +02:00
co-authored by Claude Opus 5.5
parent b24d90d8b0
commit 99eda25452
10 changed files with 173 additions and 15 deletions
+20
View File
@@ -396,4 +396,24 @@ echo "$VIA_EXECUTE" | jq -e 'has("__error")' >/dev/null || fail "9b: lit/divi-in
echo "$VIA_EXECUTE" | grep -q 'not found' || fail "9b: expected ability not found: $VIA_EXECUTE"
pass "Divi read-only: content-create divi refused, content-create without divi works, divi-insert not executable"
# 9c: control characters cannot hide the markup (ruling S52) – as the user without unfiltered_html,
# on create, update and the attachment description.
wp option delete lit_mcp_exposure >/dev/null 2>&1 || true
[ "$ORIGINAL_EXPOSURE" = "__none__" ] || wp option update lit_mcp_exposure "$ORIGINAL_EXPOSURE" --format=json >/dev/null
ATTACHMENT_ID=$(wp eval 'echo wp_insert_attachment(["post_title" => "LIT Divi Anhang", "post_mime_type" => "image/png", "post_status" => "inherit"]);')
CREATED_POSTS+=("$ATTACHMENT_ID")
connect "$EDITOR_LOGIN"
for hidden in '<!--\u0000 wp:divi/text {\"a\":1} /-->' '<!-- wp:di\u0000vi/text /-->' '<!-- wp:divi\u0001/text /-->' '[et_\u0000pb_text]x[/et_pb_text]'; do
NUL_CREATE=$(tool lit-content-create "$(jq -cn --argjson c "\"$hidden\"" '{post_type: "page", title: "LIT Divi NUL", content: $c}')")
[ "$(error_code "$NUL_CREATE")" = "validation_failed" ] || fail "9c: create accepted hidden Divi markup $hidden: $NUL_CREATE"
NUL_REV=$(tool lit-content-get "{\"post_id\":$PLAIN_ID}" | jq -r '.rev')
NUL_UPDATE=$(tool lit-content-update "$(jq -cn --argjson id "$PLAIN_ID" --arg rev "$NUL_REV" --argjson c "\"$hidden\"" '{post_id: $id, rev: $rev, content: $c}')")
[ "$(error_code "$NUL_UPDATE")" = "validation_failed" ] || fail "9c: update accepted hidden Divi markup $hidden: $NUL_UPDATE"
NUL_MEDIA=$(ability lit/media-update "$(jq -cn --argjson id "$ATTACHMENT_ID" --argjson c "\"$hidden\"" '{items: [{id: $id, description: $c}]}')")
[ "$(error_code "$NUL_MEDIA")" = "validation_failed" ] || fail "9c: media description accepted hidden Divi markup $hidden: $NUL_MEDIA"
done
if content_of "$PLAIN_ID" | grep -q 'wp:divi'; then fail "9c: Divi markup reached the stored content"; fi
if content_of "$ATTACHMENT_ID" | grep -q 'wp:divi'; then fail "9c: Divi markup reached the attachment description"; fi
pass "control characters inside the Divi delimiter or shortcode refused (create, update, media description)"
finish
+50
View File
@@ -0,0 +1,50 @@
<?php
declare(strict_types=1);
namespace LIT\MCP\Tests\Unit\Abilities;
use LIT\MCP\Abilities\DiviMarkup;
use PHPUnit\Framework\TestCase;
/**
* Ruling S52 (fix round 1 of security audit M1): raw content is checked for Divi markup in the
* form storage will see – after the removals of wp_kses_no_null() (C0 controls, "\0" sequences)
* and ControlChars (plus DEL) –, so control characters inside a delimiter cannot hide it.
*/
final class DiviMarkupTest extends TestCase
{
/** @return array<string, array{string}> */
public static function hidden(): array
{
$cases = [];
foreach (["\0", "\x01", "\x08", "\x0B", "\x0C", "\x1F", "\x7F", '\\0', '\\\\00'] as $i => $c) {
$cases["opener-after-dash $i"] = ["<!--$c wp:divi/text {\"a\":1} /-->"];
$cases["inside-namespace $i"] = ["<!-- wp:di{$c}vi/text /-->"];
$cases["before-slash $i"] = ["<!-- wp:divi$c/text /-->"];
$cases["inside-wp $i"] = ["<!-- w{$c}p:divi/section --><!-- /wp:divi/section -->"];
$cases["closer $i"] = ["<!-- /{$c}wp:divi/row -->"];
$cases["between-dashes $i"] = ["<!-$c- wp:divi/text /-->"];
$cases["shortcode $i"] = ["[et_{$c}pb_section][/et_pb_section]"];
$cases["shortcode-start $i"] = ["[$c/et_pb_text]"];
}
return $cases;
}
/** @dataProvider hidden */
public function testControlCharactersDoNotHideDiviMarkup(string $content): void
{
self::assertTrue(DiviMarkup::contains($content));
}
public function testPlainDelimitersAndOtherMarkup(): void
{
self::assertTrue(DiviMarkup::contains("<!--\t\n wp:divi/text /-->"));
self::assertTrue(DiviMarkup::contains('[et_pb_section]'));
self::assertFalse(DiviMarkup::contains('<!-- wp:paragraph --><p>Divi</p><!-- /wp:paragraph -->'));
self::assertFalse(DiviMarkup::contains('<!--wp:divi/text /-->'), 'no whitespace: no block for the parser');
self::assertFalse(DiviMarkup::contains('<!-- WP:DIVI/text /-->'), 'the parser is case-sensitive');
self::assertSame('<!-- wp:divi/text /-->', DiviMarkup::normalized("<!--\0 wp:di\x7Fvi/text /-->"));
}
}
@@ -506,4 +506,20 @@ final class MediaImporterTest extends TestCase
$this->expectException(\WpOrg\Requests\Exception::class);
$listener('http://169.254.169.254/latest/meta-data/');
}
/** Fix round 1: the log names scheme, host and path only – no credentials, no query string. */
public function testTheFailureLogOmitsUserinfoAndQuery(): void
{
Functions\when('wp_http_validate_url')->returnArg();
Functions\when('wp_parse_url')->alias('parse_url');
Functions\when('wp_safe_remote_get')->justReturn(new WP_Error('http_request_failed', 'timed out'));
$this->importer()->fromUrl('https://nutzer:geheim@example.com:8443/bilder/a.jpg?token=abc#x', null, []);
$log = (string) file_get_contents($this->errorLog);
self::assertStringContainsString('https://example.com:8443/bilder/a.jpg', $log);
self::assertStringNotContainsString('geheim', $log);
self::assertStringNotContainsString('nutzer', $log);
self::assertStringNotContainsString('token=abc', $log);
}
}
@@ -267,4 +267,19 @@ final class MediaUpdateAbilityTest extends TestCase
self::assertSame(50, $ability->inputSchema()['properties']['items']['maxItems']);
self::assertStringContainsString('modified', $ability->description());
}
/** Ruling S52: description is the attachment's post_content – no Divi markup there either. */
public function testDescriptionRefusesDiviMarkupEvenWithControlCharacters(): void
{
foreach (["<!-- wp:divi/text /-->", "<!--\0 wp:divi/text /-->", "[et_\x01pb_text]x[/et_pb_text]"] as $description) {
$result = (new MediaUpdateAbility($this->log()))->execute(['items' => [['id' => 33, 'description' => $description]]]);
self::assertToolError($result, 'validation_failed', $description);
self::assertStringContainsString('Divi', $result->get_error_data()['failures'][0]['error']);
}
self::assertSame([], $this->postUpdates);
(new MediaUpdateAbility(FakeChangeLog::any(9)))->execute(['items' => [['id' => 33, 'description' => '<em>Team</em>']]]);
self::assertSame([['post_content' => '<em>Team</em>', 'ID' => 33]], $this->postUpdates);
}
}
@@ -89,6 +89,12 @@ final class PostWriterTest extends TestCase
"<p>x</p><!--\twp:divi/text {\"a\":1} /-->",
"<!--\n\n/wp:divi/row -->",
'[et_pb_section][/et_pb_section]',
// Ruling S52: control characters are removed before storage, so they cannot hide the markup.
"<!--\0 wp:divi/text {\"a\":1} /-->",
"<!-- wp:di\0vi/text /-->",
"<!-- wp:divi\x01/text /-->",
"[et_\0pb_section][/et_pb_section]",
'<!-- wp:di\\0vi/text /-->',
];
foreach ($refused as $content) {
foreach ([true, false] as $isCreate) {