diff options
5 files changed, 46 insertions, 3 deletions
diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index 2951691129..5c271b81e3 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -236,6 +236,8 @@ export class HttpRequestService { @bindThis public async getActivityJson(url: string, isLocalAddressAllowed = false): Promise<IObjectWithId> { + this.apUtilityService.assertApUrl(url); + const res = await this.send(url, { method: 'GET', headers: { diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts index 7118ce1e02..b665b51700 100644 --- a/packages/backend/src/core/activitypub/ApRequestService.ts +++ b/packages/backend/src/core/activitypub/ApRequestService.ts @@ -155,6 +155,8 @@ export class ApRequestService { @bindThis public async signedPost(user: { id: MiUser['id'] }, url: string, object: unknown, digest?: string): Promise<void> { + this.apUtilityService.assertApUrl(url); + const body = typeof object === 'string' ? object : JSON.stringify(object); const keypair = await this.userKeypairService.getUserKeypair(user.id); @@ -186,6 +188,8 @@ export class ApRequestService { */ @bindThis public async signedGet(url: string, user: { id: MiUser['id'] }, followAlternate?: boolean): Promise<IObjectWithId> { + 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 ae6e4997e4..c3958cdf42 100644 --- a/packages/backend/src/core/activitypub/ApUtilityService.ts +++ b/packages/backend/src/core/activitypub/ApUtilityService.ts @@ -78,15 +78,47 @@ export class ApUtilityService { } /** + * Verifies that a provided URL is in a format acceptable for federation. + * @throws {IdentifiableError} If URL cannot be parsed + * @throws {IdentifiableError} If URL contains a fragment + * @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`); + } + } + + // Hash component breaks federation + if (url.hash) { + throw new IdentifiableError('0bedd29b-e3bf-4604-af51-d3352e2518af', `invalid AP url ${url}: contains a fragment (#)`); + } + + // 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): boolean { + private checkHttps(url: string | URL): boolean { const isNonProd = this.envService.env.NODE_ENV !== 'production'; - // noinspection HttpUrlsUsage - return url.startsWith('https://') || (url.startsWith('http://') && isNonProd); + 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/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts index f6152e3888..7811b81795 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -95,6 +95,7 @@ export class ApNoteService { actor?: MiRemoteUser, user?: MiRemoteUser, ): Error | null { + this.apUtilityService.assertApUrl(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 4b685f7e1b..5c6716a0b8 100644 --- a/packages/backend/src/core/activitypub/models/ApPersonService.ts +++ b/packages/backend/src/core/activitypub/models/ApPersonService.ts @@ -153,6 +153,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { */ @bindThis private validateActor(x: IObject, uri: string): IActor { + this.apUtilityService.assertApUrl(uri); const expectHost = this.utilityService.punyHostPSLDomain(uri); if (!isActor(x)) { @@ -167,6 +168,7 @@ export class ApPersonService implements OnModuleInit, OnApplicationShutdown { throw new UnrecoverableError(`invalid Actor ${uri} - wrong inbox type`); } + this.apUtilityService.assertApUrl(x.inbox); const inboxHost = this.utilityService.punyHostPSLDomain(x.inbox); if (inboxHost !== expectHost) { throw new UnrecoverableError(`invalid Actor ${uri} - wrong inbox ${inboxHost}`); @@ -175,6 +177,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); if (!(typeof sharedInbox === 'string' && sharedInbox.length > 0 && this.utilityService.punyHostPSLDomain(sharedInbox) === expectHost)) { throw new UnrecoverableError(`invalid Actor ${uri} - wrong shared inbox ${sharedInbox}`); } @@ -185,6 +188,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); if (this.utilityService.punyHostPSLDomain(collectionUri) !== expectHost) { throw new UnrecoverableError(`invalid Actor ${uri} - wrong ${collection} ${collectionUri}`); } |