Desktop: Security: Make attachment and file links safer (#10250)

.eslintignore

@@ -474,6 +474,8 @@ packages/app-desktop/utils/7zip/pathToBundled7Zip.js
 packages/app-desktop/utils/checkForUpdatesUtils.test.js
 packages/app-desktop/utils/checkForUpdatesUtils.js
 packages/app-desktop/utils/checkForUpdatesUtilsTestData.js
+packages/app-desktop/utils/isSafeToOpen.test.js
+packages/app-desktop/utils/isSafeToOpen.js
 packages/app-desktop/utils/markupLanguageUtils.js
 packages/app-desktop/utils/restartInSafeModeFromMain.test.js
 packages/app-desktop/utils/restartInSafeModeFromMain.js

.gitignore

@@ -454,6 +454,8 @@ packages/app-desktop/utils/7zip/pathToBundled7Zip.js
 packages/app-desktop/utils/checkForUpdatesUtils.test.js
 packages/app-desktop/utils/checkForUpdatesUtils.js
 packages/app-desktop/utils/checkForUpdatesUtilsTestData.js
+packages/app-desktop/utils/isSafeToOpen.test.js
+packages/app-desktop/utils/isSafeToOpen.js
 packages/app-desktop/utils/markupLanguageUtils.js
 packages/app-desktop/utils/restartInSafeModeFromMain.test.js
 packages/app-desktop/utils/restartInSafeModeFromMain.js

packages/app-desktop/app.ts

@@ -137,6 +137,10 @@ class Application extends BaseApplication {
 			webFrame.setZoomFactor(Setting.value('windowContentZoomFactor') / 100);
 		}
 
+		if (action.type === 'SETTING_UPDATE_ONE' && action.key === 'linking.extraAllowedExtensions' || action.type === 'SETTING_UPDATE_ALL') {
+			bridge().extraAllowedOpenExtensions = Setting.value('linking.extraAllowedExtensions');
+		}
+
 		if (['EVENT_NOTE_ALARM_FIELD_CHANGE', 'NOTE_DELETE'].indexOf(action.type) >= 0) {
 			await AlarmService.updateNoteNotification(action.id, action.type === 'NOTE_DELETE');
 		}
@@ -609,6 +613,9 @@ class Application extends BaseApplication {
 		}
 
 		bridge().addEventListener('nativeThemeUpdated', this.bridge_nativeThemeUpdated);
+		bridge().setOnAllowedExtensionsChangeListener((newExtensions) => {
+			Setting.setValue('linking.extraAllowedExtensions', newExtensions);
+		});
 
 		await this.initPluginService();
 

packages/app-desktop/bridge.ts

@@ -1,7 +1,7 @@
 import ElectronAppWrapper from './ElectronAppWrapper';
 import shim from '@joplin/lib/shim';
 import { _, setLocale } from '@joplin/lib/locale';
-import { BrowserWindow, nativeTheme, nativeImage, dialog, shell, MessageBoxSyncOptions } from 'electron';
+import { BrowserWindow, nativeTheme, nativeImage, shell, dialog, MessageBoxSyncOptions } from 'electron';
 import { dirname, toSystemSlashes } from '@joplin/lib/path-utils';
 import { fileUriToPath } from '@joplin/utils/url';
 import { urlDecode } from '@joplin/lib/string-utils';
@@ -9,7 +9,8 @@ import * as Sentry from '@sentry/electron/main';
 import { homedir } from 'os';
 import { msleep } from '@joplin/utils/time';
 import { pathExists, writeFileSync } from 'fs-extra';
-import { normalize } from 'path';
+import { extname, normalize } from 'path';
+import isSafeToOpen from './utils/isSafeToOpen';
 
 interface LastSelectedPath {
 	file: string;
@@ -23,6 +24,7 @@ interface OpenDialogOptions {
 	filters?: any[];
 }
 
+type OnAllowedExtensionsChange = (newExtensions: string[])=> void;
 interface MessageDialogOptions extends Omit<MessageBoxSyncOptions, 'message'> {
 	message?: string;
 }
@@ -36,6 +38,9 @@ export class Bridge {
 	private appName_: string;
 	private appId_: string;
 
+	private extraAllowedExtensions_: string[] = [];
+	private onAllowedExtensionsChangeListener_: OnAllowedExtensionsChange = ()=>{};
+
 	public constructor(electronWrapper: ElectronAppWrapper, appId: string, appName: string, rootProfileDir: string, autoUploadCrashDumps: boolean) {
 		this.electronWrapper_ = electronWrapper;
 		this.appId_ = appId;
@@ -104,6 +109,23 @@ export class Bridge {
 		this.autoUploadCrashDumps_ = v;
 	}
 
+	public get extraAllowedOpenExtensions() {
+		return this.extraAllowedExtensions_;
+	}
+
+	public set extraAllowedOpenExtensions(newValue: string[]) {
+		const oldValue = this.extraAllowedExtensions_;
+		const changed = newValue.length !== oldValue.length || newValue.some((v, idx) => v !== oldValue[idx]);
+		if (changed) {
+			this.extraAllowedExtensions_ = newValue;
+			this.onAllowedExtensionsChangeListener_?.(this.extraAllowedExtensions_);
+		}
+	}
+
+	public setOnAllowedExtensionsChangeListener(listener: OnAllowedExtensionsChange) {
+		this.onAllowedExtensionsChangeListener_ = listener;
+	}
+
 	public async captureException(error: any) {
 		Sentry.captureException(error);
 		// We wait to give the "beforeSend" event handler time to process the crash dump and write
@@ -325,11 +347,43 @@ export class Bridge {
 			fullPath = fileUriToPath(urlDecode(fullPath), shim.platformName());
 		}
 		fullPath = normalize(fullPath);
-
 		// Note: pathExists is intended to mitigate a security issue related to network drives
 		//       on Windows.
 		if (await pathExists(fullPath)) {
-			return shell.openPath(fullPath);
+			const fileExtension = extname(fullPath);
+			const userAllowedExtension = this.extraAllowedOpenExtensions.includes(fileExtension);
+			if (userAllowedExtension || isSafeToOpen(fullPath)) {
+				return shell.openPath(fullPath);
+			} else {
+				const allowOpenId = 2;
+				const learnMoreId = 1;
+				const fileExtensionDescription = JSON.stringify(fileExtension);
+				const result = await dialog.showMessageBox(this.window(), {
+					title: _('Unknown file type'),
+					message:
+						_('Joplin doesn\'t recognise the %s extension. Opening this file could be dangerous. What would you like to do?', fileExtensionDescription),
+					type: 'warning',
+					checkboxLabel: _('Always open %s files without asking.', fileExtensionDescription),
+					buttons: [
+						_('Cancel'),
+						_('Learn more'),
+						_('Open it'),
+					],
+				});
+
+				if (result.response === learnMoreId) {
+					void this.openExternal('https://joplinapp.org/help/apps/attachments#unknown-filetype-warning');
+					return 'Learn more shown';
+				} else if (result.response !== allowOpenId) {
+					return 'Cancelled by user';
+				}
+
+				if (result.checkboxChecked) {
+					this.extraAllowedOpenExtensions = this.extraAllowedOpenExtensions.concat(fileExtension);
+				}
+
+				return shell.openPath(fullPath);
+			}
 		} else {
 			return 'Path does not exist.';
 		}

packages/app-desktop/utils/isSafeToOpen.test.ts

@@ -0,0 +1,32 @@
+import { remove, writeFile } from 'fs-extra';
+import { createTempDir } from '@joplin/lib/testing/test-utils';
+import { join } from 'path';
+import isUnsafeToOpen from './isSafeToOpen';
+
+
+describe('isUnsafeToOpen', () => {
+	test.each([
+		{ fileName: 'a.txt', expected: true },
+		{ fileName: 'a.json', expected: true },
+		{ fileName: 'a.JSON', expected: true },
+		{ fileName: 'a.tar.gz', expected: true },
+		{ fileName: 'a.exe', expected: false },
+		{ fileName: 'test.com', expected: false },
+		{ fileName: 'a.pyc', expected: false },
+		{ fileName: 'a.pyo', expected: false },
+		{ fileName: 'a.pyw', expected: false },
+		{ fileName: 'a.jar', expected: false },
+		{ fileName: 'a.bat', expected: false },
+		{ fileName: 'a.cmd', expected: false },
+		{ fileName: 'noExtension', expected: false },
+	])('should mark executable files as possibly unsafe to open (%j)', async ({ fileName, expected }) => {
+		const tempDir = await createTempDir();
+		try {
+			const fullPath = join(tempDir, fileName);
+			await writeFile(fullPath, 'test');
+			expect(await isUnsafeToOpen(fullPath)).toBe(expected);
+		} finally {
+			await remove(tempDir);
+		}
+	});
+});

packages/app-desktop/utils/isSafeToOpen.ts

@@ -0,0 +1,179 @@
+
+
+const isSafeToOpen = (path: string) => {
+	// This is intended to fix an issue where some platforms would execute attachment
+	// files without confirmation depending on the file extension (e.g. .EXE). This is
+	// mostly for Windows.
+	//
+	// Below is a list of extensions that we should be able to safely pass to shell.openItem
+	// without code execution.
+
+	// Prevent CSpell from marking file extensions as misspellings:
+	// cSpell:disable
+	const generallySafeExtensions = [
+		// Image files (see https://developer.mozilla.org/en-US/docs/Web/Media/Formats/Image_types)
+		'.apng',
+		'.avif',
+		'.bmp',
+		'.gif',
+		'.heic',
+		'.heics',
+		'.ico',
+		'.jpeg',
+		'.jpg',
+		'.pjp',
+		'.pjpeg',
+		'.png',
+		'.svg',
+		'.svgz',
+		'.webp',
+		'.tiff',
+		'.tif',
+
+		// Video files
+		'.3g2',
+		'.3gp',
+		'.avi',
+		'.divx',
+		'.h261',
+		'.h263',
+		'.h264',
+		'.jpgv',
+		'.m4p',
+		'.m4v',
+		'.mk3d',
+		'.mkv',
+		'.mov',
+		'.movie',
+		'.mp4',
+		'.mp4v',
+		'.mpeg',
+		'.mpg',
+		'.mpg4',
+		'.ogv',
+		'.webm',
+		'.wmv',
+
+		// Audio files
+		// https://developer.mozilla.org/en-US/docs/Web/Media/Formats/Containers#browser_compatibility
+		'.3gp',
+		'.aac',
+		'.flac',
+		'.m2a',
+		'.m3a',
+		'.m4a',
+		'.mp2',
+		'.mp3',
+		'.mpga',
+		'.oga',
+		'.ogg',
+		'.wav',
+		'.wma',
+
+		// Document files
+		'.epub',
+		'.pdf',
+		'.txt',
+
+		// By default (as of Feb 2024), Word/Excel disable macros by default, and should thus
+		// be reasonably safe to include here. Pay attention to these Office extensions in the future.
+		'.docx', // Word (no macros)
+		'.odp',
+		'.ods',
+		'.odt',
+		'.pptx', // PowerPoint (no macros)
+		// '.psd', // PhotoShop
+		// MS Word can try to convert RTF files, causing security issues: https://en.wikipedia.org/wiki/Rich_Text_Format#Security_concerns
+		// However, macros are disabled by default in modern Office.
+		// '.rtf',
+		'.xlsx', // Excel (no macros)
+		// '.xopp', // Xournal++ -- Can include LaTeX. Verify it's safe before including.
+
+		// Text files
+		'.c',
+		'.cjs',
+		'.cpp',
+		'.css',
+		'.csv',
+		'.cxx',
+		'.dart',
+		'.go',
+		'.h',
+		'.hh',
+		'.f', // Fortran
+		'.for',
+		'.f77',
+		'.f90',
+		'.hmm',
+		'.hpp',
+		'.hxx',
+		'.java',
+		'.json',
+		'.json5',
+		'.jsx',
+		'.log',
+		'.m',
+		'.md',
+		'.markdown',
+		'.mdx',
+		'.mjs',
+		'.mm',
+		'.patch',
+		'.r',
+		'.rmd',
+		'.rs', // Rust
+		'.sass',
+		'.scss',
+		'.sha256',
+		'.sha512',
+		'.swift',
+		'.tex',
+		'.toml',
+		'.ts',
+		'.tsv',
+		'.tsx',
+		'.xml',
+		'.yaml',
+		// .js may be set to run with Windows Script Host on some systems
+		// .ml: OCaml
+		// .py may be set to run with the system Python interpreter on some systems
+		// .rkt: Racket
+
+		// Archives
+		'.7z',
+		'.tar.gz',
+		'.tar',
+		'.zip',
+
+		// Fonts
+		'.ttf',
+		'.woff',
+		'.otf',
+
+		// Calendar
+		'.ics',
+		'.ical',
+		'.icalendar',
+
+		// Joplin
+		'.jex',
+		// '.jpl', // Plugin files
+
+		// Other
+		'.htm',
+		'.html',
+		'.enex', // Evernote
+		// '.desktop', // May be auto-executed on some Linux systems
+	];
+	// cSpell:enable
+
+	const lowercasedPath = path.trim().toLowerCase();
+	for (const ext of generallySafeExtensions) {
+		if (lowercasedPath.endsWith(ext)) {
+			return true;
+		}
+	}
+	return false;
+};
+
+export default isSafeToOpen;

packages/lib/models/Setting.ts

@@ -1559,6 +1559,15 @@ class Setting extends BaseModel {
 				isGlobal: true,
 			},
 
+			'linking.extraAllowedExtensions': {
+				value: [],
+				type: SettingItemType.Array,
+				public: false,
+				appTypes: [AppType.Desktop],
+				label: () => 'Additional file types that can be opened without confirmation.',
+				storage: SettingStorage.File,
+			},
+
 			'net.customCertificates': {
 				value: '',
 				type: SettingItemType.String,

readme/apps/attachments.md

@@ -10,4 +10,22 @@ Resources that are not attached to any note will be automatically deleted in acc
 
 ## Downloading attachments
 
-The way the attachments are downloaded during synchronisation can be customised in the [Configuration screen](https://github.com/laurent22/joplin/blob/dev/readme/apps/config_screen.md), under "Attachment download behaviour". The default option ("Always") is to download all the attachments, all the time, so that the data is available even when the device is offline. There is also the option to download the attachments manually (option "Manual"), by clicking on it, or automatically (Option "Auto"), in which case the attachments are downloaded only when a note is opened. These options should help saving disk space and network bandwidth, especially on mobile.
+The way the attachments are downloaded during synchronisation can be customised in the [Configuration screen](https://github.com/laurent22/joplin/blob/dev/readme/apps/config_screen.md), under "Attachment download behaviour". The default option ("Always") is to download all the attachments, all the time, so that the data is available even when the device is offline. There is also the option to download the attachments manually (option "Manual"), by clicking on it, or automatically (Option "Auto"), in which case the attachments are downloaded only when a note is opened. These options should help saving disk space and network bandwidth, especially on mobile.
+
+## Opening attachments
+
+**In the note viewer,** clicking an attachment link opens that attachment.
+
+**In the rich text editor,** <kbd>ctrl</kbd>+<kbd>click</kbd> opens that attachment.
+
+In both cases, the attachment is opened in the default editor for that file type. The editor can generally be customised in system settings.
+
+### Unknown file type warning
+
+Opening some types of attachments can be dangerous. Some computers try to run some attachment types as programs. This means that opening attachments from an untrusted source could allow someone to steal information from your computer or worse.
+
+**When is it safe to open an unknown file type?** If you created the file, attached it to a Joplin note, and haven't shared the note with anyone, it should be safe to open. Otherwise, avoid opening the attachment, especially if you don't recognise the file type.
+
+If you're certain that a file type is always safe to open, regardless of its source, check the `Always open this file type without asking` box to stop receiving warnings for that file type. It might make sense to do this, for example, if you work with a large number of `.doc` files and know that they're safe to open in the version of Word you have installed.
+
+**What are some examples of unsafe file types?** This depends on your computer. For example, on most Windows systems, `.COM`, `.EXE`, and `.BAT` files are unsafe.