fix: StoredNodeGuard keeps plain relative links copyable (P18)

A link without scheme and without whitespace, quotes, <, >, backslashes or
control characters passes although esc_url_raw() would prefix http://; every
value with a scheme, and protocol-relative //host, must survive url() unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-09-27 01:58:47 +02:00
co-authored by Claude Opus 5.5
parent e808832fd3
commit 16c6d59725
2 changed files with 123 additions and 3 deletions
@@ -29,8 +29,8 @@ defined('ABSPATH') || exit;
* also when the builder-only key sits below the state value, * also when the builder-only key sits below the state value,
* - a string with control characters (S32), rich text or code that the sanitizer would change * - a string with control characters (S32), rich text or code that the sanitizer would change
* (not kses-stable), a plain-text string with markup that is not kses-stable, a URL the * (not kses-stable), a plain-text string with markup that is not kses-stable, a URL the
* sanitizer would change, or a key inside a value that is no plain identifier (S13) – routed * sanitizer would change (plain relative links excepted, ruling P18), or a key inside a value
* exactly like ValueCleaner (kindAt(), VALUE_KEY), * that is no plain identifier (S13) – routed exactly like ValueCleaner (kindAt(), VALUE_KEY),
* - a $variable(…)$ reference VariableRef::canonical() refuses for the target post, or whose * - a $variable(…)$ reference VariableRef::canonical() refuses for the target post, or whose
* settings canonical() would clean (e.g. before/after with markup such a user may not write). * settings canonical() would clean (e.g. before/after with markup such a user may not write).
* Users with unfiltered_html may move anything (the same line as the S7 gate). Pure PHP. * Users with unfiltered_html may move anything (the same line as the S7 gate). Pure PHP.
@@ -42,6 +42,9 @@ final class StoredNodeGuard
/** An instance's globalModule: the library post ID, digits only (GlobalInstance::of()). */ /** An instance's globalModule: the library post ID, digits only (GlobalInstance::of()). */
private const LIBRARY_ID = '/^[1-9][0-9]*$/D'; private const LIBRARY_ID = '/^[1-9][0-9]*$/D';
/** P18: no scheme before the first "/", "?" or "#"; no whitespace, quotes, <, >, backslash or controls. */
private const RELATIVE_URL = '~^[^:/?#\s"\'<>\\\\\x00-\x1F\x7F]*(?:[/?#][^\s"\'<>\\\\\x00-\x1F\x7F]*)?$~D';
private const BUILDER_ONLY = 'nur im Divi-Builder übertragbar (oder mit dem Recht unfiltered_html)'; private const BUILDER_ONLY = 'nur im Divi-Builder übertragbar (oder mit dem Recht unfiltered_html)';
public function __construct( public function __construct(
@@ -247,13 +250,25 @@ final class StoredNodeGuard
ValueKind::Html => $this->sanitizer->html($value) === $value, ValueKind::Html => $this->sanitizer->html($value) === $value,
ValueKind::Code => $this->sanitizer->code($value) === $value, ValueKind::Code => $this->sanitizer->code($value) === $value,
ValueKind::Text => ! str_contains($value, '<') || $this->sanitizer->html($value) === $value, ValueKind::Text => ! str_contains($value, '<') || $this->sanitizer->html($value) === $value,
ValueKind::Url => $this->sanitizer->url($value) === trim($value), ValueKind::Url => $this->sanitizer->url($value) === trim($value) || self::isPlainRelativeUrl($value),
}; };
return $stable ? null : 'Inhalt, den dieses Konto so nicht schreiben darf (HTML, Skript oder Link) – ' return $stable ? null : 'Inhalt, den dieses Konto so nicht schreiben darf (HTML, Skript oder Link) – '
. self::BUILDER_ONLY . '.'; . self::BUILDER_ONLY . '.';
} }
/**
* Ruling P18: a link without scheme – nothing before the first "/", "?" or "#" contains ":" – and
* without whitespace, quotes, "<", ">", backslashes or control characters ("kontakt/", "/pfad",
* "#anker", "?q=1"). esc_url_raw() would prefix "http://" to some of them, yet they cannot carry
* javascript: or data:. Protocol-relative "//host" (and "\\host", which browsers read the same)
* does not count: it must survive url() unchanged.
*/
private static function isPlainRelativeUrl(string $value): bool
{
return preg_match(self::RELATIVE_URL, $value) === 1 && ! str_starts_with($value, '//');
}
/** Same check as a client reference (VariableRef::canonical()); refused when cleaning would change it. */ /** Same check as a client reference (VariableRef::canonical()); refused when cleaning would change it. */
private function referenceProblem(string $value, bool $url): ?string private function referenceProblem(string $value, bool $url): ?string
{ {
@@ -10,6 +10,7 @@ use LIT\MCP\Integrations\Divi\BlockTree\Parser;
use LIT\MCP\Integrations\Divi\BlockTree\Serializer; use LIT\MCP\Integrations\Divi\BlockTree\Serializer;
use LIT\MCP\Integrations\Divi\Compose\StoredNodeGuard; use LIT\MCP\Integrations\Divi\Compose\StoredNodeGuard;
use LIT\MCP\Integrations\Divi\Compose\ValueCleaner; use LIT\MCP\Integrations\Divi\Compose\ValueCleaner;
use LIT\MCP\Integrations\Divi\Compose\ValueSanitizer;
use LIT\MCP\Integrations\Divi\Compose\VariableRef; use LIT\MCP\Integrations\Divi\Compose\VariableRef;
use LIT\MCP\Integrations\Divi\Pages\DynamicContentMap; use LIT\MCP\Integrations\Divi\Pages\DynamicContentMap;
use LIT\MCP\Tests\Support\ComposeFixture; use LIT\MCP\Tests\Support\ComposeFixture;
@@ -322,4 +323,108 @@ final class StoredNodeGuardTest extends TestCase
self::assertCount(2, $problems); self::assertCount(2, $problems);
self::assertSame($markup, Serializer::block($block)); self::assertSame($markup, Serializer::block($block));
} }
/**
* Like esc_url_raw() for an editor: only allowed protocols, characters outside its list go, and a
* value without scheme that does not start with "/", "#" or "?" gets "http://" (so url() changes
* relative links like "kontakt/"). $keepProtocolRelative decides what "//host" becomes.
*/
private static function wpLikeGuard(bool $keepProtocolRelative = true): StoredNodeGuard
{
$inner = ComposeFixture::sanitizer();
$sanitizer = new class ($inner, $keepProtocolRelative) implements ValueSanitizer {
public function __construct(private ValueSanitizer $inner, private bool $keepProtocolRelative)
{
}
public function text(string $value): string
{
return $this->inner->text($value);
}
public function html(string $value): string
{
return $this->inner->html($value);
}
public function code(string $value): string
{
return $this->inner->code($value);
}
public function url(string $value): string
{
$allowed = '|[^a-z0-9-~+_.?#=!&;,/:%@$\|*\'()\[\]\x80-\xff]|i';
$value = (string) preg_replace($allowed, '', trim($value));
if (str_starts_with($value, '//')) {
return $this->keepProtocolRelative ? $value : 'http:' . $value;
}
if (preg_match('/^([a-z][a-z0-9+.-]*):/i', $value, $scheme) === 1) {
return in_array(strtolower($scheme[1]), ['http', 'https', 'mailto', 'tel'], true) ? $value : '';
}
return preg_match('/^[\/#?]/', $value) === 1 ? $value : 'http://' . $value;
}
public function allowsUnfilteredHtml(): bool
{
return false;
}
public function canReadPost(int $id): bool
{
return $this->inner->canReadPost($id);
}
};
$references = new VariableRef(
DiviFixtures::designSystem(),
DynamicContentMap::fromOptions(DiviFixtures::dynamicContentRegistry())
);
$cleaner = new ValueCleaner(DiviFixtures::catalog(), $references, $sanitizer);
return new StoredNodeGuard($cleaner, $references, $sanitizer);
}
private static function link(string $url): Block
{
return self::block(self::node('divi/button', ['button' => ['innerContent' => self::desktop([
'text' => 'Kontakt',
'linkUrl' => $url,
])]]));
}
/** Ruling P18: relative links stay copyable although esc_url_raw() would change them. */
public function testRelativeLinksWithoutSchemePass(): void
{
$accepted = ['kontakt/', '/pfad', '#anker', '?q=1', 'https://kunde.test/a?b=1#c', 'mailto:info@kunde.test'];
foreach ($accepted as $url) {
self::assertSame([], self::wpLikeGuard()->problems(self::link($url)), $url);
}
}
/** Ruling P18: every value with a scheme must survive url() unchanged; odd relative values are refused. */
public function testLinksWithASchemeOrOddCharactersAreRefused(): void
{
$refused = [
'javascript:alert(1)',
'JaVaScRiPt:alert(1)',
'java' . chr(9) . 'script:alert(1)',
'data:text/html,probe',
'kontakt/"data-probe="1',
"kontakt/'x",
'kontakt/ x',
'kontakt/<b>',
'\\evil.example',
'/\\evil.example',
];
foreach ($refused as $url) {
self::assertFalse(self::link($url)->isOpaque(), $url);
self::assertNotSame([], self::wpLikeGuard()->problems(self::link($url)), $url);
}
// Protocol-relative: only when url() leaves it as it is.
self::assertSame([], self::wpLikeGuard()->problems(self::link('//evil.example')));
self::assertNotSame([], self::wpLikeGuard(false)->problems(self::link('//evil.example')));
}
} }