Skip to content

Commit 149ce96

Browse files
svpernova09svpernova09
committed
Do not abort cross-origin requests with an empty 200 (#1955)
1 parent 05cb1bf commit 149ce96

2 files changed

Lines changed: 281 additions & 6 deletions

File tree

‎include/prepend.inc‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,39 @@ header("Permissions-Policy: interest-cohort=()");
2121

2222
/* Fix Silly Same Origin Policies */
2323
(function (): void {
24+
// The presence of CORS headers below depends on the Origin request header,
25+
// so shared caches must not reuse a response across different origins.
26+
header("Vary: Origin", false);
27+
2428
if (!isset($_SERVER["HTTP_ORIGIN"])) {
2529
return;
2630
}
2731

32+
// Browsers send a literal "null" origin for sandboxed iframes, documents
33+
// loaded from data:/file: URLs and some cross origin redirects. parse_url()
34+
// reports no host at all for those, so treat them as untrusted.
2835
$host = parse_url($_SERVER["HTTP_ORIGIN"]);
29-
if (!preg_match('/^(.+\.)?php\.net$/', $host["host"])) {
30-
if ($host["host"] != $_SERVER["SERVER_NAME"]) {
31-
exit(10);
36+
$hostname = $host["host"] ?? "";
37+
38+
$isTrustedOrigin = $hostname !== "" && (
39+
preg_match('/^(.+\.)?php\.net$/', $hostname) === 1
40+
|| $hostname === ($_SERVER["SERVER_NAME"] ?? "")
41+
);
42+
43+
if (!$isTrustedOrigin) {
44+
// Serve the page normally, only without any CORS headers.
45+
if (in_array($_SERVER["REQUEST_METHOD"] ?? "GET", ["GET", "HEAD", "OPTIONS"], true)) {
46+
return;
3247
}
48+
49+
// Never respond with an empty body here: that is what ended up cached.
50+
header("Cache-Control: no-store");
51+
http_response_code(403);
52+
exit("Cross-origin requests are not allowed for this endpoint.\n");
3353
}
54+
3455
if (isset($host["port"])) {
35-
$hostname = $host["host"] . ":" . $host["port"];
36-
} else {
37-
$hostname = $host["host"];
56+
$hostname .= ":" . $host["port"];
3857
}
3958

4059
header("Access-Control-Allow-Origin: http://$hostname");

‎tests/EndToEnd/OriginHeaderTest.php‎

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace phpweb\Test\EndToEnd;
6+
7+
use PHPUnit\Framework;
8+
9+
/**
10+
* Regression tests for the blank-page bug reported in
11+
* https://github.com/php/web-php/issues/1955.
12+
*
13+
* The CORS guard in include/prepend.inc used to terminate the request with
14+
* exit(10) whenever the Origin request header did not belong to php.net.
15+
* exit(10) emits an integer exit status, not a body, so the response was a
16+
* perfectly valid "200 OK" with a zero byte body.
17+
*
18+
* Since Origin was not part of the CDN cache key and no Vary header was sent,
19+
* a single request carrying a foreign Origin header was enough for the CDN to
20+
* store that empty 200 and replay it to every subsequent visitor as a blank
21+
* white page, until the cache was purged.
22+
*
23+
* SmokeTest only asserts the status code, so it could not catch this: a blank
24+
* page is still a 200.
25+
*/
26+
#[Framework\Attributes\CoversNothing]
27+
final class OriginHeaderTest extends Framework\TestCase
28+
{
29+
/**
30+
* A cross-origin request must never produce an empty body, because that
31+
* body is what the CDN caches and replays to everyone else.
32+
*/
33+
#[Framework\Attributes\DataProvider('provideForeignOrigin')]
34+
public function testForeignOriginDoesNotReturnBlankPage(string $origin): void
35+
{
36+
$response = self::get('/', $origin);
37+
38+
self::assertSame(200, $response['status'], sprintf(
39+
'Expected "/" to return 200 for Origin "%s", got "%d".',
40+
$origin,
41+
$response['status'],
42+
));
43+
44+
self::assertNotSame('', $response['body'], sprintf(
45+
'Blank page: "/" returned a %d response with a zero byte body for Origin "%s". '
46+
. 'The CDN caches this empty body and serves it to all visitors.',
47+
$response['status'],
48+
$origin,
49+
));
50+
}
51+
52+
/**
53+
* Every page is affected, not just the homepage.
54+
*/
55+
#[Framework\Attributes\DataProvider('providePath')]
56+
public function testPathDoesNotReturnBlankPageForForeignOrigin(string $path): void
57+
{
58+
$response = self::get($path, 'https://example.com');
59+
60+
self::assertNotSame('', $response['body'], sprintf(
61+
'Blank page: "%s" returned a %d response with a zero byte body for a foreign Origin.',
62+
$path,
63+
$response['status'],
64+
));
65+
}
66+
67+
/**
68+
* Guards the fix from the other side: legitimate php.net origins, and
69+
* requests with no Origin at all, must keep working.
70+
*/
71+
#[Framework\Attributes\DataProvider('provideAllowedOrigin')]
72+
public function testAllowedOriginReturnsContent(?string $origin): void
73+
{
74+
$response = self::get('/', $origin);
75+
76+
self::assertSame(200, $response['status']);
77+
self::assertNotSame('', $response['body']);
78+
}
79+
80+
/**
81+
* The CORS headers depend on the Origin request header, so shared caches
82+
* have to be told not to reuse a response across origins.
83+
*/
84+
public function testResponseVariesOnOrigin(): void
85+
{
86+
$response = self::get('/');
87+
88+
self::assertStringContainsStringIgnoringCase('origin', $response['vary'], sprintf(
89+
'Expected a "Vary: Origin" response header, got "%s".',
90+
$response['vary'],
91+
));
92+
}
93+
94+
/**
95+
* The blank page must not be traded for a cross site request forgery hole:
96+
* the state changing endpoints have no CSRF tokens, so a cross origin POST
97+
* still has to be rejected.
98+
*/
99+
public function testCrossOriginPostIsRejected(): void
100+
{
101+
$response = self::post('/manual/vote-note.php', 'https://example.com');
102+
103+
self::assertSame(403, $response['status'], sprintf(
104+
'Expected a cross-origin POST to be rejected with 403, got "%d".',
105+
$response['status'],
106+
));
107+
}
108+
109+
/**
110+
* Rejections still need a body, otherwise they are just the blank page
111+
* again wearing a different status code.
112+
*/
113+
public function testRejectedCrossOriginPostStillHasABody(): void
114+
{
115+
$response = self::post('/manual/vote-note.php', 'https://example.com');
116+
117+
self::assertNotSame('', $response['body'], 'A rejected cross-origin POST returned an empty body.');
118+
}
119+
120+
/**
121+
* Same-origin POSTs must keep working.
122+
*/
123+
public function testSameOriginPostIsNotRejected(): void
124+
{
125+
$httpHost = getenv('HTTP_HOST');
126+
127+
$response = self::post('/manual/vote-note.php', sprintf('http://%s', $httpHost));
128+
129+
self::assertNotSame(403, $response['status'], 'A same-origin POST was rejected.');
130+
}
131+
132+
/**
133+
* @return \Generator<string, array{0: string}>
134+
*/
135+
public static function provideForeignOrigin(): \Generator
136+
{
137+
$origins = [
138+
// Any unrelated site embedding or fetching php.net.
139+
'https://example.com',
140+
// Browsers send a literal "null" origin for sandboxed iframes,
141+
// documents from data:/file: URLs, and some cross-origin redirects.
142+
'null',
143+
// Must not be treated as php.net just because it contains it.
144+
'https://evil-php.net.attacker.com',
145+
'https://notphp.net',
146+
];
147+
148+
foreach ($origins as $origin) {
149+
yield $origin => [$origin];
150+
}
151+
}
152+
153+
/**
154+
* @return \Generator<string, array{0: ?string}>
155+
*/
156+
public static function provideAllowedOrigin(): \Generator
157+
{
158+
$origins = [
159+
'no Origin header' => null,
160+
'https://www.php.net' => 'https://www.php.net',
161+
'https://php.net' => 'https://php.net',
162+
'https://qa.php.net' => 'https://qa.php.net',
163+
];
164+
165+
foreach ($origins as $name => $origin) {
166+
yield $name => [$origin];
167+
}
168+
}
169+
170+
/**
171+
* @return \Generator<string, array{0: string}>
172+
*/
173+
public static function providePath(): \Generator
174+
{
175+
$paths = [
176+
'/',
177+
'/downloads.php',
178+
'/contact.php',
179+
'/manual/en/function.str-replace.php',
180+
'/releases/',
181+
];
182+
183+
foreach ($paths as $path) {
184+
yield $path => [$path];
185+
}
186+
}
187+
188+
/**
189+
* @return array{status: int, body: string, vary: string}
190+
*/
191+
private static function post(string $path, string $origin): array
192+
{
193+
return self::get($path, $origin, ['vote' => 'up']);
194+
}
195+
196+
/**
197+
* @param ?array<string, string> $postFields
198+
*
199+
* @return array{status: int, body: string, vary: string}
200+
*/
201+
private static function get(
202+
string $path,
203+
?string $origin = null,
204+
?array $postFields = null,
205+
): array
206+
{
207+
$httpHost = getenv('HTTP_HOST');
208+
209+
if (!is_string($httpHost)) {
210+
throw new \RuntimeException('Environment variable "HTTP_HOST" is not set.');
211+
}
212+
213+
$headers = [];
214+
215+
if (is_string($origin)) {
216+
$headers[] = sprintf('Origin: %s', $origin);
217+
}
218+
219+
$vary = '';
220+
221+
$handle = curl_init();
222+
223+
if (is_array($postFields)) {
224+
curl_setopt($handle, CURLOPT_POST, true);
225+
curl_setopt($handle, CURLOPT_POSTFIELDS, http_build_query($postFields));
226+
}
227+
228+
curl_setopt_array($handle, [
229+
CURLOPT_RETURNTRANSFER => true,
230+
CURLOPT_HTTPHEADER => $headers,
231+
CURLOPT_URL => sprintf('http://%s%s', $httpHost, $path),
232+
CURLOPT_HEADERFUNCTION => static function ($handle, string $header) use (&$vary): int {
233+
if (stripos($header, 'vary:') === 0) {
234+
$vary = trim(substr($header, strlen('vary:')));
235+
}
236+
237+
return strlen($header);
238+
},
239+
]);
240+
241+
$body = curl_exec($handle);
242+
$status = curl_getinfo($handle, CURLINFO_HTTP_CODE);
243+
244+
curl_close($handle);
245+
246+
if (!is_string($body)) {
247+
throw new \RuntimeException(sprintf('Failed to request "%s".', $path));
248+
}
249+
250+
return [
251+
'status' => $status,
252+
'body' => $body,
253+
'vary' => $vary,
254+
];
255+
}
256+
}

0 commit comments

Comments
 (0)