fix(security): URL import with own range check and a generic error (audit M3, cheap part)
PublicAddress blocks every private/reserved IPv4 and IPv6 range (169.254/16, 100.64/10, fc00::/7, fe80::/10, IPv4-mapped/NAT64/6to4 by their embedded address, ...) independent of the core version; checked before the request and on every redirect (Requests before_redirect). Network failures return only "Die Datei konnte nicht geladen werden."; the detail goes to the error log. DNS-rebinding pinning stays deferred. 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:
@@ -81,9 +81,10 @@ final class MediaImporterTest extends TestCase
|
||||
parent::tearDown();
|
||||
}
|
||||
|
||||
private function importer(): MediaImporter
|
||||
/** @param list<string> $addresses What every host name resolves to (no DNS in unit tests). */
|
||||
private function importer(array $addresses = ['93.184.216.34']): MediaImporter
|
||||
{
|
||||
return new MediaImporter(new Limits());
|
||||
return new MediaImporter(new Limits(), static fn (string $host): array => $addresses);
|
||||
}
|
||||
|
||||
private function assertTempFilesRemoved(): void
|
||||
@@ -332,8 +333,11 @@ final class MediaImporterTest extends TestCase
|
||||
|
||||
$result = $this->importer()->fromUrl('https://example.com/fehlt.jpg', null, []);
|
||||
|
||||
// Security M3: no refused/timeout/404 oracle for the client; the detail goes to the log only.
|
||||
self::assertToolError($result, 'validation_failed');
|
||||
self::assertStringContainsString('Not Found', $result->get_error_message());
|
||||
self::assertStringContainsString('Die Datei konnte nicht geladen werden.', $result->get_error_message());
|
||||
self::assertStringNotContainsString('Not Found', $result->get_error_message());
|
||||
self::assertStringContainsString('Not Found', (string) file_get_contents($this->errorLog));
|
||||
$this->assertTempFilesRemoved();
|
||||
}
|
||||
|
||||
@@ -345,7 +349,9 @@ final class MediaImporterTest extends TestCase
|
||||
$result = $this->importer()->fromUrl('https://example.com/fehlt.jpg', null, []);
|
||||
|
||||
self::assertToolError($result, 'validation_failed');
|
||||
self::assertStringContainsString('404', $result->get_error_message());
|
||||
self::assertStringContainsString('Die Datei konnte nicht geladen werden.', $result->get_error_message());
|
||||
self::assertStringNotContainsString('404', $result->get_error_message());
|
||||
self::assertStringContainsString('404', (string) file_get_contents($this->errorLog));
|
||||
$this->assertTempFilesRemoved();
|
||||
}
|
||||
|
||||
@@ -464,4 +470,40 @@ final class MediaImporterTest extends TestCase
|
||||
|
||||
self::assertToolError($result, 'write_failed');
|
||||
}
|
||||
|
||||
/** Security M3: the plugin's own range check, whatever wp_http_validate_url() of the core version says. */
|
||||
public function testPrivateTargetsAreRejectedEvenWhenCoreLetsThemPass(): void
|
||||
{
|
||||
Functions\when('wp_http_validate_url')->returnArg();
|
||||
Functions\expect('wp_safe_remote_get')->never();
|
||||
|
||||
foreach (['http://169.254.169.254/latest/meta-data/', 'http://100.64.0.1/x.png', 'http://[fd00::1]/x.png'] as $url) {
|
||||
self::assertToolError($this->importer()->fromUrl($url, null, []), 'validation_failed', $url);
|
||||
}
|
||||
|
||||
$internal = $this->importer(['10.0.0.5'])->fromUrl('https://intern.example.com/x.png', null, []);
|
||||
self::assertToolError($internal, 'validation_failed');
|
||||
$unresolvable = $this->importer([])->fromUrl('https://nirgends.example.com/x.png', null, []);
|
||||
self::assertToolError($unresolvable, 'validation_failed');
|
||||
}
|
||||
|
||||
/** Redirect targets get the same check (Requests' before_redirect hook, only during the import). */
|
||||
public function testRedirectsToPrivateTargetsAreStopped(): void
|
||||
{
|
||||
$this->respond('PNGDATA', 200);
|
||||
$listener = null;
|
||||
Actions\expectAdded('requests-requests.before_redirect')->once()
|
||||
->whenHappen(static function (callable $callback) use (&$listener): void {
|
||||
$listener = $callback;
|
||||
});
|
||||
Functions\when('wp_check_filetype_and_ext')->justReturn(['ext' => false, 'type' => false, 'proper_filename' => false]);
|
||||
|
||||
$this->importer()->fromUrl('https://example.com/logo.png', null, []);
|
||||
|
||||
self::assertIsCallable($listener);
|
||||
self::assertFalse(has_action('requests-requests.before_redirect'));
|
||||
$listener('https://cdn.example.com/logo.png'); // public: no exception
|
||||
$this->expectException(\WpOrg\Requests\Exception::class);
|
||||
$listener('http://169.254.169.254/latest/meta-data/');
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user