Loosen-IT/WP-LIT-MCP
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 7 Wiki Activity

fix(audit L8, DNS-Rebinding): RemoteFileFetcher nur mit cURL und mit CURLOPT_RESOLVE auf die geprüfte Adresse (Host und Weiterleitungsziele)

Browse Source 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Jan-Niclas Loosen
2026-10-19 23:55:09 +02:00
co-authored by Claude Sonnet 5.5
parent 06b798ab37
commit 20e02164a5
2 changed files with 149 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Integrations/Media/RemoteFileFetcher.php
+51 -7
View File
@@ -16,8 +16,11 @@ defined('ABSPATH') || exit;
* the font upload. Only http(s), checked with wp_http_validate_url() and the plugin's own range check
* (PublicAddress, independent of the core version) before any request and on every redirect, fetched with
* wp_safe_remote_get() over IPv4 only (core re-validates redirects too), streamed into the temp file and cut off
* one byte above $maxBytes. A failure reaches the caller only as null (no refused/timeout/status oracle, security
* audit M3); the reason goes to the log. Not final so abilities can be tested with a fake.
* one byte above $maxBytes. cURL only: without it the request is refused (the IPv4 rule and the address pinning
* are cURL options), and every host of the chain is pinned to the address that was checked (CURLOPT_RESOLVE,
* so a second DNS answer cannot redirect the connection). A failure reaches the caller only as null (no
* refused/timeout/status oracle, security audit M3); the reason goes to the log. Not final so abilities can be
* tested with a fake.
*/
class RemoteFileFetcher
{
@@ -54,22 +57,50 @@ class RemoteFileFetcher
return $this->failed($context, $url, 'URL refused');
}
// Audit L8: the IPv4-only rule and the pinning below are cURL options; another transport (fsockopen) would
// ignore them and could reach an AAAA address nobody checked.
if (! $this->curlAvailable()) {
return $this->failed($context, $url, 'no cURL transport');
}
// Audit DNS rebinding: allows() resolved the host once and cURL would resolve it again. Every host of this
// request chain (the URL, each redirect target) is resolved here once, checked and handed to cURL as
// CURLOPT_RESOLVE, so the connection goes to the address that was checked.
$pins = [];
$pin = function (string $target) use (&$pins): void {
$host = (string) wp_parse_url($target, PHP_URL_HOST);
$port = (int) wp_parse_url($target, PHP_URL_PORT)
?: (strtolower((string) wp_parse_url($target, PHP_URL_SCHEME)) === 'https' ? 443 : 80);
$key = $host . ':' . $port;
if (isset($pins[$key]) || filter_var(trim($host, '[]'), FILTER_VALIDATE_IP) !== false) {
return;
}
$addresses = ($this->resolve)($host);
if ($addresses === [] || array_filter($addresses, [PublicAddress::class, 'isPublicIp']) !== $addresses) {
throw new RequestsException('Host resolved to a non-public address', 'lit_mcp_rebinding');
}
$pins[$key] = $key . ':' . $addresses[0];
};
// IPv4 only, for this request only: wp_http_validate_url() checks the IPv4 address
// (gethostbyname), so an AAAA record must not lead cURL to an unchecked IPv6 target.
$ipv4 = static function ($handle): void {
$ipv4 = function ($handle, $args = null, $target = null) use (&$pins, $pin, $url): void {
if (defined('CURLOPT_IPRESOLVE') && defined('CURL_IPRESOLVE_V4')) {
// phpcs:ignore WordPress.WP.AlternativeFunctions.curl_curl_setopt -- http_api_curl hands us the handle.
curl_setopt($handle, CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V4);
$this->curlSetopt($handle, CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V4);
}
$pin(is_string($target) ? $target : $url);
if ($pins !== [] && defined('CURLOPT_RESOLVE')) {
$this->curlSetopt($handle, CURLOPT_RESOLVE, array_values($pins));
}
};
// Every redirect target gets the plugin's own range check too; Requests turns the
// exception into a failed request (WP_Http returns a WP_Error).
$redirect = function ($location): void {
$redirect = function ($location) use ($pin): void {
if (! $this->isPublicUrl((string) $location)) {
throw new RequestsException('Redirect to a non-public address refused', 'lit_mcp_redirect');
}
$pin((string) $location);
};
add_action('http_api_curl', $ipv4);
add_action('http_api_curl', $ipv4, 10, 3);
add_action(self::REDIRECT_HOOK, $redirect);
try {
$response = wp_safe_remote_get($url, [
@@ -95,6 +126,19 @@ class RemoteFileFetcher
return $response;
}
/** Whether the cURL extension is there; a seam for tests (function_exists() cannot be undone). */
protected function curlAvailable(): bool
{
return function_exists('curl_init');
}
/** @param \CurlHandle $handle */
protected function curlSetopt($handle, int $option, mixed $value): void
{
// phpcs:ignore WordPress.WP.AlternativeFunctions.curl_curl_setopt -- http_api_curl hands us the handle.
curl_setopt($handle, $option, $value);
}
/** Host (name or IP literal) of $url resolves to public addresses only (PublicAddress). */
private function isPublicUrl(string $url): bool
{
tests/Unit/Integrations/Media/RemoteFileFetcherTest.php
+98
View File
@@ -139,4 +139,102 @@ final class RemoteFileFetcherTest extends TestCase
$this->expectException(\WpOrg\Requests\Exception::class);
$listener('http://127.0.0.1/inter.woff2');
}
/**
* A fetcher whose cURL options are recorded; $dns is called per lookup (host => addresses).
*
* @param \Closure(string): list<string> $dns
* @return object{options: list<array{0: int, 1: mixed}>}&RemoteFileFetcher
*/
private function recordingFetcher(\Closure $dns, bool $curl = true): RemoteFileFetcher
{
return new class ($dns, $curl) extends RemoteFileFetcher {
/** @var list<array{0: int, 1: mixed}> */
public array $options = [];
public function __construct(\Closure $dns, private bool $curl)
{
parent::__construct($dns);
}
protected function curlAvailable(): bool
{
return $this->curl;
}
protected function curlSetopt($handle, int $option, mixed $value): void
{
$this->options[] = [$option, $value];
}
};
}
/** Audit L8: no cURL, no request – the IPv4 rule and the pinning are cURL options. */
public function testWithoutCurlNothingIsRequested(): void
{
Functions\expect('wp_safe_remote_get')->never();
$fetcher = $this->recordingFetcher(static fn (): array => ['93.184.216.34'], false);
self::assertNull($fetcher->fetch('https://fonts.example.com/inter.woff2', $this->tmp, 2048, 'font upload'));
self::assertStringContainsString('no cURL transport', (string) file_get_contents($this->errorLog));
}
/** @return array{0: callable, 1: callable} The http_api_curl and the redirect listener of one fetch(). */
private function listeners(RemoteFileFetcher $fetcher, string $url): array
{
$curl = $redirect = null;
Actions\expectAdded('http_api_curl')->once()->whenHappen(static function (callable $cb) use (&$curl): void {
$curl = $cb;
});
Actions\expectAdded('requests-requests.before_redirect')->once()
->whenHappen(static function (callable $cb) use (&$redirect): void {
$redirect = $cb;
});
Functions\when('wp_safe_remote_get')->justReturn(['response' => ['code' => 200], 'headers' => []]);
$fetcher->fetch($url, $this->tmp, 2048, 'x');
return [$curl, $redirect];
}
/** Audit (DNS rebinding): cURL connects to the address that was checked, IPv4 only. */
public function testTheCheckedAddressIsPinnedForTheHostAndEveryRedirectTarget(): void
{
$fetcher = $this->recordingFetcher(static fn (string $host): array => [
'fonts.example.com' => ['93.184.216.34', '93.184.216.35'], 'cdn.example.com' => ['198.51.101.7'],
][$host]);
[$curl, $redirect] = $this->listeners($fetcher, 'https://fonts.example.com/inter.woff2');
$curl(curl_init(), [], 'https://fonts.example.com/inter.woff2');
self::assertSame(
[[CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V4], [CURLOPT_RESOLVE, ['fonts.example.com:443:93.184.216.34']]],
$fetcher->options
);
$fetcher->options = [];
$redirect('http://cdn.example.com:8080/inter.woff2');
$curl(curl_init(), [], 'https://fonts.example.com/inter.woff2'); // the redirect request: same original URL
self::assertSame(
[
[CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V4],
[CURLOPT_RESOLVE, ['fonts.example.com:443:93.184.216.34', 'cdn.example.com:8080:198.51.101.7']],
],
$fetcher->options
);
}
public function testAHostThatTurnsPrivateAfterTheCheckIsNotConnected(): void
{
$lookups = 0;
$fetcher = $this->recordingFetcher(static function () use (&$lookups): array {
return ++$lookups === 1 ? ['93.184.216.34'] : ['127.0.0.1'];
});
[$curl] = $this->listeners($fetcher, 'https://rebind.example.com/a.woff2');
$this->expectException(\WpOrg\Requests\Exception::class);
try {
$curl(curl_init(), [], 'https://rebind.example.com/a.woff2');
} finally {
self::assertSame([[CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V4]], $fetcher->options, 'no pin to the new address');
}
}
}