Desktop: Re-enable media with local file URLs in the note viewer (#11244)

packages/app-desktop/app.ts

@@ -127,6 +127,12 @@ class Application extends BaseApplication {
 			bridge().setLocale(Setting.value('locale'));
 		}
 
+		if (action.type === 'SETTING_UPDATE_ONE' && action.key === 'renderer.fileUrls' || action.type === 'SETTING_UPDATE_ALL') {
+			bridge().electronApp().getCustomProtocolHandler().setMediaAccessEnabled(
+				Setting.value('renderer.fileUrls'),
+			);
+		}
+
 		if (action.type === 'SETTING_UPDATE_ONE' && action.key === 'showTrayIcon' || action.type === 'SETTING_UPDATE_ALL') {
 			this.updateTray();
 		}

packages/app-desktop/gui/NoteTextViewer.tsx

@@ -29,6 +29,7 @@ export default class NoteTextViewerComponent extends React.Component<Props, any>
 	private webviewRef_: React.RefObject<HTMLIFrameElement>;
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 	private webviewListeners_: any = null;
+
 	private removePluginAssetsCallback_: RemovePluginAssetsCallback|null = null;
 
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
@@ -110,7 +111,7 @@ export default class NoteTextViewerComponent extends React.Component<Props, any>
 		window.addEventListener('message', this.webview_message);
 	}
 
-	public destroyWebview() {
+	private destroyWebview() {
 		const wv = this.webviewRef_.current;
 		if (!wv || !this.initialized_) return;
 
@@ -194,14 +195,13 @@ export default class NoteTextViewerComponent extends React.Component<Props, any>
 		}
 	}
 
-	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 	public setHtml(html: string, options: SetHtmlOptions) {
+		const protocolHandler = bridge().electronApp().getCustomProtocolHandler();
+
 		// Grant & remove asset access.
 		if (options.pluginAssets) {
 			this.removePluginAssetsCallback_?.();
 
-			const protocolHandler = bridge().electronApp().getCustomProtocolHandler();
-
 			const pluginAssetPaths: string[] = options.pluginAssets.map((asset) => asset.path);
 			const assetAccesses = pluginAssetPaths.map(
 				path => protocolHandler.allowReadAccessToFile(path),
@@ -216,7 +216,10 @@ export default class NoteTextViewerComponent extends React.Component<Props, any>
 			};
 		}
 
-		this.send('setHtml', html, options);
+		this.send('setHtml', html, {
+			...options,
+			mediaAccessKey: protocolHandler.getMediaAccessKey(),
+		});
 	}
 
 	// ----------------------------------------------------------------

packages/app-desktop/gui/note-viewer/index.html

@@ -377,6 +377,20 @@
 			contentElement.scrollTop = scrollTop;
 		}
 
+		const rewriteFileUrls = (accessKey) => {
+			if (!accessKey) return;
+
+			// To allow accessing local files from the viewer's non-file URL, file:// URLs are re-written
+			// to joplin-content:// URLs:
+			const mediaElements = document.querySelectorAll('video[src], audio[src], source[src], img[src]');
+			for (const element of mediaElements) {
+				if (element.src?.startsWith('file:')) {
+					const newUrl = element.src.replace(/^file:\/\//, 'joplin-content://file-media/');
+					element.src = `${newUrl}?access-key=${accessKey}`;
+				}
+			}
+		};
+
 		ipc.setHtml = (event) => {
 			const html = event.html;
 
@@ -388,6 +402,10 @@
 
 			contentElement.innerHTML = html;
 
+			if (html.includes('file://')) {
+				rewriteFileUrls(event.options.mediaAccessKey);
+			}
+
 			scrollmap.create(event.options.markupLineCount);
 			if (typeof event.options.percent !== 'number') {
 				restorePercentScroll(); // First, a quick treatment is applied.

packages/app-desktop/utils/customProtocols/handleCustomProtocols.test.ts

@@ -42,21 +42,29 @@ const setUpProtocolHandler = () => {
 	return { protocolHandler, onRequestListener };
 };
 
+interface ExpectBlockedOptions {
+	host?: string;
+}
+
 // Although none of the paths in this test suite point to real files, file paths must be in
 // a certain format on Windows to avoid invalid path exceptions.
 const toPlatformPath = (path: string) => process.platform === 'win32' ? `C:/${path}` : path;
 
-const expectPathToBeBlocked = async (onRequestListener: ProtocolHandler, filePath: string) => {
+const toAccessUrl = (path: string, { host = 'note-viewer' }: ExpectBlockedOptions = {}) => {
-	const url = `joplin-content://note-viewer/${toPlatformPath(filePath)}`;
+	return `joplin-content://${host}/${toPlatformPath(path)}`;
-
-	await expect(
-		async () => await onRequestListener(new Request(url)),
-	).rejects.toThrowError('Read access not granted for URL');
 };
 
-const expectPathToBeUnblocked = async (onRequestListener: ProtocolHandler, filePath: string) => {
+const expectPathToBeBlocked = async (onRequestListener: ProtocolHandler, filePath: string, options?: ExpectBlockedOptions) => {
+	const url = toAccessUrl(filePath, options);
+	await expect(
+		async () => await onRequestListener(new Request(url)),
+	).rejects.toThrow(/Read access not granted for URL|Invalid or missing media access key|Media access denied/);
+};
+
+const expectPathToBeUnblocked = async (onRequestListener: ProtocolHandler, filePath: string, options?: ExpectBlockedOptions) => {
+	const url = toAccessUrl(filePath, options);
 	const handleRequestResult = await onRequestListener(
-		new Request(`joplin-content://note-viewer/${toPlatformPath(filePath)}`),
+		new Request(url),
 	);
 	expect(handleRequestResult.body).toBeTruthy();
 };
@@ -107,6 +115,34 @@ describe('handleCustomProtocols', () => {
 		await expectPathToBeBlocked(onRequestListener, '/test/path/a.txt');
 	});
 
+	test('should only allow access to file-media/ URLs when given the correct access key', async () => {
+		const { protocolHandler, onRequestListener } = setUpProtocolHandler();
+		const expectBlocked = (path: string) => {
+			return expectPathToBeBlocked(onRequestListener, path, { host: 'file-media' });
+		};
+		const expectUnblocked = (path: string) => {
+			return expectPathToBeUnblocked(onRequestListener, path, { host: 'file-media' });
+		};
+
+		fetchMock.mockImplementation(async (_url: string) => {
+			return new Response('', { headers: { 'Content-Type': 'image/jpeg' } });
+		});
+
+
+		const testPath = join(supportDir, 'photo.jpg');
+		await expectBlocked(testPath);
+		await expectBlocked(`${testPath}?access-key=wrongKey`);
+		await expectBlocked(`${testPath}?access-key=false`);
+
+		protocolHandler.setMediaAccessEnabled(true);
+		const key = protocolHandler.getMediaAccessKey();
+		await expectUnblocked(`${testPath}?access-key=${key}`);
+		await expectBlocked(`${testPath}?access-key=null`);
+		protocolHandler.setMediaAccessEnabled(false);
+
+		await expectBlocked(`${testPath}?access-key=${key}`);
+	});
+
 	test('should allow requesting part of a file', async () => {
 		const { protocolHandler, onRequestListener } = setUpProtocolHandler();
 

packages/app-desktop/utils/customProtocols/handleCustomProtocols.ts

@@ -7,10 +7,20 @@ import { LoggerWrapper } from '@joplin/utils/Logger';
 import * as fs from 'fs-extra';
 import { createReadStream } from 'fs';
 import { fromFilename } from '@joplin/lib/mime-utils';
+import { createSecureRandom } from '@joplin/lib/uuid';
+
+export interface AccessController {
+	remove(): void;
+}
 
 export interface CustomProtocolHandler {
+	// note-viewer/ URLs
 	allowReadAccessToDirectory(path: string): void;
-	allowReadAccessToFile(path: string): { remove(): void };
+	allowReadAccessToFile(path: string): AccessController;
+
+	// file-media/ URLs
+	setMediaAccessEnabled(enabled: boolean): void;
+	getMediaAccessKey(): string;
 }
 
 
@@ -130,8 +140,11 @@ const handleCustomProtocols = (logger: LoggerWrapper): CustomProtocolHandler =>
 		debug: () => {},
 	};
 
+	// Allow-listed files/directories for joplin-content://note-viewer/
 	const readableDirectories: string[] = [];
 	const readableFiles = new Map<string, number>();
+	// Access for joplin-content://file-media/
+	let mediaAccessKey: string|false = false;
 
 	// See also the protocol.handle example: https://www.electronjs.org/docs/latest/api/protocol#protocolhandlescheme-handler
 	protocol.handle(contentProtocolName, async request => {
@@ -147,10 +160,9 @@ const handleCustomProtocols = (logger: LoggerWrapper): CustomProtocolHandler =>
 
 		pathname = resolve(appBundleDirectory, pathname);
 
-		const allowedHosts = ['note-viewer'];
-
 		let canRead = false;
-		if (allowedHosts.includes(host)) {
+		let mediaOnly = true;
+		if (host === 'note-viewer') {
 			if (readableFiles.has(pathname)) {
 				canRead = true;
 			} else {
@@ -161,6 +173,20 @@ const handleCustomProtocols = (logger: LoggerWrapper): CustomProtocolHandler =>
 					}
 				}
 			}
+
+			mediaOnly = false;
+		} else if (host === 'file-media') {
+			if (!mediaAccessKey) {
+				throw new Error('Media access denied. This must be enabled with .setMediaAccessEnabled');
+			}
+
+			canRead = true;
+			mediaOnly = true;
+
+			const accessKey = url.searchParams.get('access-key');
+			if (accessKey !== mediaAccessKey) {
+				throw new Error(`Invalid or missing media access key (was ${accessKey}). An allow-listed ?access-key= parameter must be provided.`);
+			}
 		} else {
 			throw new Error(`Invalid URL ${request.url}`);
 		}
@@ -173,12 +199,26 @@ const handleCustomProtocols = (logger: LoggerWrapper): CustomProtocolHandler =>
 		logger.debug('protocol handler: Fetch file URL', asFileUrl);
 
 		const rangeHeader = request.headers.get('Range');
+		let response;
 		if (!rangeHeader) {
-			const response = await net.fetch(asFileUrl);
+			response = await net.fetch(asFileUrl);
-			return response;
 		} else {
-			return handleRangeRequest(request, pathname);
+			response = await handleRangeRequest(request, pathname);
 		}
+
+		if (mediaOnly) {
+			// Tells the browser to avoid MIME confusion attacks. See
+			// https://blog.mozilla.org/security/2016/08/26/mitigating-mime-confusion-attacks-in-firefox/
+			response.headers.set('X-Content-Type-Options', 'nosniff');
+
+			// This is an extra check to prevent loading text/html and arbitrary non-media content from the URL.
+			const contentType = response.headers.get('Content-Type');
+			if (!contentType || !contentType.match(/^(image|video|audio)\//)) {
+				throw new Error(`Attempted to access non-media file from ${request.url}, which is media-only. Content type was ${contentType}.`);
+			}
+		}
+
+		return response;
 	});
 
 	const appBundleDirectory = dirname(dirname(__dirname));
@@ -210,6 +250,18 @@ const handleCustomProtocols = (logger: LoggerWrapper): CustomProtocolHandler =>
 				},
 			};
 		},
+		setMediaAccessEnabled: (enabled: boolean) => {
+			if (enabled) {
+				mediaAccessKey ||= createSecureRandom();
+			} else {
+				mediaAccessKey = false;
+			}
+		},
+		// Allows access to all local media files, provided a matching ?access-key=<key> is added
+		// to the request URL.
+		getMediaAccessKey: () => {
+			return mediaAccessKey || null;
+		},
 	};
 };
 

packages/lib/models/settings/builtInMetadata.ts

@@ -926,6 +926,18 @@ const builtInMetadata = (Setting: typeof SettingType) => {
 		'markdown.plugin.insert': { storage: SettingStorage.File, isGlobal: true, value: false, type: SettingItemType.Bool, section: 'markdownPlugins', public: true, appTypes: [AppType.Mobile, AppType.Desktop], label: () => `${_('Enable ++insert++ syntax')}${wysiwygYes}` },
 		'markdown.plugin.multitable': { storage: SettingStorage.File, isGlobal: true, value: false, type: SettingItemType.Bool, section: 'markdownPlugins', public: true, appTypes: [AppType.Mobile, AppType.Desktop], label: () => `${_('Enable multimarkdown table extension')}${wysiwygNo}` },
 
+		// For now, applies only to the Markdown viewer
+		'renderer.fileUrls': {
+			storage: SettingStorage.File,
+			isGlobal: true,
+			value: false,
+			type: SettingItemType.Bool,
+			section: 'markdownPlugins',
+			public: true,
+			appTypes: [AppType.Desktop],
+			label: () => `${_('Enable file:// URLs for images and videos')}${wysiwygYes}`,
+		},
+
 		// Tray icon (called AppIndicator) doesn't work in Ubuntu
 		// http://www.webupd8.org/2017/04/fix-appindicator-not-working-for.html
 		// Might be fixed in Electron 18.x but no non-beta release yet. So for now

packages/tools/cspell/dictionary4.txt

@@ -132,3 +132,4 @@ Famegear
 rcompare
 tabindex
 Backblaze
+nosniff

readme/dev/spec/note_viewer_isolation.md

@@ -31,7 +31,7 @@ Here's an example:
 - Joplin checks to make sure the `joplin-content://` protocol has access to `/home/user/.config/joplin-desktop/path/here.css`. If it does, it fetches and returns the file.
 
 
-## `joplin-content://` only has access to specific directories
+## `joplin-content://note-viewer/` only has access to specific directories
 
 When `handleCustomProtocols` creates a handler for the `joplin-content://` protocol, it returns an object that allows certain directories to be marked as readable.
 
@@ -41,6 +41,13 @@ By default, the list of readable directories includes:
 - The resource directory
 - The profile directory
 
+## `joplin-content://file-media/` can only load specific file types
+
+To allow images and videos with `file://` URLs, Joplin maps `file://` URIs to `joplin-content://file-media/`. The `file-media/` host has the following restrictions:
+- Only files with certain extensions/content-types can be loaded.
+   - For example, `text/html` is disallowed but `image/png` is allowed.
+- A valid `?access-key=<...>` parameter must be provided with the request.
+   - A new access key is created for each render and old access keys are revoked.
 
 ## Why not the [`sandbox`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe#sandbox) property?