From 9a70ce8f5ea9df00001894809f5ce7bc69b14c8a Mon Sep 17 00:00:00 2001 From: tamaina Date: Sat, 17 Feb 2024 12:41:19 +0900 Subject: Merge pull request from GHSA-qqrm-9grj-6v32 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * maybe ok * fix * test wip * :v: * fix * if (res.ok) * validateContentTypeSetAsJsonLD * 条件を考慮し直す * その他の+json接尾辞が付いているメディアタイプも受け容れる * https://github.com/misskey-dev/misskey-ghsa-qqrm-9grj-6v32/pull/1#discussion_r1490999009 * add `; profile="https://www.w3.org/ns/activitystreams"` * application/ld+json; --- packages/backend/src/core/HttpRequestService.ts | 55 +++++++++++++++++----- .../src/core/activitypub/ApRequestService.ts | 6 ++- .../src/core/activitypub/ApResolverService.ts | 2 +- .../src/core/activitypub/LdSignatureService.ts | 6 ++- .../backend/src/core/activitypub/misc/validator.ts | 39 +++++++++++++++ 5 files changed, 94 insertions(+), 14 deletions(-) create mode 100644 packages/backend/src/core/activitypub/misc/validator.ts (limited to 'packages/backend/src') diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts index b36b9f6e3c..7f3cac7c58 100644 --- a/packages/backend/src/core/HttpRequestService.ts +++ b/packages/backend/src/core/HttpRequestService.ts @@ -14,9 +14,16 @@ import { DI } from '@/di-symbols.js'; 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 type { IObject } from '@/core/activitypub/type.js'; import type { Response } from 'node-fetch'; import type { URL } from 'node:url'; +export type HttpRequestSendOptions = { + throwErrorWhenResponseNotOk: boolean; + validators?: ((res: Response) => void)[]; +}; + @Injectable() export class HttpRequestService { /** @@ -104,6 +111,23 @@ export class HttpRequestService { } } + @bindThis + public async getActivityJson(url: string): Promise { + const res = await this.send(url, { + method: 'GET', + headers: { + Accept: 'application/activity+json, application/ld+json; profile="https://www.w3.org/ns/activitystreams"', + }, + timeout: 5000, + size: 1024 * 256, + }, { + throwErrorWhenResponseNotOk: true, + validators: [validateContentTypeSetAsActivityPub], + }); + + return await res.json() as IObject; + } + @bindThis public async getJson(url: string, accept = 'application/json, */*', headers?: Record): Promise { const res = await this.send(url, { @@ -132,17 +156,20 @@ export class HttpRequestService { } @bindThis - public async send(url: string, args: { - method?: string, - body?: string, - headers?: Record, - timeout?: number, - size?: number, - } = {}, extra: { - throwErrorWhenResponseNotOk: boolean; - } = { - throwErrorWhenResponseNotOk: true, - }): Promise { + public async send( + url: string, + args: { + method?: string, + body?: string, + headers?: Record, + timeout?: number, + size?: number, + } = {}, + extra: HttpRequestSendOptions = { + throwErrorWhenResponseNotOk: true, + validators: [], + }, + ): Promise { const timeout = args.timeout ?? 5000; const controller = new AbortController(); @@ -166,6 +193,12 @@ export class HttpRequestService { throw new StatusError(`${res.status} ${res.statusText}`, res.status, res.statusText); } + if (res.ok) { + for (const validator of (extra.validators ?? [])) { + validator(res); + } + } + return res; } } diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts index 202e07814e..93ac8ce9a7 100644 --- a/packages/backend/src/core/activitypub/ApRequestService.ts +++ b/packages/backend/src/core/activitypub/ApRequestService.ts @@ -14,6 +14,7 @@ import { HttpRequestService } from '@/core/HttpRequestService.js'; 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'; type Request = { url: string; @@ -70,7 +71,7 @@ export class ApRequestCreator { url: u.href, method: 'GET', headers: this.#objectAssignWithLcKey({ - 'Accept': 'application/activity+json, application/ld+json', + 'Accept': 'application/activity+json, application/ld+json; profile="https://www.w3.org/ns/activitystreams"', 'Date': new Date().toUTCString(), 'Host': new URL(args.url).host, }, args.additionalHeaders), @@ -195,6 +196,9 @@ export class ApRequestService { const res = await this.httpRequestService.send(url, { method: req.request.method, headers: req.request.headers, + }, { + throwErrorWhenResponseNotOk: true, + validators: [validateContentTypeSetAsActivityPub], }); return await res.json(); diff --git a/packages/backend/src/core/activitypub/ApResolverService.ts b/packages/backend/src/core/activitypub/ApResolverService.ts index db44c042e7..bb3c40f093 100644 --- a/packages/backend/src/core/activitypub/ApResolverService.ts +++ b/packages/backend/src/core/activitypub/ApResolverService.ts @@ -105,7 +105,7 @@ export class Resolver { const object = (this.user ? await this.apRequestService.signedGet(value, this.user) as IObject - : await this.httpRequestService.getJson(value, 'application/activity+json, application/ld+json')) as IObject; + : await this.httpRequestService.getActivityJson(value)) as IObject; if ( Array.isArray(object['@context']) ? diff --git a/packages/backend/src/core/activitypub/LdSignatureService.ts b/packages/backend/src/core/activitypub/LdSignatureService.ts index f958e9d16e..9de184336f 100644 --- a/packages/backend/src/core/activitypub/LdSignatureService.ts +++ b/packages/backend/src/core/activitypub/LdSignatureService.ts @@ -8,6 +8,7 @@ import { Injectable } from '@nestjs/common'; import { HttpRequestService } from '@/core/HttpRequestService.js'; import { bindThis } from '@/decorators.js'; import { CONTEXTS } from './misc/contexts.js'; +import { validateContentTypeSetAsJsonLD } from './misc/validator.js'; import type { JsonLdDocument } from 'jsonld'; import type { JsonLd, RemoteDocument } from 'jsonld/jsonld-spec.js'; @@ -133,7 +134,10 @@ class LdSignature { }, timeout: this.loderTimeout, }, - { throwErrorWhenResponseNotOk: false }, + { + throwErrorWhenResponseNotOk: false, + validators: [validateContentTypeSetAsJsonLD], + }, ).then(res => { if (!res.ok) { throw new Error(`${res.status} ${res.statusText}`); diff --git a/packages/backend/src/core/activitypub/misc/validator.ts b/packages/backend/src/core/activitypub/misc/validator.ts new file mode 100644 index 0000000000..6ba14a222f --- /dev/null +++ b/packages/backend/src/core/activitypub/misc/validator.ts @@ -0,0 +1,39 @@ +/* + * SPDX-FileCopyrightText: syuilo and misskey-project + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import type { Response } from 'node-fetch'; + +export function validateContentTypeSetAsActivityPub(response: Response): void { + const contentType = (response.headers.get('content-type') ?? '').toLowerCase(); + + if (contentType === '') { + throw new Error('Validate content type of AP response: No content-type header'); + } + if ( + contentType.startsWith('application/activity+json') || + (contentType.startsWith('application/ld+json;') && contentType.includes('https://www.w3.org/ns/activitystreams')) + ) { + return; + } + throw new Error('Validate content type of AP response: Content type is not application/activity+json or application/ld+json'); +} + +const plusJsonSuffixRegex = /(application|text)\/[a-zA-Z0-9\.\-\+]+\+json/; + +export function validateContentTypeSetAsJsonLD(response: Response): void { + const contentType = (response.headers.get('content-type') ?? '').toLowerCase(); + + if (contentType === '') { + throw new Error('Validate content type of JSON LD: No content-type header'); + } + if ( + contentType.startsWith('application/ld+json') || + contentType.startsWith('application/json') || + plusJsonSuffixRegex.test(contentType) + ) { + return; + } + throw new Error('Validate content type of JSON LD: Content type is not application/ld+json or application/json'); +} -- cgit v1.2.3-freya