From 0a8ffd9cfa8c374b0dfb4ae3c92a416cb79360b2 Mon Sep 17 00:00:00 2001
From: tamaina <tamaina@hotmail.co.jp>
Date: Sat, 17 Feb 2024 12:41:19 +0900
Subject: [PATCH] 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;
---
.../backend/src/core/HttpRequestService.ts | 55 +++++++++++++++----
.../src/core/activitypub/ApRequestService.ts | 6 +-
.../src/core/activitypub/ApResolverService.ts | 2 +-
.../core/activitypub/LdSignatureService.ts | 6 +-
.../src/core/activitypub/misc/validator.ts | 39 +++++++++++++
.../test/e2e/fetch-validate-ap-deny.ts | 40 ++++++++++++++
packages/backend/test/unit/activitypub.ts | 2 +-
packages/backend/test/utils.ts | 36 +++++++-----
8 files changed, 157 insertions(+), 29 deletions(-)
create mode 100644 packages/backend/src/core/activitypub/misc/validator.ts
create mode 100644 packages/backend/test/e2e/fetch-validate-ap-deny.ts
diff --git a/packages/backend/src/core/HttpRequestService.ts b/packages/backend/src/core/HttpRequestService.ts
index 73bb3dc7e9..1352e137ce 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<IObject> {
+ 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<T = unknown>(url: string, accept = 'application/json, */*', headers?: Record<string, string>): Promise<T> {
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<string, string>,
- timeout?: number,
- size?: number,
- } = {}, extra: {
- throwErrorWhenResponseNotOk: boolean;
- } = {
- throwErrorWhenResponseNotOk: true,
- }): Promise<Response> {
+ public async send(
+ url: string,
+ args: {
+ method?: string,
+ body?: string,
+ headers?: Record<string, string>,
+ timeout?: number,
+ size?: number,
+ } = {},
+ extra: HttpRequestSendOptions = {
+ throwErrorWhenResponseNotOk: true,
+ validators: [],
+ },
+ ): Promise<Response> {
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 e165c5e960..db3123bfa5 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 9ca63c9ec5..870cf6372a 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 39b5ff8abc..d8464b3839 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');
+}
diff --git a/packages/backend/test/e2e/fetch-validate-ap-deny.ts b/packages/backend/test/e2e/fetch-validate-ap-deny.ts
new file mode 100644
index 0000000000..434a9fe209
--- /dev/null
+++ b/packages/backend/test/e2e/fetch-validate-ap-deny.ts
@@ -0,0 +1,40 @@
+/*
+ * SPDX-FileCopyrightText: syuilo and misskey-project
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+process.env.NODE_ENV = 'test';
+
+import { validateContentTypeSetAsActivityPub, validateContentTypeSetAsJsonLD } from '@/core/activitypub/misc/validator.js';
+import { signup, uploadFile, relativeFetch } from '../utils.js';
+import type * as misskey from 'misskey-js';
+
+describe('validateContentTypeSetAsActivityPub/JsonLD (deny case)', () => {
+ let alice: misskey.entities.SignupResponse;
+ let aliceUploadedFile: any;
+
+ beforeAll(async () => {
+ alice = await signup({ username: 'alice' });
+ aliceUploadedFile = await uploadFile(alice);
+ }, 1000 * 60 * 2);
+
+ test('ActivityStreams: ファイルはエラーになる', async () => {
+ const res = await relativeFetch(aliceUploadedFile.webpublicUrl);
+
+ function doValidate() {
+ validateContentTypeSetAsActivityPub(res);
+ }
+
+ expect(doValidate).toThrow('Content type is not');
+ });
+
+ test('JSON-LD: ファイルはエラーになる', async () => {
+ const res = await relativeFetch(aliceUploadedFile.webpublicUrl);
+
+ function doValidate() {
+ validateContentTypeSetAsJsonLD(res);
+ }
+
+ expect(doValidate).toThrow('Content type is not');
+ });
+});
diff --git a/packages/backend/test/unit/activitypub.ts b/packages/backend/test/unit/activitypub.ts
index 4b9b0cd3d1..014e862f9c 100644
--- a/packages/backend/test/unit/activitypub.ts
+++ b/packages/backend/test/unit/activitypub.ts
@@ -202,7 +202,7 @@ describe('ActivityPub', () => {
describe('Renderer', () => {
test('Render an announce with visibility: followers', () => {
- rendererService.renderAnnounce(null, {
+ rendererService.renderAnnounce('https://example.com/notes/00example', {
id: genAidx(Date.now()),
visibility: 'followers',
} as MiNote);
diff --git a/packages/backend/test/utils.ts b/packages/backend/test/utils.ts
index a41002cc8c..22c5cec07a 100644
--- a/packages/backend/test/utils.ts
+++ b/packages/backend/test/utils.ts
@@ -13,10 +13,11 @@ import fetch, { File, RequestInit } from 'node-fetch';
import { DataSource } from 'typeorm';
import { JSDOM } from 'jsdom';
import { DEFAULT_POLICIES } from '@/core/RoleService.js';
+import { Packed } from '@/misc/json-schema.js';
+import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
import { entities } from '../src/postgres.js';
import { loadConfig } from '../src/config.js';
import type * as misskey from 'misskey-js';
-import { Packed } from '@/misc/json-schema.js';
export { server as startServer, jobQueue as startJobQueue } from '@/boot/common.js';
@@ -123,9 +124,9 @@ export function randomString(chars = 'abcdefghijklmnopqrstuvwxyz0123456789', len
function timeoutPromise<T>(p: Promise<T>, timeout: number): Promise<T> {
return Promise.race([
p,
- new Promise((reject) =>{
- setTimeout(() => { reject(new Error('timed out')); }, timeout)
- }) as never
+ new Promise((reject) => {
+ setTimeout(() => { reject(new Error('timed out')); }, timeout);
+ }) as never,
]);
}
@@ -327,7 +328,6 @@ export const uploadFile = async (user?: UserToken, { path, name, blob }: UploadO
});
const body = res.status !== 204 ? await res.json() as misskey.Endpoints['drive/files/create']['res'] : null;
-
return {
status: res.status,
headers: res.headers,
@@ -343,7 +343,7 @@ export const uploadUrl = async (user: UserToken, url: string): Promise<Packed<'D
'main',
(msg) => msg.type === 'urlUploadFinished' && msg.body.marker === marker,
(msg) => msg.body.file as Packed<'DriveFile'>,
- 60 * 1000
+ 60 * 1000,
);
await api('drive/files/upload-from-url', {
@@ -434,20 +434,20 @@ export const waitFire = async (user: UserToken, channel: string, trgr: () => any
* @returns 時間内に正常に処理できた場合に通知からextractorを通した値を得る
*/
export function makeStreamCatcher<T>(
- user: UserToken,
- channel: string,
- cond: (message: Record<string, any>) => boolean,
- extractor: (message: Record<string, any>) => T,
- timeout = 60 * 1000): Promise<T> {
- let ws: WebSocket
+ user: UserToken,
+ channel: string,
+ cond: (message: Record<string, any>) => boolean,
+ extractor: (message: Record<string, any>) => T,
+ timeout = 60 * 1000): Promise<T> {
+ let ws: WebSocket;
const p = new Promise<T>(async (resolve) => {
ws = await connectStream(user, channel, (msg) => {
if (cond(msg)) {
- resolve(extractor(msg))
+ resolve(extractor(msg));
}
});
}).finally(() => {
- ws?.close();
+ ws.close();
});
return timeoutPromise(p, timeout);
@@ -476,6 +476,14 @@ export const simpleGet = async (path: string, accept = '*/*', cookie: any = unde
'text/html; charset=utf-8',
];
+ if (res.ok && (
+ accept.startsWith('application/activity+json') ||
+ (accept.startsWith('application/ld+json') && accept.includes('https://www.w3.org/ns/activitystreams'))
+ )) {
+ // validateContentTypeSetAsActivityPubのテストを兼ねる
+ validateContentTypeSetAsActivityPub(res);
+ }
+
const body =
jsonTypes.includes(res.headers.get('content-type') ?? '') ? await res.json() :
htmlTypes.includes(res.headers.get('content-type') ?? '') ? new JSDOM(await res.text()) :