From 6e0c7611c35ecb9664da0c48c153b103d71751a9 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 6 May 2021 13:53:27 +0100 Subject: [PATCH 1/3] Extract blob-safe MIME types --- src/utils/DecryptFile.ts | 63 ++------------------------------ src/utils/blobs.ts | 78 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 60 deletions(-) create mode 100644 src/utils/blobs.ts diff --git a/src/utils/DecryptFile.ts b/src/utils/DecryptFile.ts index 93cedbc707..d073393170 100644 --- a/src/utils/DecryptFile.ts +++ b/src/utils/DecryptFile.ts @@ -17,63 +17,8 @@ limitations under the License. // Pull in the encryption lib so that we can decrypt attachments. import encrypt from 'browser-encrypt-attachment'; import {mediaFromContent} from "../customisations/Media"; -import {IEncryptedFile} from "../customisations/models/IMediaEventContent"; - -// WARNING: We have to be very careful about what mime-types we allow into blobs, -// as for performance reasons these are now rendered via URL.createObjectURL() -// rather than by converting into data: URIs. -// -// This means that the content is rendered using the origin of the script which -// called createObjectURL(), and so if the content contains any scripting then it -// will pose a XSS vulnerability when the browser renders it. This is particularly -// bad if the user right-clicks the URI and pastes it into a new window or tab, -// as the blob will then execute with access to Element's full JS environment(!) -// -// See https://github.com/matrix-org/matrix-react-sdk/pull/1820#issuecomment-385210647 -// for details. -// -// We mitigate this by only allowing mime-types into blobs which we know don't -// contain any scripting, and instantiate all others as application/octet-stream -// regardless of what mime-type the event claimed. Even if the payload itself -// is some malicious HTML, the fact we instantiate it with a media mimetype or -// application/octet-stream means the browser doesn't try to render it as such. -// -// One interesting edge case is image/svg+xml, which empirically *is* rendered -// correctly if the blob is set to the src attribute of an img tag (for thumbnails) -// *even if the mimetype is application/octet-stream*. However, empirically JS -// in the SVG isn't executed in this scenario, so we seem to be okay. -// -// Tested on Chrome 65 and Firefox 60 -// -// The list below is taken mainly from -// https://developer.mozilla.org/en-US/docs/Web/HTML/Supported_media_formats -// N.B. Matrix doesn't currently specify which mimetypes are valid in given -// events, so we pick the ones which HTML5 browsers should be able to display -// -// For the record, mime-types which must NEVER enter this list below include: -// text/html, text/xhtml, image/svg, image/svg+xml, image/pdf, and similar. - -const ALLOWED_BLOB_MIMETYPES = [ - 'image/jpeg', - 'image/gif', - 'image/png', - - 'video/mp4', - 'video/webm', - 'video/ogg', - - 'audio/mp4', - 'audio/webm', - 'audio/aac', - 'audio/mpeg', - 'audio/ogg', - 'audio/wave', - 'audio/wav', - 'audio/x-wav', - 'audio/x-pn-wav', - 'audio/flac', - 'audio/x-flac', -]; +import { IEncryptedFile } from "../customisations/models/IMediaEventContent"; +import { getBlobSafeMimeType } from "./blobs"; /** * Decrypt a file attached to a matrix event. @@ -100,9 +45,7 @@ export function decryptFile(file: IEncryptedFile): Promise { // browser (e.g. by copying the URI into a new tab or window.) // See warning at top of file. let mimetype = file.mimetype ? file.mimetype.split(";")[0].trim() : ''; - if (!ALLOWED_BLOB_MIMETYPES.includes(mimetype)) { - mimetype = 'application/octet-stream'; - } + mimetype = getBlobSafeMimeType(mimetype); return new Blob([dataArray], {type: mimetype}); }); diff --git a/src/utils/blobs.ts b/src/utils/blobs.ts new file mode 100644 index 0000000000..4e073a3936 --- /dev/null +++ b/src/utils/blobs.ts @@ -0,0 +1,78 @@ +/* +Copyright 2021 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// WARNING: We have to be very careful about what mime-types we allow into blobs, +// as for performance reasons these are now rendered via URL.createObjectURL() +// rather than by converting into data: URIs. +// +// This means that the content is rendered using the origin of the script which +// called createObjectURL(), and so if the content contains any scripting then it +// will pose a XSS vulnerability when the browser renders it. This is particularly +// bad if the user right-clicks the URI and pastes it into a new window or tab, +// as the blob will then execute with access to Element's full JS environment(!) +// +// See https://github.com/matrix-org/matrix-react-sdk/pull/1820#issuecomment-385210647 +// for details. +// +// We mitigate this by only allowing mime-types into blobs which we know don't +// contain any scripting, and instantiate all others as application/octet-stream +// regardless of what mime-type the event claimed. Even if the payload itself +// is some malicious HTML, the fact we instantiate it with a media mimetype or +// application/octet-stream means the browser doesn't try to render it as such. +// +// One interesting edge case is image/svg+xml, which empirically *is* rendered +// correctly if the blob is set to the src attribute of an img tag (for thumbnails) +// *even if the mimetype is application/octet-stream*. However, empirically JS +// in the SVG isn't executed in this scenario, so we seem to be okay. +// +// Tested on Chrome 65 and Firefox 60 +// +// The list below is taken mainly from +// https://developer.mozilla.org/en-US/docs/Web/HTML/Supported_media_formats +// N.B. Matrix doesn't currently specify which mimetypes are valid in given +// events, so we pick the ones which HTML5 browsers should be able to display +// +// For the record, mime-types which must NEVER enter this list below include: +// text/html, text/xhtml, image/svg, image/svg+xml, image/pdf, and similar. + +const ALLOWED_BLOB_MIMETYPES = [ + 'image/jpeg', + 'image/gif', + 'image/png', + + 'video/mp4', + 'video/webm', + 'video/ogg', + + 'audio/mp4', + 'audio/webm', + 'audio/aac', + 'audio/mpeg', + 'audio/ogg', + 'audio/wave', + 'audio/wav', + 'audio/x-wav', + 'audio/x-pn-wav', + 'audio/flac', + 'audio/x-flac', +]; + +export function getBlobSafeMimeType(mimetype: string): string { + if (!ALLOWED_BLOB_MIMETYPES.includes(mimetype)) { + return 'application/octet-stream'; + } + return mimetype; +} From 437f13cf764cfcc289bddb758f6eff73f4c93649 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 6 May 2021 14:11:34 +0100 Subject: [PATCH 2/3] Convert UploadConfirmDialog to TSX --- ...nfirmDialog.js => UploadConfirmDialog.tsx} | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) rename src/components/views/dialogs/{UploadConfirmDialog.js => UploadConfirmDialog.tsx} (80%) diff --git a/src/components/views/dialogs/UploadConfirmDialog.js b/src/components/views/dialogs/UploadConfirmDialog.tsx similarity index 80% rename from src/components/views/dialogs/UploadConfirmDialog.js rename to src/components/views/dialogs/UploadConfirmDialog.tsx index 2ff16b9440..8fdfbfda12 100644 --- a/src/components/views/dialogs/UploadConfirmDialog.js +++ b/src/components/views/dialogs/UploadConfirmDialog.tsx @@ -1,5 +1,5 @@ /* -Copyright 2019 New Vector Ltd +Copyright 2019, 2021 The Matrix.org Foundation C.I.C. Copyright 2019 Michael Telatynski <7t3chguy@gmail.com> Licensed under the Apache License, Version 2.0 (the "License"); @@ -16,20 +16,21 @@ limitations under the License. */ import React from 'react'; -import PropTypes from 'prop-types'; import * as sdk from '../../../index'; import { _t } from '../../../languageHandler'; import filesize from "filesize"; -import {replaceableComponent} from "../../../utils/replaceableComponent"; +import { replaceableComponent } from "../../../utils/replaceableComponent"; + +interface IProps { + file: File; + currentIndex: number; + totalFiles?: number; + onFinished: (uploadConfirmed: boolean, uploadAll?: boolean) => void; +} @replaceableComponent("views.dialogs.UploadConfirmDialog") -export default class UploadConfirmDialog extends React.Component { - static propTypes = { - file: PropTypes.object.isRequired, - currentIndex: PropTypes.number, - totalFiles: PropTypes.number, - onFinished: PropTypes.func.isRequired, - } +export default class UploadConfirmDialog extends React.Component { + private objectUrl: string; static defaultProps = { totalFiles: 1, @@ -38,22 +39,22 @@ export default class UploadConfirmDialog extends React.Component { constructor(props) { super(props); - this._objectUrl = URL.createObjectURL(props.file); + this.objectUrl = URL.createObjectURL(props.file); } componentWillUnmount() { - if (this._objectUrl) URL.revokeObjectURL(this._objectUrl); + if (this.objectUrl) URL.revokeObjectURL(this.objectUrl); } - _onCancelClick = () => { + private onCancelClick = () => { this.props.onFinished(false); } - _onUploadClick = () => { + private onUploadClick = () => { this.props.onFinished(true); } - _onUploadAllClick = () => { + private onUploadAllClick = () => { this.props.onFinished(true, true); } @@ -78,7 +79,7 @@ export default class UploadConfirmDialog extends React.Component { if (this.props.file.type.startsWith('image/')) { preview =
-
+
{this.props.file.name} ({filesize(this.props.file.size)})
; @@ -95,7 +96,7 @@ export default class UploadConfirmDialog extends React.Component { let uploadAllButton; if (this.props.currentIndex + 1 < this.props.totalFiles) { - uploadAllButton = ; } @@ -103,7 +104,7 @@ export default class UploadConfirmDialog extends React.Component { return ( @@ -113,7 +114,7 @@ export default class UploadConfirmDialog extends React.Component { {uploadAllButton} From dc50d27985a94a2e8f5290e4ac645461f0d272af Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 6 May 2021 14:39:44 +0100 Subject: [PATCH 3/3] Adjust MIME type of upload confirmation if needed This filters the MIME type of uploaded files to ensure we display safely. --- src/components/views/dialogs/UploadConfirmDialog.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/components/views/dialogs/UploadConfirmDialog.tsx b/src/components/views/dialogs/UploadConfirmDialog.tsx index 8fdfbfda12..7f6bcd27d1 100644 --- a/src/components/views/dialogs/UploadConfirmDialog.tsx +++ b/src/components/views/dialogs/UploadConfirmDialog.tsx @@ -20,6 +20,7 @@ import * as sdk from '../../../index'; import { _t } from '../../../languageHandler'; import filesize from "filesize"; import { replaceableComponent } from "../../../utils/replaceableComponent"; +import { getBlobSafeMimeType } from '../../../utils/blobs'; interface IProps { file: File; @@ -31,6 +32,7 @@ interface IProps { @replaceableComponent("views.dialogs.UploadConfirmDialog") export default class UploadConfirmDialog extends React.Component { private objectUrl: string; + private mimeType: string; static defaultProps = { totalFiles: 1, @@ -39,7 +41,13 @@ export default class UploadConfirmDialog extends React.Component { constructor(props) { super(props); - this.objectUrl = URL.createObjectURL(props.file); + // Create a fresh `Blob` for previewing (even though `File` already is + // one) so we can adjust the MIME type if needed. + this.mimeType = getBlobSafeMimeType(props.file.type); + const blob = new Blob([props.file], { type: + this.mimeType, + }); + this.objectUrl = URL.createObjectURL(blob); } componentWillUnmount() { @@ -76,7 +84,7 @@ export default class UploadConfirmDialog extends React.Component { } let preview; - if (this.props.file.type.startsWith('image/')) { + if (this.mimeType.startsWith('image/')) { preview =