Skip to content

Commit 5973913

Browse files
iMattProiMattPro
committed
Only allow push notifications via known whitelist of push services
1 parent ab15164 commit 5973913

4 files changed

Lines changed: 126 additions & 2 deletions

File tree

‎language/en/webpushnotifications_module_ucp.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,5 @@
5757
'NOTIFY_WEBPUSH_POPUP_DISABLE_EXPLAIN' => 'We show a reminder asking you to allow browser notifications when they are not currently enabled or when we cannot detect them. Turn this off to permanently stop these reminder pop-ups on all your devices. If you turn this off, you will not be warned if your browser notifications stop working in the future.',
5858
'NOTIFY_WEBPUSH_POPUP_ENABLER' => 'Enable reminders',
5959
'NOTIFY_WEBPUSH_POPUP_DISABLER' => 'Disable reminders',
60+
'WEBPUSH_INVALID_ENDPOINT' => 'The push notification endpoint is not from a recognised push service.',
6061
]);

‎language/ru/webpushnotifications_module_ucp.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,5 @@
5757
'NOTIFY_WEBPUSH_POPUP_DISABLE_EXPLAIN' => 'Мы показываем напоминание с просьбой разрешить уведомления браузера, если они сейчас не включены или если мы не можем их обнаружить. Отключите эту функцию, чтобы навсегда убрать эти напоминания на всех ваших устройствах. Если вы отключите эту функцию, вы больше не будете получать предупреждения, если уведомления браузера перестанут работать в будущем.',
5858
'NOTIFY_WEBPUSH_POPUP_ENABLER' => 'Включить напоминания',
5959
'NOTIFY_WEBPUSH_POPUP_DISABLER' => 'Отключить напоминания',
60+
'WEBPUSH_INVALID_ENDPOINT' => 'Конечная точка push-уведомления не принадлежит известному сервису push-уведомлений.',
6061
]);

‎tests/controller/controller_webpush_test.php‎

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ public function test_sub_unsubscribe_success()
392392

393393
$symfony_request = $this->createMock(\phpbb\symfony_request::class);
394394
$symfony_request->method('get')->willReturn(json_encode([
395-
'endpoint' => 'test_endpoint',
395+
'endpoint' => 'https://fcm.googleapis.com/fcm/send/test_endpoint',
396396
'expiration_time' => 0,
397397
'keys' => ['p256dh' => 'test_p256dh', 'auth' => 'test_auth']
398398
]));
@@ -404,7 +404,7 @@ public function test_sub_unsubscribe_success()
404404

405405
$this->assertEquals([
406406
'user_id' => '2',
407-
'endpoint' => 'test_endpoint',
407+
'endpoint' => 'https://fcm.googleapis.com/fcm/send/test_endpoint',
408408
'p256dh' => 'test_p256dh',
409409
'auth' => 'test_auth',
410410
'expiration_time' => '0',
@@ -420,6 +420,77 @@ public function test_sub_unsubscribe_success()
420420
$this->assertEmpty($this->get_subscription_data());
421421
}
422422

423+
public function data_subscribe_invalid_endpoint(): array
424+
{
425+
return [
426+
'no_host' => ['not_a_url'],
427+
'disallowed_host' => ['https://evil.example.com/push/endpoint'],
428+
'disallowed_subdomain' => ['https://evil.fcm.googleapis.com.attacker.com/push'],
429+
'empty_string' => [''],
430+
];
431+
}
432+
433+
/**
434+
* @dataProvider data_subscribe_invalid_endpoint
435+
*/
436+
public function test_subscribe_invalid_endpoint(string $endpoint): void
437+
{
438+
$this->form_helper->method('check_form_tokens')->willReturn(true);
439+
$this->request->method('is_ajax')->willReturn(true);
440+
$this->user->data['user_id'] = 2;
441+
$this->user->data['is_bot'] = false;
442+
$this->user->data['user_type'] = USER_NORMAL;
443+
444+
$symfony_request = $this->createMock(\phpbb\symfony_request::class);
445+
$symfony_request->method('get')->willReturn(json_encode([
446+
'endpoint' => $endpoint,
447+
'expiration_time' => 0,
448+
'keys' => ['p256dh' => 'test_p256dh', 'auth' => 'test_auth']
449+
]));
450+
451+
$this->expectException(http_exception::class);
452+
$this->expectExceptionMessage('WEBPUSH_INVALID_ENDPOINT');
453+
454+
$this->controller->subscribe($symfony_request);
455+
}
456+
457+
public function data_is_valid_endpoint(): array
458+
{
459+
return [
460+
// Exact whitelist matches
461+
['https://android.googleapis.com/gcm/send/test', true],
462+
['https://fcm.googleapis.com/fcm/send/test', true],
463+
['https://updates.push.services.mozilla.com/push/v1/test', true],
464+
['https://updates-autopush.stage.mozaws.net/push/v1/test', true],
465+
['https://updates-autopush.dev.mozaws.net/push/v1/test', true],
466+
// Wildcard *.notify.windows.com
467+
['https://am-0.notify.windows.com/throttledthirdparty/test', true],
468+
['https://db5.notify.windows.com/push/test', true],
469+
// Wildcard *.push.apple.com
470+
['https://api.push.apple.com/3/device/test', true],
471+
['https://api.development.push.apple.com/3/device/test', true],
472+
// Invalid: disallowed host
473+
['https://evil.example.com/push/endpoint', false],
474+
// Invalid: subdomain spoofing
475+
['https://evil.fcm.googleapis.com.attacker.com/push', false],
476+
// Invalid: not a URL
477+
['not_a_url', false],
478+
// Invalid: bare wildcard domain without subdomain
479+
['https://notify.windows.com/push', false],
480+
['https://push.apple.com/push', false],
481+
// Invalid: empty string
482+
['', false],
483+
];
484+
}
485+
486+
/**
487+
* @dataProvider data_is_valid_endpoint
488+
*/
489+
public function test_is_valid_endpoint(string $endpoint, bool $expected): void
490+
{
491+
$this->assertEquals($expected, $this->controller->is_valid_endpoint($endpoint));
492+
}
493+
423494
public function test_toggle_popup_enable_to_disable()
424495
{
425496
$this->form_helper->method('check_form_tokens')->willReturn(true);

‎ucp/controller/webpush.php‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,21 @@ class webpush
3636
/** @var string UCP form token name */
3737
public const FORM_TOKEN_UCP = 'ucp_webpush';
3838

39+
/** @var array Allowed push service endpoint hosts https://github.com/pushpad/known-push-services */
40+
public const PUSH_SERVICE_WHITELIST = [
41+
'android.googleapis.com',
42+
'fcm.googleapis.com',
43+
'updates.push.services.mozilla.com',
44+
'updates-autopush.stage.mozaws.net',
45+
'updates-autopush.dev.mozaws.net',
46+
];
47+
48+
/** @var array Allowed push service endpoint host wildcard suffixes (e.g. *.notify.windows.com) https://github.com/pushpad/known-push-services */
49+
public const PUSH_SERVICE_WILDCARD_SUFFIXES = [
50+
'.notify.windows.com',
51+
'.push.apple.com',
52+
];
53+
3954
/** @var config */
4055
protected $config;
4156

@@ -279,6 +294,37 @@ public function worker(): Response
279294
return $response;
280295
}
281296

297+
/**
298+
* Check if a push subscription endpoint belongs to an allowed push service
299+
*
300+
* @param string $endpoint The push subscription endpoint URL
301+
* @return bool True if the endpoint host matches a known/allowed push service
302+
*/
303+
public function is_valid_endpoint(string $endpoint): bool
304+
{
305+
$host = parse_url($endpoint, PHP_URL_HOST);
306+
307+
if (empty($host))
308+
{
309+
return false;
310+
}
311+
312+
if (in_array($host, self::PUSH_SERVICE_WHITELIST, true))
313+
{
314+
return true;
315+
}
316+
317+
foreach (self::PUSH_SERVICE_WILDCARD_SUFFIXES as $suffix)
318+
{
319+
if (substr($host, -strlen($suffix)) === $suffix)
320+
{
321+
return true;
322+
}
323+
}
324+
325+
return false;
326+
}
327+
282328
/**
283329
* Check (un)subscribe form for valid link hash
284330
*
@@ -312,6 +358,11 @@ public function subscribe(symfony_request $symfony_request): JsonResponse
312358

313359
$data = json_sanitizer::decode($symfony_request->get('data', ''));
314360

361+
if (!$this->is_valid_endpoint($data['endpoint']))
362+
{
363+
throw new http_exception(Response::HTTP_BAD_REQUEST, 'WEBPUSH_INVALID_ENDPOINT');
364+
}
365+
315366
$sql = 'INSERT INTO ' . $this->push_subscriptions_table . ' ' . $this->db->sql_build_array('INSERT', [
316367
'user_id' => $this->user->id(),
317368
'endpoint' => $data['endpoint'],

0 commit comments

Comments
 (0)