merge: ssrf fix for our develop (!426)

View MR for information: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/426

Approved-by: Leah <kevinlukej@gmail.com>
Approved-by: Amelia Yukii <amelia.yukii@shourai.de>
This commit is contained in:
Amelia Yukii 2024-02-17 13:15:50 +00:00
commit 7b7de4cdb3
8 changed files with 157 additions and 29 deletions

View file

@ -14,9 +14,16 @@ import { DI } from '@/di-symbols.js';
import type { Config } from '@/config.js'; import type { Config } from '@/config.js';
import { StatusError } from '@/misc/status-error.js'; import { StatusError } from '@/misc/status-error.js';
import { bindThis } from '@/decorators.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 { Response } from 'node-fetch';
import type { URL } from 'node:url'; import type { URL } from 'node:url';
export type HttpRequestSendOptions = {
throwErrorWhenResponseNotOk: boolean;
validators?: ((res: Response) => void)[];
};
@Injectable() @Injectable()
export class HttpRequestService { 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 @bindThis
public async getJson<T = unknown>(url: string, accept = 'application/json, */*', headers?: Record<string, string>): Promise<T> { public async getJson<T = unknown>(url: string, accept = 'application/json, */*', headers?: Record<string, string>): Promise<T> {
const res = await this.send(url, { const res = await this.send(url, {
@ -132,17 +156,20 @@ export class HttpRequestService {
} }
@bindThis @bindThis
public async send(url: string, args: { public async send(
url: string,
args: {
method?: string, method?: string,
body?: string, body?: string,
headers?: Record<string, string>, headers?: Record<string, string>,
timeout?: number, timeout?: number,
size?: number, size?: number,
} = {}, extra: { } = {},
throwErrorWhenResponseNotOk: boolean; extra: HttpRequestSendOptions = {
} = {
throwErrorWhenResponseNotOk: true, throwErrorWhenResponseNotOk: true,
}): Promise<Response> { validators: [],
},
): Promise<Response> {
const timeout = args.timeout ?? 5000; const timeout = args.timeout ?? 5000;
const controller = new AbortController(); const controller = new AbortController();
@ -166,6 +193,12 @@ export class HttpRequestService {
throw new StatusError(`${res.status} ${res.statusText}`, res.status, res.statusText); throw new StatusError(`${res.status} ${res.statusText}`, res.status, res.statusText);
} }
if (res.ok) {
for (const validator of (extra.validators ?? [])) {
validator(res);
}
}
return res; return res;
} }
} }

View file

@ -14,6 +14,7 @@ import { HttpRequestService } from '@/core/HttpRequestService.js';
import { LoggerService } from '@/core/LoggerService.js'; import { LoggerService } from '@/core/LoggerService.js';
import { bindThis } from '@/decorators.js'; import { bindThis } from '@/decorators.js';
import type Logger from '@/logger.js'; import type Logger from '@/logger.js';
import { validateContentTypeSetAsActivityPub } from '@/core/activitypub/misc/validator.js';
type Request = { type Request = {
url: string; url: string;
@ -70,7 +71,7 @@ export class ApRequestCreator {
url: u.href, url: u.href,
method: 'GET', method: 'GET',
headers: this.#objectAssignWithLcKey({ 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(), 'Date': new Date().toUTCString(),
'Host': new URL(args.url).host, 'Host': new URL(args.url).host,
}, args.additionalHeaders), }, args.additionalHeaders),
@ -195,6 +196,9 @@ export class ApRequestService {
const res = await this.httpRequestService.send(url, { const res = await this.httpRequestService.send(url, {
method: req.request.method, method: req.request.method,
headers: req.request.headers, headers: req.request.headers,
}, {
throwErrorWhenResponseNotOk: true,
validators: [validateContentTypeSetAsActivityPub],
}); });
return await res.json(); return await res.json();

View file

@ -105,7 +105,7 @@ export class Resolver {
const object = (this.user const object = (this.user
? await this.apRequestService.signedGet(value, this.user) as IObject ? 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 ( if (
Array.isArray(object['@context']) ? Array.isArray(object['@context']) ?

View file

@ -8,6 +8,7 @@ import { Injectable } from '@nestjs/common';
import { HttpRequestService } from '@/core/HttpRequestService.js'; import { HttpRequestService } from '@/core/HttpRequestService.js';
import { bindThis } from '@/decorators.js'; import { bindThis } from '@/decorators.js';
import { CONTEXTS } from './misc/contexts.js'; import { CONTEXTS } from './misc/contexts.js';
import { validateContentTypeSetAsJsonLD } from './misc/validator.js';
import type { JsonLdDocument } from 'jsonld'; import type { JsonLdDocument } from 'jsonld';
import type { JsonLd, RemoteDocument } from 'jsonld/jsonld-spec.js'; import type { JsonLd, RemoteDocument } from 'jsonld/jsonld-spec.js';
@ -133,7 +134,10 @@ class LdSignature {
}, },
timeout: this.loderTimeout, timeout: this.loderTimeout,
}, },
{ throwErrorWhenResponseNotOk: false }, {
throwErrorWhenResponseNotOk: false,
validators: [validateContentTypeSetAsJsonLD],
},
).then(res => { ).then(res => {
if (!res.ok) { if (!res.ok) {
throw new Error(`${res.status} ${res.statusText}`); throw new Error(`${res.status} ${res.statusText}`);

View file

@ -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 = /^\s*(application|text)\/[a-zA-Z0-9\.\-\+]+\+json\s*(;|$)/;
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');
}

View file

@ -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');
});
});

View file

@ -202,7 +202,7 @@ describe('ActivityPub', () => {
describe('Renderer', () => { describe('Renderer', () => {
test('Render an announce with visibility: followers', () => { test('Render an announce with visibility: followers', () => {
rendererService.renderAnnounce(null, { rendererService.renderAnnounce('https://example.com/notes/00example', {
id: genAidx(Date.now()), id: genAidx(Date.now()),
visibility: 'followers', visibility: 'followers',
} as MiNote); } as MiNote);

View file

@ -13,10 +13,11 @@ import fetch, { File, RequestInit } from 'node-fetch';
import { DataSource } from 'typeorm'; import { DataSource } from 'typeorm';
import { JSDOM } from 'jsdom'; import { JSDOM } from 'jsdom';
import { DEFAULT_POLICIES } from '@/core/RoleService.js'; 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 { entities } from '../src/postgres.js';
import { loadConfig } from '../src/config.js'; import { loadConfig } from '../src/config.js';
import type * as misskey from 'misskey-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'; export { server as startServer, jobQueue as startJobQueue } from '@/boot/common.js';
@ -124,8 +125,8 @@ function timeoutPromise<T>(p: Promise<T>, timeout: number): Promise<T> {
return Promise.race([ return Promise.race([
p, p,
new Promise((reject) => { new Promise((reject) => {
setTimeout(() => { reject(new Error('timed out')); }, timeout) setTimeout(() => { reject(new Error('timed out')); }, timeout);
}) as never }) 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; const body = res.status !== 204 ? await res.json() as misskey.Endpoints['drive/files/create']['res'] : null;
return { return {
status: res.status, status: res.status,
headers: res.headers, headers: res.headers,
@ -343,7 +343,7 @@ export const uploadUrl = async (user: UserToken, url: string): Promise<Packed<'D
'main', 'main',
(msg) => msg.type === 'urlUploadFinished' && msg.body.marker === marker, (msg) => msg.type === 'urlUploadFinished' && msg.body.marker === marker,
(msg) => msg.body.file as Packed<'DriveFile'>, (msg) => msg.body.file as Packed<'DriveFile'>,
60 * 1000 60 * 1000,
); );
await api('drive/files/upload-from-url', { await api('drive/files/upload-from-url', {
@ -439,15 +439,15 @@ export function makeStreamCatcher<T>(
cond: (message: Record<string, any>) => boolean, cond: (message: Record<string, any>) => boolean,
extractor: (message: Record<string, any>) => T, extractor: (message: Record<string, any>) => T,
timeout = 60 * 1000): Promise<T> { timeout = 60 * 1000): Promise<T> {
let ws: WebSocket let ws: WebSocket;
const p = new Promise<T>(async (resolve) => { const p = new Promise<T>(async (resolve) => {
ws = await connectStream(user, channel, (msg) => { ws = await connectStream(user, channel, (msg) => {
if (cond(msg)) { if (cond(msg)) {
resolve(extractor(msg)) resolve(extractor(msg));
} }
}); });
}).finally(() => { }).finally(() => {
ws?.close(); ws.close();
}); });
return timeoutPromise(p, timeout); return timeoutPromise(p, timeout);
@ -476,6 +476,14 @@ export const simpleGet = async (path: string, accept = '*/*', cookie: any = unde
'text/html; charset=utf-8', '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 = const body =
jsonTypes.includes(res.headers.get('content-type') ?? '') ? await res.json() : jsonTypes.includes(res.headers.get('content-type') ?? '') ? await res.json() :
htmlTypes.includes(res.headers.get('content-type') ?? '') ? new JSDOM(await res.text()) : htmlTypes.includes(res.headers.get('content-type') ?? '') ? new JSDOM(await res.text()) :