From 3ad6323c238bf32a03758e31fb9f126d5c058b3c Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sun, 5 Feb 2023 20:26:45 +0100 Subject: [PATCH 1/7] fix registry migration closes https://akkoma.dev/FoundKeyGang/FoundKey/issues/337 --- .../backend/migration/1675375940759-registry-remove-domain.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/migration/1675375940759-registry-remove-domain.js b/packages/backend/migration/1675375940759-registry-remove-domain.js index 50a6f6b8b..d3ee12ca9 100644 --- a/packages/backend/migration/1675375940759-registry-remove-domain.js +++ b/packages/backend/migration/1675375940759-registry-remove-domain.js @@ -6,7 +6,7 @@ export class registryRemoveDomain1675375940759 { await queryRunner.query(`ALTER TABLE "registry_item" DROP COLUMN "domain"`); await queryRunner.query(`ALTER TABLE "registry_item" ALTER COLUMN "key" TYPE text USING "key"::text`); // delete existing duplicated entries, keeping the latest updated one - await queryRunner.query(`DELETE FROM "registry_item" AS "a" WHERE "updatedAt" != (SELECT MAX("updatedAt") OVER (PARTITION BY "userId", "key", "scope") FROM "registry_item" AS "b" WHERE "a"."userId" = "b"."userId" AND "a"."key" = "b"."key" AND "a"."scope" = "b"."scope")`); + await queryRunner.query(`DELETE FROM "registry_item" AS "a" WHERE "updatedAt" != (SELECT MAX("updatedAt") FROM "registry_item" AS "b" WHERE "a"."userId" = "b"."userId" AND "a"."key" = "b"."key" AND "a"."scope" = "b"."scope" GROUP BY "userId", "key", "scope")`); await queryRunner.query(`ALTER TABLE "registry_item" ADD CONSTRAINT "UQ_b8d6509f847331273ab99daccc7" UNIQUE ("userId", "key", "scope")`); } From c1ae134c0af718a1939b0ec48bfb56f82ed072f8 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Fri, 10 Feb 2023 18:31:23 +0100 Subject: [PATCH 2/7] security: make sure there is no SQL insertion --- packages/backend/src/misc/safe-for-sql.ts | 3 --- .../backend/src/server/api/endpoints/hashtags/trend.ts | 5 ++--- .../src/server/api/endpoints/notes/search-by-tag.ts | 8 +++----- 3 files changed, 5 insertions(+), 11 deletions(-) delete mode 100644 packages/backend/src/misc/safe-for-sql.ts diff --git a/packages/backend/src/misc/safe-for-sql.ts b/packages/backend/src/misc/safe-for-sql.ts deleted file mode 100644 index 02eb7f0a2..000000000 --- a/packages/backend/src/misc/safe-for-sql.ts +++ /dev/null @@ -1,3 +0,0 @@ -export function safeForSql(text: string): boolean { - return !/[\0\x08\x09\x1a\n\r"'\\\%]/g.test(text); -} diff --git a/packages/backend/src/server/api/endpoints/hashtags/trend.ts b/packages/backend/src/server/api/endpoints/hashtags/trend.ts index b6943dd08..a68b9c48c 100644 --- a/packages/backend/src/server/api/endpoints/hashtags/trend.ts +++ b/packages/backend/src/server/api/endpoints/hashtags/trend.ts @@ -3,7 +3,6 @@ import { MINUTE, HOUR } from '@/const.js'; import { fetchMeta } from '@/misc/fetch-meta.js'; import { Notes } from '@/models/index.js'; import { Note } from '@/models/entities/note.js'; -import { safeForSql } from '@/misc/safe-for-sql.js'; import { normalizeForSearch } from '@/misc/normalize-for-search.js'; import define from '../../define.js'; @@ -122,7 +121,7 @@ export default define(meta, paramDef, async () => { for (let i = 0; i < range; i++) { countPromises.push(Promise.all(hots.map(tag => Notes.createQueryBuilder('note') .select('count(distinct note.userId)') - .where(`'{"${safeForSql(tag) ? tag : 'aichan_kawaii'}"}' <@ note.tags`) + .where(':tag = ANY(note.tags)', { tag }) .andWhere('note.createdAt < :lt', { lt: new Date(now.getTime() - (interval * i)) }) .andWhere('note.createdAt > :gt', { gt: new Date(now.getTime() - (interval * (i + 1))) }) .cache(60000) // 1 min @@ -136,7 +135,7 @@ export default define(meta, paramDef, async () => { const totalCounts = await Promise.all(hots.map(tag => Notes.createQueryBuilder('note') .select('count(distinct note.userId)') - .where(`'{"${safeForSql(tag) ? tag : 'aichan_kawaii'}"}' <@ note.tags`) + .where(':tag = ANY(note.tags)', { tag }) .andWhere('note.createdAt > :gt', { gt: new Date(now.getTime() - rangeA) }) .cache(60000 * 60) // 60 min .getRawOne() diff --git a/packages/backend/src/server/api/endpoints/notes/search-by-tag.ts b/packages/backend/src/server/api/endpoints/notes/search-by-tag.ts index 4f4711491..e0bdca6cb 100644 --- a/packages/backend/src/server/api/endpoints/notes/search-by-tag.ts +++ b/packages/backend/src/server/api/endpoints/notes/search-by-tag.ts @@ -1,6 +1,5 @@ import { Brackets } from 'typeorm'; import { Notes } from '@/models/index.js'; -import { safeForSql } from '@/misc/safe-for-sql.js'; import { normalizeForSearch } from '@/misc/normalize-for-search.js'; import define from '../../define.js'; import { makePaginationQuery } from '../../common/make-pagination-query.js'; @@ -86,15 +85,14 @@ export default define(meta, paramDef, async (ps, me) => { try { if (ps.tag) { - if (!safeForSql(ps.tag)) throw new Error('Injection'); - query.andWhere(`'{"${normalizeForSearch(ps.tag)}"}' <@ note.tags`); + query.andWhere(':tag = ANY(note.tags)', { tag: normalizeForSearch(ps.tag) }); } else { + let i = 0; query.andWhere(new Brackets(qb => { for (const tags of ps.query!) { qb.orWhere(new Brackets(qb => { for (const tag of tags) { - if (!safeForSql(tag)) throw new Error('Injection'); - qb.andWhere(`'{"${normalizeForSearch(tag)}"}' <@ note.tags`); + qb.andWhere(`:tag${++i} = ANY(note.tags)`, { ['tag' + i]: normalizeForSearch(tag) }); } })); } From af272ce3583268ebef179dde42c0567066c7241e Mon Sep 17 00:00:00 2001 From: syuilo Date: Sun, 5 Feb 2023 14:25:37 +0900 Subject: [PATCH 3/7] fix(server): validate filename and emoji name to improve security https://github.com/misskey-dev/misskey/commit/0d7256678e390af9e7576571b3fd262e265fbe18 Co-authored-by: Johann150 Changelog: Fixed --- .../backend/src/queue/processors/db/export-custom-emojis.ts | 4 ++++ .../backend/src/queue/processors/db/import-custom-emojis.ts | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/packages/backend/src/queue/processors/db/export-custom-emojis.ts b/packages/backend/src/queue/processors/db/export-custom-emojis.ts index c7e2e825d..f31531db4 100644 --- a/packages/backend/src/queue/processors/db/export-custom-emojis.ts +++ b/packages/backend/src/queue/processors/db/export-custom-emojis.ts @@ -58,6 +58,10 @@ export async function exportCustomEmojis(job: Bull.Job, done: () => void): Promi }); for (const emoji of customEmojis) { + if (!/^[a-zA-Z0-9_]+$/.test(emoji.name)) { + this.logger.error(`invalid emoji name: ${emoji.name}, skipping in emoji export`); + continue; + } const ext = mime.extension(emoji.type); const fileName = emoji.name + (ext ? '.' + ext : ''); const emojiPath = path + '/' + fileName; diff --git a/packages/backend/src/queue/processors/db/import-custom-emojis.ts b/packages/backend/src/queue/processors/db/import-custom-emojis.ts index 1d06d5ff8..855017460 100644 --- a/packages/backend/src/queue/processors/db/import-custom-emojis.ts +++ b/packages/backend/src/queue/processors/db/import-custom-emojis.ts @@ -50,6 +50,10 @@ export async function importCustomEmojis(job: Bull.Job, don for (const record of meta.emojis) { if (!record.downloaded) continue; + if (!/^[a-zA-Z0-9_]+?([a-zA-Z0-9\.]+)?$/.test(record.fileName)) { + this.logger.error(`invalid filename: ${record.fileName}, skipping in emoji import`); + continue; + } const emojiInfo = record.emoji; const emojiPath = outputPath + '/' + record.fileName; await Emojis.delete({ From 48fd543d0f8cd57f5c6e559236b9524f7daee8a7 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Fri, 10 Feb 2023 19:59:03 +0100 Subject: [PATCH 4/7] security: check URL schema of AP URIs Changelog: Fixed --- .../backend/src/remote/activitypub/type.ts | 72 ++++++++++++++----- 1 file changed, 54 insertions(+), 18 deletions(-) diff --git a/packages/backend/src/remote/activitypub/type.ts b/packages/backend/src/remote/activitypub/type.ts index 23b4ccf88..28868297d 100644 --- a/packages/backend/src/remote/activitypub/type.ts +++ b/packages/backend/src/remote/activitypub/type.ts @@ -34,21 +34,38 @@ export function getApIds(value: ApObject | undefined): string[] { return array.map(x => getApId(x)); } -/** - * Get first ActivityStreams Object id - */ -export function getOneApId(value: ApObject): string { - const firstOne = Array.isArray(value) ? value[0] : value; - return getApId(firstOne); -} - /** * Get ActivityStreams Object id */ export function getApId(value: string | Object): string { - if (typeof value === 'string') return value; - if (typeof value.id === 'string') return value.id; - throw new Error('cannot detemine id'); + let url = null; + if (typeof value === 'string') url = value; + else if (typeof value.id === 'string') url = value.id; + + if (!url || !['https:', 'http:'].includes(new URL(url).protocol)) { + throw new Error('cannot determine id'); + } else { + return url; + } +} + +/** + * Get first (valid) ActivityStreams Object id + */ +export function getOneApId(value: ApObject): string { + if (Array.isArray(value)) { + // find the first valid ID + for (const id of value) { + try { + return getApId(x); + } catch { + continue; + } + } + throw new Error('cannot determine id'); + } else { + return getApId(value); + } } /** @@ -60,15 +77,34 @@ export function getApType(value: Object): string { throw new Error('cannot detect type'); } -export function getOneApHrefNullable(value: ApObject | undefined): string | undefined { - const firstOne = Array.isArray(value) ? value[0] : value; - return getApHrefNullable(firstOne); +export function getApHrefNullable(value: string | IObject | undefined): string | undefined { + let url = null; + if (typeof value === 'string') url = value; + else if (typeof value?.href === 'string') url = value.href; + + if (!url || !['https:', 'http:'].includes(new URL(url).protocol)) { + return undefined; + } else { + return url; + } } -export function getApHrefNullable(value: string | IObject | undefined): string | undefined { - if (typeof value === 'string') return value; - if (typeof value?.href === 'string') return value.href; - return undefined; +export function getOneApHrefNullable(value: ApObject | undefined): string | undefined { + if (!value) { + return; + } else if (Array.isArray(value)) { + // find the first valid href + for (const href of value) { + try { + return getApHrefNullable(href); + } catch { + continue; + } + } + return undefined; + } else { + return getApHrefNullable(value); + } } export interface IActivity extends IObject { From 27b912b9b0e5a0be716a4d51cd4c9097d5da14f8 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Fri, 10 Feb 2023 20:01:57 +0100 Subject: [PATCH 5/7] security: check schema for URL previews Changelog: Fixed --- .../backend/src/server/web/url-preview.ts | 22 ++++++++++++------- packages/client/src/components/global/url.vue | 1 + .../client/src/components/url-preview.vue | 5 ++++- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/backend/src/server/web/url-preview.ts b/packages/backend/src/server/web/url-preview.ts index 7ff381443..259912e55 100644 --- a/packages/backend/src/server/web/url-preview.ts +++ b/packages/backend/src/server/web/url-preview.ts @@ -38,6 +38,14 @@ export const urlPreviewHandler = async (ctx: Koa.Context): Promise => { logger.succ(`Got preview of ${url}: ${summary.title}`); + if (summary.url && !(summary.url.startsWith('http://') || summary.url.startsWith('https://'))) { + throw new Error('unsupported schema included'); + } + + if (summary.player?.url && !(summary.player.url.startsWith('http://') || summary.player.url.startsWith('https://'))) { + throw new Error('unsupported schema included'); + } + summary.icon = wrap(summary.icon); summary.thumbnail = wrap(summary.thumbnail); @@ -54,12 +62,10 @@ export const urlPreviewHandler = async (ctx: Koa.Context): Promise => { }; function wrap(url?: string): string | null { - return url != null - ? url.match(/^https?:\/\//) - ? `${config.url}/proxy/preview.webp?${query({ - url, - preview: '1', - })}` - : url - : null; + if (url == null) return null; + if (!['http:', 'https:'].includes(new URL(url).protocol)) return null; + return config.url + '/proxy/preview.webp?' + query({ + url, + preview: '1', + }); } diff --git a/packages/client/src/components/global/url.vue b/packages/client/src/components/global/url.vue index babf49887..47811ae00 100644 --- a/packages/client/src/components/global/url.vue +++ b/packages/client/src/components/global/url.vue @@ -35,6 +35,7 @@ const props = withDefaults(defineProps<{ const self = props.url.startsWith(local); const uri = new URL(props.url); +if (!['http:', 'https:'].includes(url.protocol)) throw new Error('invalid url'); let el: HTMLElement | null = $ref(null); let schema = $ref(uri.protocol); diff --git a/packages/client/src/components/url-preview.vue b/packages/client/src/components/url-preview.vue index d866fd9ea..0add58761 100644 --- a/packages/client/src/components/url-preview.vue +++ b/packages/client/src/components/url-preview.vue @@ -54,6 +54,7 @@ let player = $ref({ let playerEnabled = $ref(false); const requestUrl = new URL(props.url); +if(!['http:', 'https:].includes(requestUrl.protocol)) throw new Error('invalid url'); if (requestUrl.hostname === 'music.youtube.com' && requestUrl.pathname.match('^/(?:watch|channel)')) { requestUrl.hostname = 'www.youtube.com'; @@ -72,7 +73,9 @@ fetch(`/url?url=${encodeURIComponent(requestUrl.href)}&lang=${requestLang}`).the icon = info.icon; sitename = info.sitename; fetching = false; - player = info.player; + if (['http:', 'https:'].includes(new URL(info.player.url).protocol)) { + player = info.player; + } }); }); From 09fe55379e1ad81bcb9735f4feba6592b247cf3a Mon Sep 17 00:00:00 2001 From: syuilo Date: Fri, 10 Feb 2023 20:04:45 +0100 Subject: [PATCH 6/7] client: check input for aiscript https://github.com/misskey-dev/misskey/commit/af1c9251fc2b64f4307333b5b19faad3005bf85b https://github.com/misskey-dev/misskey/commit/5f3640c7fd88a98d3c278e17d4bd91b836962395 Co-authored-by: Johann150 Changelog: Fixed --- packages/client/src/pages/scratchpad.vue | 1 + packages/client/src/plugin.ts | 3 ++- packages/client/src/scripts/aiscript/api.ts | 6 +++++- packages/client/src/widgets/aiscript.vue | 3 ++- packages/client/src/widgets/button.vue | 3 ++- 5 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/client/src/pages/scratchpad.vue b/packages/client/src/pages/scratchpad.vue index a33003ded..d72c21e6f 100644 --- a/packages/client/src/pages/scratchpad.vue +++ b/packages/client/src/pages/scratchpad.vue @@ -59,6 +59,7 @@ async function run() { os.inputText({ title: q, }).then(({ canceled, result: a }) => { + if (canceled) return; ok(a); }); }); diff --git a/packages/client/src/plugin.ts b/packages/client/src/plugin.ts index 422b6339c..0ab8648a6 100644 --- a/packages/client/src/plugin.ts +++ b/packages/client/src/plugin.ts @@ -18,7 +18,8 @@ export function install(plugin) { return new Promise(ok => { inputText({ title: q, - }).then(({ result: a }) => { + }).then(({ canceled, result: a }) => { + if (canceled) return; ok(a); }); }); diff --git a/packages/client/src/scripts/aiscript/api.ts b/packages/client/src/scripts/aiscript/api.ts index 01b8fd05f..6407d6b0d 100644 --- a/packages/client/src/scripts/aiscript/api.ts +++ b/packages/client/src/scripts/aiscript/api.ts @@ -24,7 +24,11 @@ export function createAiScriptEnv(opts) { return confirm.canceled ? values.FALSE : values.TRUE; }), 'Mk:api': values.FN_NATIVE(async ([ep, param, token]) => { - if (token) utils.assertString(token); + if (token) { + utils.assertString(token); + // In case there is a bug, it could be undefined. + if (typeof token.value !== 'string') throw new Error('invalid token'); + } apiRequests++; if (apiRequests > 16) return values.NULL; const res = await os.api(ep.value, utils.valToJs(param), token ? token.value : (opts.token || null)); diff --git a/packages/client/src/widgets/aiscript.vue b/packages/client/src/widgets/aiscript.vue index 30b16cac2..15357069e 100644 --- a/packages/client/src/widgets/aiscript.vue +++ b/packages/client/src/widgets/aiscript.vue @@ -72,7 +72,8 @@ const run = async (): Promise => { return new Promise(ok => { os.inputText({ title: q, - }).then(({ result: a }) => { + }).then(({ canceled, result: a }) => { + if (canceled) return; ok(a); }); }); diff --git a/packages/client/src/widgets/button.vue b/packages/client/src/widgets/button.vue index 9e4b84899..5a385380a 100644 --- a/packages/client/src/widgets/button.vue +++ b/packages/client/src/widgets/button.vue @@ -60,7 +60,8 @@ const run = async (): Promise => { return new Promise(ok => { os.inputText({ title: q, - }).then(({ result: a }) => { + }).then(({ canceled, result: a }) => { + if (canceled) return; ok(a); }); }); From 6a40ef35696814092944c0b35c919d36332c7136 Mon Sep 17 00:00:00 2001 From: Chloe Kudryavtsev Date: Fri, 10 Feb 2023 20:35:09 -0500 Subject: [PATCH 7/7] fix typo tfw no building before push --- packages/client/src/components/url-preview.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/client/src/components/url-preview.vue b/packages/client/src/components/url-preview.vue index 0add58761..5403ddd63 100644 --- a/packages/client/src/components/url-preview.vue +++ b/packages/client/src/components/url-preview.vue @@ -54,7 +54,7 @@ let player = $ref({ let playerEnabled = $ref(false); const requestUrl = new URL(props.url); -if(!['http:', 'https:].includes(requestUrl.protocol)) throw new Error('invalid url'); +if(!['http:', 'https:'].includes(requestUrl.protocol)) throw new Error('invalid url'); if (requestUrl.hostname === 'music.youtube.com' && requestUrl.pathname.match('^/(?:watch|channel)')) { requestUrl.hostname = 'www.youtube.com';