From afb63049798dd0277cd9045eb00a16ab1228376b Mon Sep 17 00:00:00 2001 From: Johann150 Date: Thu, 10 Feb 2022 11:47:46 +0100 Subject: [PATCH] fix: regular expressions in word mutes (#8254) * fix: handle regex exceptions for word mutes * add i18n strings Co-authored-by: rinsuki <428rinsuki+git@gmail.com> * stricter input validation in backend * add migration for hard mutes * fix * use correct regex library in migration * use query builder to avoid SQL injection Co-authored-by: Robin B Co-authored-by: rinsuki <428rinsuki+git@gmail.com> --- locales/ja-JP.yml | 2 + .../1644010796173-convert-hard-mutes.js | 64 +++++++++++++++++++ packages/backend/src/misc/check-word-mute.ts | 31 +++++---- .../src/server/api/endpoints/i/update.ts | 21 +++++- .../client/src/pages/settings/word-mute.vue | 55 ++++++++++++++-- .../client/src/scripts/check-word-mute.ts | 31 +++++---- 6 files changed, 173 insertions(+), 31 deletions(-) create mode 100644 packages/backend/migration/1644010796173-convert-hard-mutes.js diff --git a/locales/ja-JP.yml b/locales/ja-JP.yml index 62d558704..6caa3e377 100644 --- a/locales/ja-JP.yml +++ b/locales/ja-JP.yml @@ -595,6 +595,8 @@ smtpSecure: "SMTP 接続に暗黙的なSSL/TLSを使用する" smtpSecureInfo: "STARTTLS使用時はオフにします。" testEmail: "配信テスト" wordMute: "ワードミュート" +regexpError: "正規表現エラー" +regexpErrorDescription: "{tab}ワードミュートの{line}行目の正規表現にエラーが発生しました:" instanceMute: "インスタンスミュート" userSaysSomething: "{name}が何かを言いました" makeActive: "アクティブにする" diff --git a/packages/backend/migration/1644010796173-convert-hard-mutes.js b/packages/backend/migration/1644010796173-convert-hard-mutes.js new file mode 100644 index 000000000..f06da6567 --- /dev/null +++ b/packages/backend/migration/1644010796173-convert-hard-mutes.js @@ -0,0 +1,64 @@ +const RE2 = require('re2'); +const { MigrationInterface, QueryRunner } = require("typeorm"); + +module.exports = class convertHardMutes1644010796173 { + name = 'convertHardMutes1644010796173' + + async up(queryRunner) { + let entries = await queryRunner.query(`SELECT "userId", "mutedWords" FROM "user_profile"`); + for(let i = 0; i < entries.length; i++) { + let words = entries[i].mutedWords + .map(line => { + const regexp = line.join(" ").match(/^\/(.+)\/(.*)$/); + if (regexp) { + // convert regexp's + try { + new RE2(regexp[1], regexp[2]); + return `/${regexp[1]}/${regexp[2]}`; + } catch (err) { + // invalid regex, ignore it + return []; + } + } else { + // remove empty segments + return line.filter(x => x !== ''); + } + }) + // remove empty lines + .filter(x => !(Array.isArray(x) && x.length === 0)); + + await queryRunner.connection.createQueryBuilder() + .update('user_profile') + .set({ + mutedWords: words + }) + .where('userId = :id', { id: entries[i].userId }) + .execute(); + } + } + + async down(queryRunner) { + let entries = await queryRunner.query(`SELECT "userId", "mutedWords" FROM "user_profile"`); + for(let i = 0; i < entries.length; i++) { + let words = entries[i].mutedWords + .map(line => { + if (Array.isArray(line)) { + return line; + } else { + // do not split regex at spaces again + return [line]; + } + }) + // remove empty lines + .filter(x => !(Array.isArray(x) && x.length === 0)); + + await queryRunner.connection.createQueryBuilder() + .update('user_profile') + .set({ + mutedWords: words + }) + .where('userId = :id', { id: entries[i].userId }) + .execute(); + } + } +} diff --git a/packages/backend/src/misc/check-word-mute.ts b/packages/backend/src/misc/check-word-mute.ts index e2e871dd2..dedda3cdf 100644 --- a/packages/backend/src/misc/check-word-mute.ts +++ b/packages/backend/src/misc/check-word-mute.ts @@ -11,26 +11,31 @@ type UserLike = { id: User['id']; }; -export async function checkWordMute(note: NoteLike, me: UserLike | null | undefined, mutedWords: string[][]): Promise { +export async function checkWordMute(note: NoteLike, me: UserLike | null | undefined, mutedWords: Array): Promise { // 自分自身 if (me && (note.userId === me.id)) return false; - const words = mutedWords - // Clean up - .map(xs => xs.filter(x => x !== '')) - .filter(xs => xs.length > 0); - - if (words.length > 0) { + if (mutedWords.length > 0) { if (note.text == null) return false; - const matched = words.some(and => - and.every(keyword => { - const regexp = keyword.match(/^\/(.+)\/(.*)$/); - if (regexp) { + const matched = mutedWords.some(filter => { + if (Array.isArray(filter)) { + return filter.every(keyword => note.text!.includes(keyword)); + } else { + // represents RegExp + const regexp = filter.match(/^\/(.+)\/(.*)$/); + + // This should never happen due to input sanitisation. + if (!regexp) return false; + + try { return new RE2(regexp[1], regexp[2]).test(note.text!); + } catch (err) { + // This should never happen due to input sanitisation. + return false; } - return note.text!.includes(keyword); - })); + } + }); if (matched) return true; } diff --git a/packages/backend/src/server/api/endpoints/i/update.ts b/packages/backend/src/server/api/endpoints/i/update.ts index eb57aa2bf..aec7bbd2e 100644 --- a/packages/backend/src/server/api/endpoints/i/update.ts +++ b/packages/backend/src/server/api/endpoints/i/update.ts @@ -1,3 +1,4 @@ +const RE2 = require('re2'); import $ from 'cafy'; import * as mfm from 'mfm-js'; import { ID } from '@/misc/cafy-id'; @@ -117,7 +118,7 @@ export const meta = { }, mutedWords: { - validator: $.optional.arr($.arr($.str)), + validator: $.optional.arr($.either($.arr($.str.min(1)).min(1), $.str)), }, mutedInstances: { @@ -163,6 +164,12 @@ export const meta = { code: 'NO_SUCH_PAGE', id: '8e01b590-7eb9-431b-a239-860e086c408e', }, + + invalidRegexp: { + message: 'Invalid Regular Expression.', + code: 'INVALID_REGEXP', + id: '0d786918-10df-41cd-8f33-8dec7d9a89a5', + } }, res: { @@ -191,6 +198,18 @@ export default define(meta, async (ps, _user, token) => { if (ps.avatarId !== undefined) updates.avatarId = ps.avatarId; if (ps.bannerId !== undefined) updates.bannerId = ps.bannerId; if (ps.mutedWords !== undefined) { + // validate regular expression syntax + ps.mutedWords.filter(x => !Array.isArray(x)).forEach(x => { + const regexp = x.match(/^\/(.+)\/(.*)$/); + if (!regexp) throw new ApiError(meta.errors.invalidRegexp); + + try { + new RE2(regexp[1], regexp[2]); + } catch (err) { + throw new ApiError(meta.errors.invalidRegexp); + } + }); + profileUpdates.mutedWords = ps.mutedWords; profileUpdates.enableWordMute = ps.mutedWords.length > 0; } diff --git a/packages/client/src/pages/settings/word-mute.vue b/packages/client/src/pages/settings/word-mute.vue index 19980dea1..bdccd7f1e 100644 --- a/packages/client/src/pages/settings/word-mute.vue +++ b/packages/client/src/pages/settings/word-mute.vue @@ -81,18 +81,65 @@ export default defineComponent({ }, async created() { - this.softMutedWords = this.$store.state.mutedWords.map(x => x.join(' ')).join('\n'); - this.hardMutedWords = this.$i.mutedWords.map(x => x.join(' ')).join('\n'); + const render = (mutedWords) => mutedWords.map(x => { + if (Array.isArray(x)) { + return x.join(' '); + } else { + return x; + } + }).join('\n'); + + this.softMutedWords = render(this.$store.state.mutedWords); + this.hardMutedWords = render(this.$i.mutedWords); this.hardWordMutedNotesCount = (await os.api('i/get-word-muted-notes-count', {})).count; }, methods: { async save() { - this.$store.set('mutedWords', this.softMutedWords.trim().split('\n').map(x => x.trim().split(' '))); + const parseMutes = (mutes, tab) => { + // split into lines, remove empty lines and unnecessary whitespace + let lines = mutes.trim().split('\n').map(line => line.trim()).filter(line => line != ''); + + // check each line if it is a RegExp or not + for(let i = 0; i < lines.length; i++) { + const line = lines[i] + const regexp = line.match(/^\/(.+)\/(.*)$/); + if (regexp) { + // check that the RegExp is valid + try { + new RegExp(regexp[1], regexp[2]); + // note that regex lines will not be split by spaces! + } catch (err) { + // invalid syntax: do not save, do not reset changed flag + os.alert({ + type: 'error', + title: this.$ts.regexpError, + text: this.$t('regexpErrorDescription', { tab, line: i + 1 }) + "\n" + err.toString() + }); + // re-throw error so these invalid settings are not saved + throw err; + } + } else { + lines[i] = line.split(' '); + } + } + }; + + let softMutes, hardMutes; + try { + softMutes = parseMutes(this.softMutedWords, this.$ts._wordMute.soft); + hardMutes = parseMutes(this.hardMutedWords, this.$ts._wordMute.hard); + } catch (err) { + // already displayed error message in parseMutes + return; + } + + this.$store.set('mutedWords', softMutes); await os.api('i/update', { - mutedWords: this.hardMutedWords.trim().split('\n').map(x => x.trim().split(' ')), + mutedWords: hardMutes, }); + this.changed = false; }, diff --git a/packages/client/src/scripts/check-word-mute.ts b/packages/client/src/scripts/check-word-mute.ts index 55637bb3b..74e258186 100644 --- a/packages/client/src/scripts/check-word-mute.ts +++ b/packages/client/src/scripts/check-word-mute.ts @@ -1,23 +1,28 @@ -export function checkWordMute(note: Record, me: Record | null | undefined, mutedWords: string[][]): boolean { +export function checkWordMute(note: Record, me: Record | null | undefined, mutedWords: Array): boolean { // 自分自身 if (me && (note.userId === me.id)) return false; - const words = mutedWords - // Clean up - .map(xs => xs.filter(x => x !== '')) - .filter(xs => xs.length > 0); - - if (words.length > 0) { + if (mutedWords.length > 0) { if (note.text == null) return false; - const matched = words.some(and => - and.every(keyword => { - const regexp = keyword.match(/^\/(.+)\/(.*)$/); - if (regexp) { + const matched = mutedWords.some(filter => { + if (Array.isArray(filter)) { + return filter.every(keyword => note.text!.includes(keyword)); + } else { + // represents RegExp + const regexp = filter.match(/^\/(.+)\/(.*)$/); + + // This should never happen due to input sanitisation. + if (!regexp) return false; + + try { return new RegExp(regexp[1], regexp[2]).test(note.text!); + } catch (err) { + // This should never happen due to input sanitisation. + return false; } - return note.text!.includes(keyword); - })); + } + }); if (matched) return true; }