From a6e257f502296495cf2c1bdc7565e9ce34848506 Mon Sep 17 00:00:00 2001 From: Amelia Yukii Date: Thu, 1 Feb 2024 15:06:21 +0000 Subject: Merge branch 'feture/code-injection-fix' into 'develop' CVE: Fixed code injection from twitter import See merge request TransFem-org/Sharkey!390 (cherry picked from commit 127f8556d409a1082f0050a7ebf57ba846263f6f) 2a8e93e4 Fixed code injection from twitter import --- .../processors/ImportNotesProcessorService.ts | 42 +++++++++++++--------- 1 file changed, 25 insertions(+), 17 deletions(-) (limited to 'packages/backend/src') diff --git a/packages/backend/src/queue/processors/ImportNotesProcessorService.ts b/packages/backend/src/queue/processors/ImportNotesProcessorService.ts index 03a0e951b3..d64a861b03 100644 --- a/packages/backend/src/queue/processors/ImportNotesProcessorService.ts +++ b/packages/backend/src/queue/processors/ImportNotesProcessorService.ts @@ -130,6 +130,17 @@ export class ImportNotesProcessorService { return typeof obj[Symbol.iterator] === 'function'; } + private parseTwitterFile(str : string) : null | [{ tweet: any }] { + const removed = str.replace(new RegExp('window\\.YTD\\.tweets\\.part0 = ', 'g'), ''); + + try { + return JSON.parse(removed); + } catch (error) { + //The format is not what we expected. Either this file was tampered with or twitters exports changed + return null; + } + } + @bindThis public async process(job: Bull.Job): Promise { this.logger.info(`Starting note import of ${job.data.user.id} ...`); @@ -175,23 +186,20 @@ export class ImportNotesProcessorService { try { this.logger.succ(`Unzipping to ${outputPath}`); ZipReader.withDestinationPath(outputPath).viaBuffer(await fs.promises.readFile(destPath)); - const fakeWindow: any = { - window: { - YTD: { - tweets: { - part0: {}, - }, - }, - }, - }; - const script = new vm.Script(fs.readFileSync(outputPath + '/data/tweets.js', 'utf-8')); - const context = vm.createContext(fakeWindow); - script.runInContext(context); - const tweets = Object.keys(fakeWindow.window.YTD.tweets.part0).reduce((m, key, i, obj) => { - return m.concat(fakeWindow.window.YTD.tweets.part0[key].tweet); - }, []); - const processedTweets = await this.recreateChain(['id_str'], ['in_reply_to_status_id_str'], tweets, false); - this.queueService.createImportTweetsToDbJob(job.data.user, processedTweets, null); + + const unprocessedTweetJson = this.parseTwitterFile(fs.readFileSync(outputPath + '/data/tweets.js', 'utf-8')); + + //Make sure that it isnt null (because if something went wrong in parseTwitterFile it returns null) + if (unprocessedTweetJson) { + const tweets = Object.keys(unprocessedTweetJson).reduce((m, key, i, obj) => { + return m.concat(unprocessedTweetJson[i].tweet); + }, []); + + const processedTweets = await this.recreateChain(['id_str'], ['in_reply_to_status_id_str'], tweets, false); + this.queueService.createImportTweetsToDbJob(job.data.user, processedTweets, null); + } else { + this.logger.warn('Failed to import twitter notes due to malformed file'); + } } finally { cleanup(); } -- cgit v1.2.3-freya From 1948ca9aa8aff9bd36011d8217e6aba2547759b5 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 --- 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 +++++++++++++ .../backend/test/e2e/fetch-validate-ap-deny.ts | 40 +++++++++++++ packages/backend/test/unit/activitypub.ts | 2 +- packages/backend/test/utils.ts | 67 +++++++++++++++++++--- 8 files changed, 195 insertions(+), 22 deletions(-) create mode 100644 packages/backend/src/core/activitypub/misc/validator.ts create mode 100644 packages/backend/test/e2e/fetch-validate-ap-deny.ts (limited to 'packages/backend/src') 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 { + 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 b59ce5241f..bd7b9bdf09 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; @@ -66,7 +67,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), @@ -190,6 +191,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 46b8ea9cdd..8c5fa0006c 100644 --- a/packages/backend/test/utils.ts +++ b/packages/backend/test/utils.ts @@ -13,6 +13,8 @@ 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'; @@ -110,6 +112,20 @@ export function randomString(chars = 'abcdefghijklmnopqrstuvwxyz0123456789', len return randomString; } +/** + * @brief プロミスにタイムアウト追加 + * @param p 待ち対象プロミス + * @param timeout 待機ミリ秒 + */ +function timeoutPromise(p: Promise, timeout: number): Promise { + return Promise.race([ + p, + new Promise((reject) => { + setTimeout(() => { reject(new Error('timed out')); }, timeout); + }) as never, + ]); +} + export const signup = async (params?: Partial): Promise> => { const q = Object.assign({ username: randomString(), @@ -304,7 +320,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, @@ -317,12 +332,13 @@ export const uploadUrl = async (user: UserToken, url: string) => { const file = new Promise(ok => resolve = ok); const marker = Math.random().toString(); - const ws = await connectStream(user, 'main', (msg) => { - if (msg.type === 'urlUploadFinished' && msg.body.marker === marker) { - ws.close(); - resolve(msg.body.file); - } - }); + const catcher = makeStreamCatcher( + user, + 'main', + (msg) => msg.type === 'urlUploadFinished' && msg.body.marker === marker, + (msg) => msg.body.file as Packed<'DriveFile'>, + 60 * 1000, + ); await api('drive/files/upload-from-url', { url, @@ -402,6 +418,35 @@ export const waitFire = async (user: UserToken, channel: string, trgr: () => any }); }; +/** + * @brief WebSocketストリームから特定条件の通知を拾うプロミスを生成 + * @param user ユーザー認証情報 + * @param channel チャンネル + * @param cond 条件 + * @param extractor 取り出し処理 + * @param timeout ミリ秒タイムアウト + * @returns 時間内に正常に処理できた場合に通知からextractorを通した値を得る + */ +export function makeStreamCatcher( + user: UserToken, + channel: string, + cond: (message: Record) => boolean, + extractor: (message: Record) => T, + timeout = 60 * 1000): Promise { + let ws: WebSocket; + const p = new Promise(async (resolve) => { + ws = await connectStream(user, channel, (msg) => { + if (cond(msg)) { + resolve(extractor(msg)); + } + }); + }).finally(() => { + ws.close(); + }); + + return timeoutPromise(p, timeout); +} + export type SimpleGetResponse = { status: number, body: any | JSDOM | null, @@ -425,6 +470,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()) : -- cgit v1.2.3-freya From 6132bc3b3e51ad059d8e005536a990c0e1226bd2 Mon Sep 17 00:00:00 2001 From: syuilo <4439005+syuilo@users.noreply.github.com> Date: Sat, 17 Feb 2024 14:26:48 +0900 Subject: fix of 9a70ce8f5ea9df00001894809f5ce7bc69b14c8a Co-Authored-By: RyotaK <49341894+Ry0taK@users.noreply.github.com> --- packages/backend/src/core/activitypub/misc/validator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/backend/src') diff --git a/packages/backend/src/core/activitypub/misc/validator.ts b/packages/backend/src/core/activitypub/misc/validator.ts index 6ba14a222f..690beeffef 100644 --- a/packages/backend/src/core/activitypub/misc/validator.ts +++ b/packages/backend/src/core/activitypub/misc/validator.ts @@ -20,7 +20,7 @@ export function validateContentTypeSetAsActivityPub(response: Response): void { 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/; +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(); -- cgit v1.2.3-freya From 6ecfe7c7c3c8d11b486c2a78d93f307ba266b5ec Mon Sep 17 00:00:00 2001 From: dakkar Date: Sat, 2 Mar 2024 17:34:31 +0000 Subject: remove duplicate method --- .../src/queue/processors/ImportNotesProcessorService.ts | 11 ----------- 1 file changed, 11 deletions(-) (limited to 'packages/backend/src') diff --git a/packages/backend/src/queue/processors/ImportNotesProcessorService.ts b/packages/backend/src/queue/processors/ImportNotesProcessorService.ts index 10cd90c4ad..7cef858c51 100644 --- a/packages/backend/src/queue/processors/ImportNotesProcessorService.ts +++ b/packages/backend/src/queue/processors/ImportNotesProcessorService.ts @@ -130,17 +130,6 @@ export class ImportNotesProcessorService { return typeof obj[Symbol.iterator] === 'function'; } - private parseTwitterFile(str : string) : null | [{ tweet: any }] { - const removed = str.replace(new RegExp('window\\.YTD\\.tweets\\.part0 = ', 'g'), ''); - - try { - return JSON.parse(removed); - } catch (error) { - //The format is not what we expected. Either this file was tampered with or twitters exports changed - return null; - } - } - @bindThis private parseTwitterFile(str : string) : { tweet: object }[] { const jsonStr = str.replace(/^\s*window\.YTD\.tweets\.part0\s*=\s*/, ''); -- cgit v1.2.3-freya From 56dca6dbf5656368e5b449bdf1d05e58082e0959 Mon Sep 17 00:00:00 2001 From: dakkar Date: Sun, 7 Apr 2024 16:58:13 +0100 Subject: hide images/videos in og cards, when under a CW - fixes #487 --- packages/backend/src/server/web/views/note.pug | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'packages/backend/src') diff --git a/packages/backend/src/server/web/views/note.pug b/packages/backend/src/server/web/views/note.pug index 9bc652b6a1..47c89ae2d7 100644 --- a/packages/backend/src/server/web/views/note.pug +++ b/packages/backend/src/server/web/views/note.pug @@ -5,8 +5,8 @@ block vars - const title = user.name ? `${user.name} (@${user.username})` : `@${user.username}`; - const url = `${config.url}/notes/${note.id}`; - const isRenote = note.renote && note.text == null && note.fileIds.length == 0 && note.poll == null; - - const images = (note.files || []).filter(file => file.type.startsWith('image/') && !file.isSensitive) - - const videos = (note.files || []).filter(file => file.type.startsWith('video/') && !file.isSensitive) + - const images = note.cw ? [] : (note.files || []).filter(file => file.type.startsWith('image/') && !file.isSensitive) + - const videos = note.cw ? [] : (note.files || []).filter(file => file.type.startsWith('video/') && !file.isSensitive) block title = `${title} | ${instanceName}` -- cgit v1.2.3-freya