Author: Joe Ferguson (svpernova09) Committer: Derick Rethans (derickr) Date: 2026-07-27T08:11:31+01:00
Commit: https://github.com/php/web-php/commit/430b2bed08283f7ef9ec517a3930a64352bd7387 Raw diff: https://github.com/php/web-php/commit/430b2bed08283f7ef9ec517a3930a64352bd7387.diff Do not abort cross-origin requests with an empty 200 (#1955) Changed paths: A tests/EndToEnd/OriginHeaderTest.php M include/prepend.inc Diff: diff --git a/include/prepend.inc b/include/prepend.inc index b2f3969f11..c1fa1f99bd 100644 --- a/include/prepend.inc +++ b/include/prepend.inc @@ -21,20 +21,39 @@ header("Permissions-Policy: interest-cohort=()"); /* Fix Silly Same Origin Policies */ (function (): void { + // The presence of CORS headers below depends on the Origin request header, + // so shared caches must not reuse a response across different origins. + header("Vary: Origin", false); + if (!isset($_SERVER["HTTP_ORIGIN"])) { return; } + // Browsers send a literal "null" origin for sandboxed iframes, documents + // loaded from data:/file: URLs and some cross origin redirects. parse_url() + // reports no host at all for those, so treat them as untrusted. $host = parse_url($_SERVER["HTTP_ORIGIN"]); - if (!preg_match('/^(.+\.)?php\.net$/', $host["host"])) { - if ($host["host"] != $_SERVER["SERVER_NAME"]) { - exit(10); + $hostname = $host["host"] ?? ""; + + $isTrustedOrigin = $hostname !== "" && ( + preg_match('/^(.+\.)?php\.net$/', $hostname) === 1 + || $hostname === ($_SERVER["SERVER_NAME"] ?? "") + ); + + if (!$isTrustedOrigin) { + // Serve the page normally, only without any CORS headers. + if (in_array($_SERVER["REQUEST_METHOD"] ?? "GET", ["GET", "HEAD", "OPTIONS"], true)) { + return; } + + // Never respond with an empty body here: that is what ended up cached. + header("Cache-Control: no-store"); + http_response_code(403); + exit("Cross-origin requests are not allowed for this endpoint.\n"); } + if (isset($host["port"])) { - $hostname = $host["host"] . ":" . $host["port"]; - } else { - $hostname = $host["host"]; + $hostname .= ":" . $host["port"]; } header("Access-Control-Allow-Origin: http://$hostname"); diff --git a/tests/EndToEnd/OriginHeaderTest.php b/tests/EndToEnd/OriginHeaderTest.php new file mode 100644 index 0000000000..e62f30acd6 --- /dev/null +++ b/tests/EndToEnd/OriginHeaderTest.php @@ -0,0 +1,256 @@ +<?php + +declare(strict_types=1); + +namespace phpweb\Test\EndToEnd; + +use PHPUnit\Framework; + +/** + * Regression tests for the blank-page bug reported in + * https://github.com/php/web-php/issues/1955. + * + * The CORS guard in include/prepend.inc used to terminate the request with + * exit(10) whenever the Origin request header did not belong to php.net. + * exit(10) emits an integer exit status, not a body, so the response was a + * perfectly valid "200 OK" with a zero byte body. + * + * Since Origin was not part of the CDN cache key and no Vary header was sent, + * a single request carrying a foreign Origin header was enough for the CDN to + * store that empty 200 and replay it to every subsequent visitor as a blank + * white page, until the cache was purged. + * + * SmokeTest only asserts the status code, so it could not catch this: a blank + * page is still a 200. + */ +#[Framework\Attributes\CoversNothing] +final class OriginHeaderTest extends Framework\TestCase +{ + /** + * A cross-origin request must never produce an empty body, because that + * body is what the CDN caches and replays to everyone else. + */ + #[Framework\Attributes\DataProvider('provideForeignOrigin')] + public function testForeignOriginDoesNotReturnBlankPage(string $origin): void + { + $response = self::get('/', $origin); + + self::assertSame(200, $response['status'], sprintf( + 'Expected "/" to return 200 for Origin "%s", got "%d".', + $origin, + $response['status'], + )); + + self::assertNotSame('', $response['body'], sprintf( + 'Blank page: "/" returned a %d response with a zero byte body for Origin "%s". ' + . 'The CDN caches this empty body and serves it to all visitors.', + $response['status'], + $origin, + )); + } + + /** + * Every page is affected, not just the homepage. + */ + #[Framework\Attributes\DataProvider('providePath')] + public function testPathDoesNotReturnBlankPageForForeignOrigin(string $path): void + { + $response = self::get($path, 'https://example.com'); + + self::assertNotSame('', $response['body'], sprintf( + 'Blank page: "%s" returned a %d response with a zero byte body for a foreign Origin.', + $path, + $response['status'], + )); + } + + /** + * Guards the fix from the other side: legitimate php.net origins, and + * requests with no Origin at all, must keep working. + */ + #[Framework\Attributes\DataProvider('provideAllowedOrigin')] + public function testAllowedOriginReturnsContent(?string $origin): void + { + $response = self::get('/', $origin); + + self::assertSame(200, $response['status']); + self::assertNotSame('', $response['body']); + } + + /** + * The CORS headers depend on the Origin request header, so shared caches + * have to be told not to reuse a response across origins. + */ + public function testResponseVariesOnOrigin(): void + { + $response = self::get('/'); + + self::assertStringContainsStringIgnoringCase('origin', $response['vary'], sprintf( + 'Expected a "Vary: Origin" response header, got "%s".', + $response['vary'], + )); + } + + /** + * The blank page must not be traded for a cross site request forgery hole: + * the state changing endpoints have no CSRF tokens, so a cross origin POST + * still has to be rejected. + */ + public function testCrossOriginPostIsRejected(): void + { + $response = self::post('/manual/vote-note.php', 'https://example.com'); + + self::assertSame(403, $response['status'], sprintf( + 'Expected a cross-origin POST to be rejected with 403, got "%d".', + $response['status'], + )); + } + + /** + * Rejections still need a body, otherwise they are just the blank page + * again wearing a different status code. + */ + public function testRejectedCrossOriginPostStillHasABody(): void + { + $response = self::post('/manual/vote-note.php', 'https://example.com'); + + self::assertNotSame('', $response['body'], 'A rejected cross-origin POST returned an empty body.'); + } + + /** + * Same-origin POSTs must keep working. + */ + public function testSameOriginPostIsNotRejected(): void + { + $httpHost = getenv('HTTP_HOST'); + + $response = self::post('/manual/vote-note.php', sprintf('http://%s', $httpHost)); + + self::assertNotSame(403, $response['status'], 'A same-origin POST was rejected.'); + } + + /** + * @return \Generator<string, array{0: string}> + */ + public static function provideForeignOrigin(): \Generator + { + $origins = [ + // Any unrelated site embedding or fetching php.net. + 'https://example.com', + // Browsers send a literal "null" origin for sandboxed iframes, + // documents from data:/file: URLs, and some cross-origin redirects. + 'null', + // Must not be treated as php.net just because it contains it. + 'https://evil-php.net.attacker.com', + 'https://notphp.net', + ]; + + foreach ($origins as $origin) { + yield $origin => [$origin]; + } + } + + /** + * @return \Generator<string, array{0: ?string}> + */ + public static function provideAllowedOrigin(): \Generator + { + $origins = [ + 'no Origin header' => null, + 'https://www.php.net' => 'https://www.php.net', + 'https://php.net' => 'https://php.net', + 'https://qa.php.net' => 'https://qa.php.net', + ]; + + foreach ($origins as $name => $origin) { + yield $name => [$origin]; + } + } + + /** + * @return \Generator<string, array{0: string}> + */ + public static function providePath(): \Generator + { + $paths = [ + '/', + '/downloads.php', + '/contact.php', + '/manual/en/function.str-replace.php', + '/releases/', + ]; + + foreach ($paths as $path) { + yield $path => [$path]; + } + } + + /** + * @return array{status: int, body: string, vary: string} + */ + private static function post(string $path, string $origin): array + { + return self::get($path, $origin, ['vote' => 'up']); + } + + /** + * @param ?array<string, string> $postFields + * + * @return array{status: int, body: string, vary: string} + */ + private static function get( + string $path, + ?string $origin = null, + ?array $postFields = null, + ): array + { + $httpHost = getenv('HTTP_HOST'); + + if (!is_string($httpHost)) { + throw new \RuntimeException('Environment variable "HTTP_HOST" is not set.'); + } + + $headers = []; + + if (is_string($origin)) { + $headers[] = sprintf('Origin: %s', $origin); + } + + $vary = ''; + + $handle = curl_init(); + + if (is_array($postFields)) { + curl_setopt($handle, CURLOPT_POST, true); + curl_setopt($handle, CURLOPT_POSTFIELDS, http_build_query($postFields)); + } + + curl_setopt_array($handle, [ + CURLOPT_RETURNTRANSFER => true, + CURLOPT_HTTPHEADER => $headers, + CURLOPT_URL => sprintf('http://%s%s', $httpHost, $path), + CURLOPT_HEADERFUNCTION => static function ($handle, string $header) use (&$vary): int { + if (stripos($header, 'vary:') === 0) { + $vary = trim(substr($header, strlen('vary:'))); + } + + return strlen($header); + }, + ]); + + $body = curl_exec($handle); + $status = curl_getinfo($handle, CURLINFO_HTTP_CODE); + + curl_close($handle); + + if (!is_string($body)) { + throw new \RuntimeException(sprintf('Failed to request "%s".', $path)); + } + + return [ + 'status' => $status, + 'body' => $body, + 'vary' => $vary, + ]; + } +}
