merge: include static analysis / security checks #407 (!414)

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

.gitlab-ci.yml

@@ -103,3 +103,9 @@ mergeManifests:
   only:
     - stable
     - develop
+
+include:
+  - template: Jobs/Dependency-Scanning.latest.gitlab-ci.yml
+  - template: Jobs/License-Scanning.latest.gitlab-ci.yml
+  - template: Jobs/SAST.latest.gitlab-ci.yml
+  - template: Jobs/Secret-Detection.latest.gitlab-ci.yml

.njsscan

@@ -0,0 +1,6 @@
+---
+- ignore-filenames:
+  # there may be a "header injection", I'm pretty sure we don't
+  # trigger it given the way we use this, but please remove this
+  # when/if the library gets updated
+  - packages/frontend/assets/libopenmpt.js

.semgrepignore

@@ -0,0 +1,4 @@
+/packages/frontend/.storybook/
+/packages/frontend/assets/
+/scripts/
+/tossface-emojis/

locales/index.js

@@ -49,7 +49,7 @@ const primaries = {
 };
 
 // 何故か文字列にバックスペース文字が混入することがあり、YAMLが壊れるので取り除く
-const clean = (text) => text.replace(new RegExp(String.fromCodePoint(0x08), 'g'), '');
+const clean = (text) => text.replace(new RegExp(String.fromCodePoint(0x08), 'g'), ''); // eslint-disable-line detect-non-literal-regexp
 
 export function build() {
 	const locales = languages.reduce((a, c) => (a[c] = yaml.load(clean(fs.readFileSync(new URL(`${c}.yml`, import.meta.url), 'utf-8'))) || {}, a), {});

packages/backend/src/core/UserAuthService.ts

@@ -11,6 +11,7 @@ import type { MiUserProfile, UserProfilesRepository, UsersRepository } from '@/m
 import { bindThis } from '@/decorators.js';
 import { isDuplicateKeyValueError } from '@/misc/is-duplicate-key-value-error.js';
 import type { MiLocalUser } from '@/models/User.js';
+import { secureishCompare } from '@/misc/secure-ish-compare.js';
 
 @Injectable()
 export class UserAuthService {
@@ -27,7 +28,9 @@ export class UserAuthService {
 	public async twoFactorAuthenticate(profile: MiUserProfile, token: string): Promise<void> {
 		if (profile.twoFactorBackupSecret?.includes(token)) {
 			await this.userProfilesRepository.update({ userId: profile.userId }, {
-				twoFactorBackupSecret: profile.twoFactorBackupSecret.filter((secret) => secret !== token),
+				twoFactorBackupSecret: profile.twoFactorBackupSecret.filter(
+					(secret) => !secureishCompare(secret, token)
+				),
 			});
 		} else {
 			const delta = OTPAuth.TOTP.validate({

packages/backend/src/misc/secure-ish-compare.ts

@@ -0,0 +1,33 @@
+/*
+ * SPDX-FileCopyrightText: dakkar and other sharkey contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import * as crypto from 'node:crypto';
+
+export function secureishCompare(expected: string, given:string) {
+	const expectedLen = expected.length;
+	const givenLen = given.length;
+
+	/*
+		this ensures that the two strings are the same length, so
+		`timingSafeEqual` will not throw an error
+
+		notice that we perform the same operations regardless of the
+		length of the strings, we're trying not to leak timing
+		information!
+	*/
+	const paddedExpected = Buffer.from(expected + given);
+	const paddedGiven = Buffer.from(given + expected);
+
+	/*
+		if the two strings were equal, `sortOfEqual` will be true
+
+		if will be true in other cases, too! like `abc` and `abcabc`; but
+		then, `expectedLen` and `givenLen` would be different, and we'd
+		return false
+	*/
+	const sortOfEqual = crypto.timingSafeEqual(paddedExpected, paddedGiven);
+
+	return sortOfEqual && (givenLen === expectedLen);
+}

packages/backend/src/queue/processors/ExportAntennasProcessorService.ts

@@ -46,6 +46,7 @@ export class ExportAntennasProcessorService {
 			return;
 		}
 		const [path, cleanup] = await createTemp();
+		// eslint-disable-next-line detect-non-literal-fs-filename
 		const stream = fs.createWriteStream(path, { flags: 'a' });
 		const write = (input: string): Promise<void> => {
 			return new Promise((resolve, reject) => {

packages/backend/src/server/ActivityPubServerService.ts

@@ -38,6 +38,7 @@ import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOption
 import type { FindOptionsWhere } from 'typeorm';
 import type Logger from '@/logger.js';
 import { LoggerService } from '@/core/LoggerService.js';
+import { secureishCompare } from '@/misc/secure-ish-compare.js';
 
 const ACTIVITY_JSON = 'application/activity+json; charset=utf-8';
 const LD_JSON = 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"; charset=utf-8';
@@ -287,7 +288,7 @@ export class ActivityPubServerService {
 
 			const hash = crypto.createHash('sha256').update(request.rawBody).digest('base64');
 
-			if (hash !== digestValue) {
+			if (!secureishCompare(hash, digestValue)) {
 				// Invalid digest
 				reply.code(401);
 				return;

packages/backend/src/server/api/AuthenticateService.ts

@@ -42,7 +42,7 @@ export class AuthenticateService implements OnApplicationShutdown {
 
 	@bindThis
 	public async authenticate(token: string | null | undefined): Promise<[MiLocalUser | null, MiAccessToken | null]> {
-		if (token == null) {
+		if (token == null) { // eslint-disable-line detect-possible-timing-attacks
 			return [null, null];
 		}
 

packages/backend/src/server/api/endpoints/i/2fa/key-done.ts

@@ -76,7 +76,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> {
 			const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id });
 
 			if (profile.twoFactorEnabled) {
-				if (token == null) {
+				if (token == null) { // eslint-disable-line detect-possible-timing-attacks
 					throw new Error('authentication failed');
 				}
 

packages/backend/src/server/api/endpoints/i/2fa/register-key.ts

@@ -208,7 +208,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> {
 
 			// Compare password
 			if (profile.twoFactorEnabled) {
-				if (token == null) {
+				if (token == null) { // eslint-disable-line detect-possible-timing-attacks
 					throw new Error('authentication failed');
 				}
 

packages/backend/src/server/api/endpoints/i/2fa/register.ts

@@ -67,7 +67,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 			const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id });
 
 			if (profile.twoFactorEnabled) {
-				if (token == null) {
+				if (token == null) { // eslint-disable-line detect-possible-timing-attacks
 					throw new Error('authentication failed');
 				}
 

packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts

@@ -57,7 +57,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 
 			// Compare password
 			if (profile.twoFactorEnabled) {
-				if (token == null) {
+				if (token == null) { // eslint-disable-line detect-possible-timing-attacks
 					throw new Error('authentication failed');
 				}
 

packages/backend/src/server/api/endpoints/i/2fa/unregister.ts

@@ -52,7 +52,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 			const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id });
 
 			if (profile.twoFactorEnabled) {
-				if (token == null) {
+				if (token == null) { // eslint-disable-line detect-possible-timing-attacks
 					throw new Error('authentication failed');
 				}
 

packages/backend/src/server/api/endpoints/i/change-password.ts

@@ -40,7 +40,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 			const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id });
 
 			if (profile.twoFactorEnabled) {
-				if (token == null) {
+				if (token == null) { // eslint-disable-line detect-possible-timing-attacks
 					throw new Error('authentication failed');
 				}
 

packages/backend/src/server/api/endpoints/i/delete-account.ts

@@ -44,7 +44,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 			const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id });
 
 			if (profile.twoFactorEnabled) {
-				if (token == null) {
+				if (token == null) { // eslint-disable-line detect-possible-timing-attacks
 					throw new Error('authentication failed');
 				}
 

packages/backend/src/server/api/endpoints/i/update-email.ts

@@ -77,7 +77,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 			const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id });
 
 			if (profile.twoFactorEnabled) {
-				if (token == null) {
+				if (token == null) { // eslint-disable-line detect-possible-timing-attacks
 					throw new Error('authentication failed');
 				}
 

packages/backend/src/server/web/ClientServerService.ts

@@ -200,7 +200,7 @@ export class ClientServerService {
 			const url = decodeURI(request.routeOptions.url);
 			if (url === bullBoardPath || url.startsWith(bullBoardPath + '/')) {
 				const token = request.cookies.token;
-				if (token == null) {
+				if (token == null) { // eslint-disable-line detect-possible-timing-attacks
 					reply.code(401).send('Login required');
 					return;
 				}

packages/backend/test/unit/misc/secure-ish-compare.ts

@@ -0,0 +1,25 @@
+/*
+ * SPDX-FileCopyrightText: dakkar and other sharkey contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import { secureishCompare } from '@/misc/secure-ish-compare.js';
+
+describe(secureishCompare, () => {
+	it('should return true if strings are equal', () => {
+		expect(secureishCompare('abc','abc')).toBe(true);
+		expect(secureishCompare('aaa','aaa')).toBe(true);
+	});
+	it('should return false if strings are different', () => {
+		expect(secureishCompare('abc','def')).toBe(false);
+	});
+	it('should return false if one is prefix of the other', () => {
+		expect(secureishCompare('abc','abcabc')).toBe(false);
+		expect(secureishCompare('abcabc','abc')).toBe(false);
+		expect(secureishCompare('aaa','aa')).toBe(false);
+	});
+	it('should return false if strings are very different', () => {
+		expect(secureishCompare('abc','defghi')).toBe(false);
+		expect(secureishCompare('defghi','abc')).toBe(false);
+	});
+});

packages/frontend/public/mockServiceWorker.js

@@ -117,6 +117,7 @@ self.addEventListener('fetch', function (event) {
   }
 
   // Generate unique request ID.
+  // eslint-disable-next-line node_insecure_random_generator
   const requestId = Math.random().toString(16).slice(2)
 
   event.respondWith(

packages/frontend/src/components/MkAutocomplete.vue

@@ -25,7 +25,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 			<MkCustomEmoji v-if="'isCustomEmoji' in emoji && emoji.isCustomEmoji" :name="emoji.emoji" :class="$style.emoji"/>
 			<MkEmoji v-else :emoji="emoji.emoji" :class="$style.emoji"/>
 			<!-- eslint-disable-next-line vue/no-v-html -->
-			<span v-if="q" :class="$style.emojiName" v-html="sanitizeHtml(emoji.name.replace(q, `<b>${q}</b>`))"></span>
+			<span v-if="q" :class="$style.emojiName" v-html="sanitizeHtml(emoji.name.replace(q, `<b>${q}</b>`))"></span><!-- njsscan-ignore:vue_template -->
 			<span v-else v-text="emoji.name"></span>
 			<span v-if="emoji.aliasOf" :class="$style.emojiAlias">({{ emoji.aliasOf }})</span>
 		</li>

packages/frontend/src/components/MkCode.core.vue

@@ -5,7 +5,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 
 <!-- eslint-disable vue/no-v-html -->
 <template>
-<div :class="[$style.codeBlockRoot, { [$style.codeEditor]: codeEditor }]" v-html="html"></div>
+<div :class="[$style.codeBlockRoot, { [$style.codeEditor]: codeEditor }]" v-html="html"></div><!-- njsscan-ignore:vue_template -->
 </template>
 
 <script lang="ts" setup>

packages/frontend/src/components/MkFormula.vue

@@ -4,8 +4,8 @@ SPDX-License-Identifier: AGPL-3.0-only
 -->
 
 <template>
-	<div v-if="block" :class="$style.block" v-html="renderedFormula"></div>
+	<div v-if="block" :class="$style.block" v-html="renderedFormula"></div><!-- njsscan-ignore:vue_template -->
-	<span v-else v-html="renderedFormula"></span>
+	<span v-else v-html="renderedFormula"></span><!-- njsscan-ignore:vue_template -->
 </template>
 
 <script lang="ts" setup>

packages/frontend/src/components/MkSignupDialog.rules.vue

@@ -24,7 +24,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 				<template #suffix><i v-if="agreeServerRules" class="ph-check ph-bold ph-lg" style="color: var(--success)"></i></template>
 
 				<ol class="_gaps_s" :class="$style.rules">
-					<li v-for="item in instance.serverRules" :class="$style.rule"><div :class="$style.ruleText" v-html="sanitizeHtml(item)"></div></li>
+					<li v-for="item in instance.serverRules" :class="$style.rule"><div :class="$style.ruleText" v-html="sanitizeHtml(item)"></div></li><!-- njsscan-ignore:vue_template -->
 				</ol>
 
 				<MkSwitch :modelValue="agreeServerRules" style="margin-top: 16px;" @update:modelValue="updateAgreeServerRules">{{ i18n.ts.agree }}</MkSwitch>

packages/frontend/src/components/MkVisitorDashboard.vue

@@ -16,7 +16,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 			</h1>
 			<div :class="$style.mainAbout">
 				<!-- eslint-disable-next-line vue/no-v-html -->
-				<div v-html="sanitizeHtml(meta.description) || i18n.ts.headlineMisskey"></div>
+				<div v-html="sanitizeHtml(meta.description) || i18n.ts.headlineMisskey"></div><!-- njsscan-ignore:vue_template -->
 			</div>
 			<div v-if="instance.disableRegistration" :class="$style.mainWarn">
 				<MkInfo warn>{{ i18n.ts.invitationRequiredToRegister }}</MkInfo>

packages/frontend/src/pages/about.vue

@@ -20,7 +20,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 
 				<MkKeyValue>
 					<template #key>{{ i18n.ts.description }}</template>
-					<template #value><div v-html="sanitizeHtml(instance.description)"></div></template>
+					<template #value><div v-html="sanitizeHtml(instance.description)"></div></template><!-- njsscan-ignore:vue_template -->
 				</MkKeyValue>
 
 				<FormSection>
@@ -29,8 +29,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 							<template #key>Sharkey</template>
 							<template #value>{{ version }}</template>
 						</MkKeyValue>
-						<div v-html="i18n.tsx.poweredByMisskeyDescription({ name: instance.name ?? host })">
+						<div v-html="i18n.tsx.poweredByMisskeyDescription({ name: instance.name ?? host })"></div><!-- njsscan-ignore:vue_template -->
-						</div>
 						<FormLink to="/about-sharkey">{{ i18n.ts.aboutMisskey }}</FormLink>
 					</div>
 				</FormSection>
@@ -53,7 +52,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 								<template #label>{{ i18n.ts.serverRules }}</template>
 
 								<ol class="_gaps_s" :class="$style.rules">
-									<li v-for="item, index in instance.serverRules" :key="index" :class="$style.rule"><div :class="$style.ruleText" v-html="sanitizeHtml(item)"></div></li>
+									<li v-for="item, index in instance.serverRules" :key="index" :class="$style.rule"><div :class="$style.ruleText" v-html="sanitizeHtml(item)"></div></li><!-- njsscan-ignore:vue_template -->
 								</ol>
 							</MkFolder>
 							<FormLink v-if="instance.tosUrl" :to="instance.tosUrl" external>{{ i18n.ts.termsOfService }}</FormLink>

packages/frontend/src/pages/admin/roles.editor.vue

@@ -32,7 +32,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 
 	<MkSelect v-model="rolePermission" :readonly="readonly">
 		<template #label><i class="ph-shield ph-bold ph-lg-lock"></i> {{ i18n.ts._role.permission }}</template>
-		<template #caption><div v-html="i18n.ts._role.descriptionOfPermission.replaceAll('\n', '<br>')"></div></template>
+		<template #caption><div v-html="i18n.ts._role.descriptionOfPermission.replaceAll('\n', '<br>')"></div></template><!-- njsscan-ignore:vue_template -->
 		<option value="normal">{{ i18n.ts.normalUser }}</option>
 		<option value="moderator">{{ i18n.ts.moderator }}</option>
 		<option value="administrator">{{ i18n.ts.administrator }}</option>
@@ -40,7 +40,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 
 	<MkSelect v-model="role.target" :readonly="readonly">
 		<template #label><i class="ph-users ph-bold ph-lg"></i> {{ i18n.ts._role.assignTarget }}</template>
-		<template #caption><div v-html="i18n.ts._role.descriptionOfAssignTarget.replaceAll('\n', '<br>')"></div></template>
+		<template #caption><div v-html="i18n.ts._role.descriptionOfAssignTarget.replaceAll('\n', '<br>')"></div></template><!-- njsscan-ignore:vue_template -->
 		<option value="manual">{{ i18n.ts._role.manual }}</option>
 		<option value="conditional">{{ i18n.ts._role.conditional }}</option>
 	</MkSelect>

packages/frontend/src/scripts/check-word-mute.ts

@@ -27,6 +27,11 @@ export function checkWordMute(note: Record<string, any>, me: Record<string, any>
 				if (!regexp) return false;
 
 				try {
+					/*
+						this could actually be a problem, but not here: here it's
+						user-supplied regexes running on the user's browser
+
+						eslint-disable-next-line detect-non-literal-regexp */
 					return new RegExp(regexp[1], regexp[2]).test(text);
 				} catch (err) {
 					// This should never happen due to input sanitisation.

packages/frontend/src/scripts/misskey-api.ts

@@ -31,7 +31,7 @@ export function misskeyApi<
 	const promise = new Promise<_ResT>((resolve, reject) => {
 		// Append a credential
 		if ($i) (data as any).i = $i.token;
-		if (token !== undefined) (data as any).i = token;
+		if (token !== undefined) (data as any).i = token; // eslint-disable-line detect-possible-timing-attacks
 
 		// Send request
 		window.fetch(`${apiUrl}/${endpoint}`, {

packages/megalodon/src/misskey.ts

@@ -1470,6 +1470,11 @@ export default class Misskey implements MegalodonInterface {
    */
   public async uploadMedia(file: any, _options?: { description?: string; focus?: string }): Promise<Response<Entity.Attachment>> {
     const formData = new FormData()
+    /*
+      this is called from MastodonApiServerService and `file` is
+      generated by `falstify` upload code, so should be safe
+
+      eslint-disable-next-line detect-non-literal-fs-filename */
     formData.append('file', fs.createReadStream(file.path), {
 			contentType: file.mimetype,
 		});

scripts/build-pre.js

@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 
+// eslint-disable detect-non-literal-fs-filename
+
 const fs = require('fs');
 const packageJsonPath = __dirname + '/../package.json'
 
@@ -13,7 +15,7 @@ function build() {
 		fs.mkdirSync(__dirname + '/../built', { recursive: true });
 		fs.writeFileSync(__dirname + '/../built/meta.json', JSON.stringify({ version: meta.version }), 'utf-8');
 	} catch (e) {
-		console.error(e)
+		console.error(e) // njsscan-ignore:generic_error_disclosure
 	}
 }
 

scripts/trim-deps.mjs

@@ -1,5 +1,6 @@
 // trims dependencies for production
 // only run after a full build
+// eslint-disable detect-non-literal-fs-filename
 
 import fs from 'node:fs'