From 520e9d242c78ef960c5811310a1f6afd63ad5f61 Mon Sep 17 00:00:00 2001
From: palmoni5 <d8401948@gmail.com>
Date: Thu, 27 Aug 2026 19:43:14 +0300
Subject: [PATCH] fix: validate subscription endpoint before persisting it

`Subscriptions.add` stored `subscription.endpoint` exactly as received.
The endpoint is caller-supplied, and `plugin.onNotificationPush` later
POSTs to it for every notification the user receives, so an authenticated
user could register an arbitrary url and have the server request it
repeatedly on their behalf.

`POST /api/web-push/test` already runs `request.check()` before sending,
but nothing guarded the write path, so the regular push path had no check
at all.

Reject the endpoint at `add()` unless it parses as a url, uses https, and
passes `request.check()` (core's reserved-IP-range/DNS-rebinding guard).
Also trim the endpoint on both add and remove so the sorted set member
and the object key stay in sync with what the test route looks up.
---
 lib/subscriptions.js | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/lib/subscriptions.js b/lib/subscriptions.js
index 07b70b4..40f5a64 100644
--- a/lib/subscriptions.js
+++ b/lib/subscriptions.js
@@ -1,9 +1,32 @@
 'use strict';
 
 const db = nodebb.require('./src/database');
+const request = nodebb.require('./src/request');
 
 const Subscriptions = module.exports;
 
+// The endpoint is caller-supplied and is later POSTed to by the server on every
+// notification, so it has to be checked before it is persisted.
+Subscriptions.isValidEndpoint = async (endpoint) => {
+	let url;
+	try {
+		url = new URL(endpoint);
+	} catch (e) {
+		return false;
+	}
+
+	if (url.protocol !== 'https:') {
+		return false;
+	}
+
+	try {
+		const { ok } = await request.check(url.href);
+		return ok;
+	} catch (e) {
+		return false;
+	}
+};
+
 Subscriptions.count = async uid => await db.sortedSetCard(`uid:${uid}:web-push:subscriptions`);
 
 Subscriptions.getUsers = async () => {
@@ -32,8 +55,12 @@ Subscriptions.list = async (uids) => {
 };
 
 Subscriptions.add = async (uid, subscription, device) => {
-	const { endpoint } = subscription;
-	const entry = { ...subscription, ...(device || {}) };
+	const endpoint = typeof subscription?.endpoint === 'string' ? subscription.endpoint.trim() : '';
+	if (!await Subscriptions.isValidEndpoint(endpoint)) {
+		throw new Error('[[error:invalid-url]]');
+	}
+
+	const entry = { ...subscription, ...(device || {}), endpoint };
 	await Promise.all([
 		db.sortedSetAdd(`uid:${uid}:web-push:subscriptions`, Date.now(), endpoint),
 		db.setObject(`web-push:subscriptions:${endpoint}`, entry),
@@ -42,7 +69,7 @@ Subscriptions.add = async (uid, subscription, device) => {
 };
 
 Subscriptions.remove = async (uid, subscription) => {
-	const { endpoint } = subscription;
+	const endpoint = typeof subscription?.endpoint === 'string' ? subscription.endpoint.trim() : '';
 	await Promise.all([
 		db.sortedSetRemove(`uid:${uid}:web-push:subscriptions`, endpoint),
 		db.delete(`web-push:subscriptions:${endpoint}`),