From 982223ad38e428ca4e2269fff56bccd332ca0222 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Fri, 4 Jul 2025 12:16:18 -0400 Subject: [PATCH] validate all URLs before fetch --- .../backend/src/core/HttpRequestService.ts | 8 ++- packages/backend/src/core/UtilityService.ts | 59 ++++++++++++++++- .../src/core/activitypub/ApRequestService.ts | 4 -- .../src/core/activitypub/ApUtilityService.ts | 63 +++++-------------- .../core/activitypub/models/ApNoteService.ts | 2 +- .../activitypub/models/ApPersonService.ts | 8 +-- .../unit/core/activitypub/ApUtilityService.ts | 30 ++++++--- 7 files changed, 101 insertions(+), 73 deletions(-) diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 151097095d..046b0dc244 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -17,7 +17,8 @@ import { StatusError } from '@/misc/status-error.js'; import { bindThis } from '@/decorators.js'; import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js'; import type { IObject, IObjectWithId } from '@/core/activitypub/type.js'; -import { ApUtilityService } from './activitypub/ApUtilityService.js'; +import { UtilityService } from '@/core/UtilityService.js'; +import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js'; import type { Response } from 'node-fetch'; import type { URL } from 'node:url'; import type { Socket } from 'node:net'; @@ -132,6 +133,7 @@ export class HttpRequestService { @Inject(DI.config) private config: Config, private readonly apUtilityService: ApUtilityService, + private readonly utilityService: UtilityService, ) { const cache = new CacheableLookup({ maxTtl: 3600, // 1hours @@ -236,8 +238,6 @@ export class HttpRequestService { @bindThis public async getActivityJson(url: string, isLocalAddressAllowed = false, allowAnonymous = false): Promise { - this.apUtilityService.assertApUrl(url); - const res = await this.send(url, { method: 'GET', headers: { @@ -311,6 +311,8 @@ export class HttpRequestService { ): Promise { const timeout = args.timeout ?? 5000; + this.utilityService.assertUrl(url); + const controller = new AbortController(); setTimeout(() => { controller.abort(); diff --git a/packages/backend/src/core/UtilityService.ts b/packages/backend/src/core/UtilityService.ts index 3098367392..281edccca3 100644 --- a/packages/backend/src/core/UtilityService.ts +++ b/packages/backend/src/core/UtilityService.ts @@ -10,7 +10,10 @@ import psl from 'psl'; import { DI } from '@/di-symbols.js'; import type { Config } from '@/config.js'; import { bindThis } from '@/decorators.js'; -import { MiMeta } from '@/models/Meta.js'; +import { MiMeta, SoftwareSuspension } from '@/models/Meta.js'; +import { MiInstance } from '@/models/Instance.js'; +import { IdentifiableError } from '@/misc/identifiable-error.js'; +import { EnvService } from '@/core/EnvService.js'; @Injectable() export class UtilityService { @@ -20,6 +23,8 @@ export class UtilityService { @Inject(DI.meta) private meta: MiMeta, + + private readonly envService: EnvService, ) { } @@ -181,8 +186,8 @@ export class UtilityService { } @bindThis - public punyHostPSLDomain(url: string): string { - const urlObj = new URL(url); + public punyHostPSLDomain(url: string | URL): string { + const urlObj = typeof(url) === 'object' ? url : new URL(url); const hostname = urlObj.hostname; const domain = this.specialSuffix(hostname) ?? psl.get(hostname) ?? hostname; const host = `${this.toPuny(domain)}${urlObj.port.length > 0 ? ':' + urlObj.port : ''}`; @@ -213,4 +218,52 @@ export class UtilityService { return ''; } } + + /** + * Verifies that a provided URL is in a format acceptable for federation. + * @throws {IdentifiableError} If URL cannot be parsed + * @throws {IdentifiableError} If URL is not HTTPS + * @throws {IdentifiableError} If URL contains credentials + */ + @bindThis + public assertUrl(url: string | URL): URL | never { + // If string, parse and validate + if (typeof(url) === 'string') { + try { + url = new URL(url); + } catch { + throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid url ${url}: not a valid URL`); + } + } + + // Must be HTTPS + if (!this.checkHttps(url)) { + throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid url ${url}: unsupported protocol ${url.protocol}`); + } + + // Must not have credentials + if (url.username || url.password) { + throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid url ${url}: contains embedded credentials`); + } + + return url; + } + + /** + * Checks if the URL contains HTTPS. + * Additionally, allows HTTP in non-production environments. + * Based on check-https.ts. + */ + @bindThis + public checkHttps(url: string | URL): boolean { + const isNonProd = this.envService.env.NODE_ENV !== 'production'; + + try { + const proto = new URL(url).protocol; + return proto === 'https:' || (proto === 'http:' && isNonProd); + } catch { + // Invalid URLs don't "count" as HTTPS + return false; + } + } } diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts index e4db9b237c..7669ce9669 100644 --- a/packages/backend/src/core/activitypub/ApRequestService.ts +++ b/packages/backend/src/core/activitypub/ApRequestService.ts @@ -157,8 +157,6 @@ export class ApRequestService { @bindThis public async signedPost(user: { id: MiUser['id'] }, url: string, object: unknown, digest?: string): Promise { - this.apUtilityService.assertApUrl(url); - const body = typeof object === 'string' ? object : JSON.stringify(object); const keypair = await this.userKeypairService.getUserKeypair(user.id); @@ -191,8 +189,6 @@ export class ApRequestService { */ @bindThis public async signedGet(url: string, user: { id: MiUser['id'] }, allowAnonymous = false, followAlternate?: boolean): Promise { - this.apUtilityService.assertApUrl(url); - const _followAlternate = followAlternate ?? true; const keypair = await this.userKeypairService.getUserKeypair(user.id); diff --git a/packages/backend/src/core/activitypub/ApUtilityService.ts b/packages/backend/src/core/activitypub/ApUtilityService.ts index 227dc3b9b3..eede48f0c6 100644 --- a/packages/backend/src/core/activitypub/ApUtilityService.ts +++ b/packages/backend/src/core/activitypub/ApUtilityService.ts @@ -7,14 +7,12 @@ import { Injectable } from '@nestjs/common'; import { UtilityService } from '@/core/UtilityService.js'; import { IdentifiableError } from '@/misc/identifiable-error.js'; import { toArray } from '@/misc/prelude/array.js'; -import { EnvService } from '@/core/EnvService.js'; -import { getApId, getOneApHrefNullable, IObject } from './type.js'; +import { getApId, getOneApHrefNullable, IObject } from '@/core/activitypub/type.js'; @Injectable() export class ApUtilityService { constructor( private readonly utilityService: UtilityService, - private readonly envService: EnvService, ) {} /** @@ -39,8 +37,11 @@ export class ApUtilityService { public haveSameAuthority(url1: string, url2: string): boolean { if (url1 === url2) return true; - const authority1 = this.utilityService.punyHostPSLDomain(url1); - const authority2 = this.utilityService.punyHostPSLDomain(url2); + const parsed1 = this.utilityService.assertUrl(url1); + const parsed2 = this.utilityService.assertUrl(url2); + + const authority1 = this.utilityService.punyHostPSLDomain(parsed1); + const authority2 = this.utilityService.punyHostPSLDomain(parsed2); return authority1 === authority2; } @@ -63,12 +64,16 @@ export class ApUtilityService { : undefined, })) .filter(({ url, type }) => { - if (!url) return false; - if (!this.checkHttps(url)) return false; - if (!isAcceptableUrlType(type)) return false; + try { + if (!url) return false; + if (!isAcceptableUrlType(type)) return false; + const parsed = this.utilityService.assertUrl(url); - const urlAuthority = this.utilityService.punyHostPSLDomain(url); - return urlAuthority === targetAuthority; + const urlAuthority = this.utilityService.punyHostPSLDomain(parsed); + return urlAuthority === targetAuthority; + } catch { + return false; + } }) .sort((a, b) => { return rankUrlType(a.type) - rankUrlType(b.type); @@ -76,44 +81,6 @@ export class ApUtilityService { return acceptableUrls[0]?.url ?? null; } - - /** - * Verifies that a provided URL is in a format acceptable for federation. - * @throws {IdentifiableError} If URL cannot be parsed - * @throws {IdentifiableError} If URL is not HTTPS - */ - public assertApUrl(url: string | URL): void { - // If string, parse and validate - if (typeof(url) === 'string') { - try { - url = new URL(url); - } catch { - throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid AP url ${url}: not a valid URL`); - } - } - - // Must be HTTPS - if (!this.checkHttps(url)) { - throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid AP url ${url}: unsupported protocol ${url.protocol}`); - } - } - - /** - * Checks if the URL contains HTTPS. - * Additionally, allows HTTP in non-production environments. - * Based on check-https.ts. - */ - private checkHttps(url: string | URL): boolean { - const isNonProd = this.envService.env.NODE_ENV !== 'production'; - - try { - const proto = new URL(url).protocol; - return proto === 'https:' || (proto === 'http:' && isNonProd); - } catch { - // Invalid URLs don't "count" as HTTPS - return false; - } - } } function isAcceptableUrlType(type: string | undefined): boolean { diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts index 2a28405121..a67b02a9dc 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -96,7 +96,7 @@ export class ApNoteService { actor?: MiRemoteUser, user?: MiRemoteUser, ): Error | null { - this.apUtilityService.assertApUrl(uri); + this.utilityService.assertUrl(uri); const expectHost = this.utilityService.extractDbHost(uri); const apType = getApType(object); diff --git a/packages/backend/src/core/activitypub/models/ApPersonService.ts b/packages/backend/src/core/activitypub/models/ApPersonService.ts index 29f7459219..b715d90a21 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -155,7 +155,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { */ @bindThis private validateActor(x: IObject, uri: string): IActor { - this.apUtilityService.assertApUrl(uri); + this.utilityService.assertUrl(uri); const expectHost = this.utilityService.punyHostPSLDomain(uri); if (!isActor(x)) { @@ -170,7 +170,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { throw new UnrecoverableError(`invalid Actor ${uri}: wrong inbox type`); } - this.apUtilityService.assertApUrl(x.inbox); + this.utilityService.assertUrl(x.inbox); const inboxHost = this.utilityService.punyHostPSLDomain(x.inbox); if (inboxHost !== expectHost) { throw new UnrecoverableError(`invalid Actor ${uri}: wrong inbox host ${inboxHost}`); @@ -179,7 +179,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { const sharedInboxObject = x.sharedInbox ?? (x.endpoints ? x.endpoints.sharedInbox : undefined); if (sharedInboxObject != null) { const sharedInbox = getApId(sharedInboxObject); - this.apUtilityService.assertApUrl(sharedInbox); + this.utilityService.assertUrl(sharedInbox); if (!(typeof sharedInbox === 'string' && sharedInbox.length > 0 && this.utilityService.punyHostPSLDomain(sharedInbox) === expectHost)) { throw new UnrecoverableError(`invalid Actor ${uri}: wrong shared inbox ${sharedInbox}`); } @@ -190,7 +190,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { if (xCollection != null) { const collectionUri = getApId(xCollection); if (typeof collectionUri === 'string' && collectionUri.length > 0) { - this.apUtilityService.assertApUrl(collectionUri); + this.utilityService.assertUrl(collectionUri); if (this.utilityService.punyHostPSLDomain(collectionUri) !== expectHost) { throw new UnrecoverableError(`invalid Actor ${uri}: wrong ${collection} host ${collectionUri}`); } diff --git a/packages/backend/test/unit/core/activitypub/ApUtilityService.ts b/packages/backend/test/unit/core/activitypub/ApUtilityService.ts index 325a94dc5a..a49ba35112 100644 --- a/packages/backend/test/unit/core/activitypub/ApUtilityService.ts +++ b/packages/backend/test/unit/core/activitypub/ApUtilityService.ts @@ -3,30 +3,40 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import type { UtilityService } from '@/core/UtilityService.js'; import type { IObject } from '@/core/activitypub/type.js'; import type { EnvService } from '@/core/EnvService.js'; +import type { MiMeta } from '@/models/Meta.js'; +import type { Config } from '@/config.js'; import { ApUtilityService } from '@/core/activitypub/ApUtilityService.js'; +import { UtilityService } from '@/core/UtilityService.js'; describe(ApUtilityService, () => { let serviceUnderTest: ApUtilityService; let env: Record; beforeEach(() => { - const utilityService = { - punyHostPSLDomain(input: string) { - const host = new URL(input).host; - const parts = host.split('.'); - return `${parts[parts.length - 2]}.${parts[parts.length - 1]}`; - }, - } as unknown as UtilityService; - env = {}; const envService = { env, } as unknown as EnvService; - serviceUnderTest = new ApUtilityService(utilityService, envService); + const config = { + host: 'example.com', + blockedHosts: [], + silencedHosts: [], + mediaSilencedHosts: [], + federationHosts: [], + bubbleInstances: [], + deliverSuspendedSoftware: [], + federation: 'all', + } as unknown as Config; + const meta = { + + } as MiMeta; + + const utilityService = new UtilityService(config, meta, envService); + + serviceUnderTest = new ApUtilityService(utilityService); }); describe('assertIdMatchesUrlAuthority', () => {