fix(backend): tighten an overly relaxed criteria and remove capability of matching multiple final URLs in URL authority checking (#15655)

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

3 files changed, 18 insertions, 12 deletions

diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts
index 13d8f7f43b..3ddfe52045 100644
--- a/packages/backend/src/core/HttpRequestService.ts
+++ b/packages/backend/src/core/HttpRequestService.ts
@@ -16,7 +16,7 @@ import type { Config } from '@/config.js';
 import { StatusError } from '@/misc/status-error.js';
 import { bindThis } from '@/decorators.js';
 import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
-import { assertActivityMatchesUrls, FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
+import { assertActivityMatchesUrl, FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
 import type { IObject } from '@/core/activitypub/type.js';
 import type { Response } from 'node-fetch';
 import type { URL } from 'node:url';
@@ -265,7 +265,7 @@ export class HttpRequestService {
 		const finalUrl = res.url; // redirects may have been involved
 		const activity = await res.json() as IObject;
 
-		assertActivityMatchesUrls(url, activity, [finalUrl], allowSoftfail);
+		assertActivityMatchesUrl(url, activity, finalUrl, allowSoftfail);
 
 		return activity;
 	}
diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts
index 6c29cce325..61d328ccac 100644
--- a/packages/backend/src/core/activitypub/ApRequestService.ts
+++ b/packages/backend/src/core/activitypub/ApRequestService.ts
@@ -17,7 +17,7 @@ import { LoggerService } from '@/core/LoggerService.js';
 import { bindThis } from '@/decorators.js';
 import type Logger from '@/logger.js';
 import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
-import { assertActivityMatchesUrls, FetchAllowSoftFailMask as FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
+import { assertActivityMatchesUrl, FetchAllowSoftFailMask as FetchAllowSoftFailMask } from '@/core/activitypub/misc/check-against-url.js';
 import type { IObject } from './type.js';
 
 type Request = {
@@ -258,7 +258,7 @@ export class ApRequestService {
 		const finalUrl = res.url; // redirects may have been involved
 		const activity = await res.json() as IObject;
 
-		assertActivityMatchesUrls(url, activity, [finalUrl], allowSoftfail);
+		assertActivityMatchesUrl(url, activity, finalUrl, allowSoftfail);
 
 		return activity;
 	}
diff --git a/packages/backend/src/core/activitypub/misc/check-against-url.ts b/packages/backend/src/core/activitypub/misc/check-against-url.ts
index dfcfb1943e..bbfe57f9fa 100644
--- a/packages/backend/src/core/activitypub/misc/check-against-url.ts
+++ b/packages/backend/src/core/activitypub/misc/check-against-url.ts
@@ -75,7 +75,7 @@ function normalizeSynonymousSubdomain(url: URL | string): URL {
 	return new URL(urlParsed.toString().replace(host, normalizedHost));
 }
 
-export function assertActivityMatchesUrls(requestUrl: string | URL, activity: IObject, candidateUrls: (string | URL)[], allowSoftfail: FetchAllowSoftFailMask): FetchAllowSoftFailMask {
+export function assertActivityMatchesUrl(requestUrl: string | URL, activity: IObject, finalUrl: string | URL, allowSoftfail: FetchAllowSoftFailMask): FetchAllowSoftFailMask {
 	// must have a unique identifier to verify authority
 	if (!activity.id) {
 		throw new Error('bad Activity: missing id field');
@@ -95,26 +95,32 @@ export function assertActivityMatchesUrls(requestUrl: string | URL, activity: IO
 	const requestUrlParsed = normalizeSynonymousSubdomain(requestUrl);
 	const idParsed = normalizeSynonymousSubdomain(activity.id);
 
-	const candidateUrlsParsed = candidateUrls.map(it => normalizeSynonymousSubdomain(it));
+	const finalUrlParsed = normalizeSynonymousSubdomain(finalUrl);
+
+	// mastodon sends activities with hash in the URL
+	// currently it only happens with likes, deletes etc.
+	// but object ID never has hash
+	requestUrlParsed.hash = '';
+	finalUrlParsed.hash = '';
 
 	const requestUrlSecure = requestUrlParsed.protocol === 'https:';
-	const finalUrlSecure = candidateUrlsParsed.every(it => it.protocol === 'https:');
+	const finalUrlSecure = finalUrlParsed.protocol === 'https:';
 	if (requestUrlSecure && !finalUrlSecure) {
 		throw new Error(`bad Activity: id(${activity.id}) is not allowed to have http:// in the url`);
 	}
 
 	// Compare final URL to the ID
-	if (!candidateUrlsParsed.some(it => it.href === idParsed.href)) {
-		requireSoftfail(FetchAllowSoftFailMask.NonCanonicalId, `bad Activity: id(${activity.id}) does not match response url(${candidateUrlsParsed.map(it => it.toString())})`);
+	if (finalUrlParsed.href !== idParsed.href) {
+		requireSoftfail(FetchAllowSoftFailMask.NonCanonicalId, `bad Activity: id(${activity.id}) does not match response url(${finalUrlParsed.toString()})`);
 
 		// at lease host need to match exactly (ActivityPub requirement)
-		if (!candidateUrlsParsed.some(it => idParsed.host === it.host)) {
-			throw new Error(`bad Activity: id(${activity.id}) does not match response host(${candidateUrlsParsed.map(it => it.host)})`);
+		if (idParsed.host !== finalUrlParsed.host) {
+			throw new Error(`bad Activity: id(${activity.id}) does not match response host(${finalUrlParsed.host})`);
 		}
 	}
 
 	// Compare request URL to the ID
-	if (!requestUrlParsed.href.includes(idParsed.href)) {
+	if (requestUrlParsed.href !== idParsed.href) {
 		requireSoftfail(FetchAllowSoftFailMask.NonCanonicalId, `bad Activity: id(${activity.id}) does not match request url(${requestUrlParsed.toString()})`);
 
 		// if cross-origin lookup is allowed, we can accept some variation between the original request URL to the final object ID (but not between the final URL and the object ID)