diff --git a/src/components/views/messages/MAudioBody.js b/src/components/views/messages/MAudioBody.js index ab53918987c..1e9962846c0 100644 --- a/src/components/views/messages/MAudioBody.js +++ b/src/components/views/messages/MAudioBody.js @@ -20,7 +20,7 @@ import React from 'react'; import MFileBody from './MFileBody'; import MatrixClientPeg from '../../../MatrixClientPeg'; -import { decryptFile, readBlobAsDataUri } from '../../../utils/DecryptFile'; +import { decryptFile } from '../../../utils/DecryptFile'; import { _t } from '../../../languageHandler'; export default class MAudioBody extends React.Component { @@ -54,7 +54,7 @@ export default class MAudioBody extends React.Component { let decryptedBlob; decryptFile(content.file).then(function(blob) { decryptedBlob = blob; - return readBlobAsDataUri(decryptedBlob); + return URL.createObjectURL(decryptedBlob); }).done((url) => { this.setState({ decryptedUrl: url, diff --git a/src/components/views/messages/MImageBody.js b/src/components/views/messages/MImageBody.js index d5ce533bda7..bb36e9b1ef9 100644 --- a/src/components/views/messages/MImageBody.js +++ b/src/components/views/messages/MImageBody.js @@ -25,7 +25,7 @@ import ImageUtils from '../../../ImageUtils'; import Modal from '../../../Modal'; import sdk from '../../../index'; import dis from '../../../dispatcher'; -import { decryptFile, readBlobAsDataUri } from '../../../utils/DecryptFile'; +import { decryptFile } from '../../../utils/DecryptFile'; import Promise from 'bluebird'; import { _t } from '../../../languageHandler'; import SettingsStore from "../../../settings/SettingsStore"; @@ -169,14 +169,14 @@ export default class extends React.Component { thumbnailPromise = decryptFile( content.info.thumbnail_file, ).then(function(blob) { - return readBlobAsDataUri(blob); + return URL.createObjectURL(blob); }); } let decryptedBlob; thumbnailPromise.then((thumbnailUrl) => { return decryptFile(content.file).then(function(blob) { decryptedBlob = blob; - return readBlobAsDataUri(blob); + return URL.createObjectURL(blob); }).then((contentUrl) => { this.setState({ decryptedUrl: contentUrl, diff --git a/src/components/views/messages/MVideoBody.js b/src/components/views/messages/MVideoBody.js index 21edbae9a2a..345c3f02e2f 100644 --- a/src/components/views/messages/MVideoBody.js +++ b/src/components/views/messages/MVideoBody.js @@ -20,7 +20,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import MFileBody from './MFileBody'; import MatrixClientPeg from '../../../MatrixClientPeg'; -import { decryptFile, readBlobAsDataUri } from '../../../utils/DecryptFile'; +import { decryptFile } from '../../../utils/DecryptFile'; import Promise from 'bluebird'; import { _t } from '../../../languageHandler'; import SettingsStore from "../../../settings/SettingsStore"; @@ -94,14 +94,14 @@ module.exports = React.createClass({ thumbnailPromise = decryptFile( content.info.thumbnail_file, ).then(function(blob) { - return readBlobAsDataUri(blob); + return URL.createObjectURL(blob); }); } let decryptedBlob; thumbnailPromise.then((thumbnailUrl) => { return decryptFile(content.file).then(function(blob) { decryptedBlob = blob; - return readBlobAsDataUri(blob); + return URL.createObjectURL(blob); }).then((contentUrl) => { this.setState({ decryptedUrl: contentUrl, diff --git a/src/utils/DecryptFile.js b/src/utils/DecryptFile.js index cb5e407407e..f8327832d11 100644 --- a/src/utils/DecryptFile.js +++ b/src/utils/DecryptFile.js @@ -1,5 +1,6 @@ /* Copyright 2016 OpenMarket Ltd +Copyright 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -22,24 +23,61 @@ import 'isomorphic-fetch'; import MatrixClientPeg from '../MatrixClientPeg'; import Promise from 'bluebird'; +// 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 Riot'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. -/** - * Read blob as a data:// URI. - * @return {Promise} A promise that resolves with the data:// URI. - */ -export function readBlobAsDataUri(file) { - const deferred = Promise.defer(); - const reader = new FileReader(); - reader.onload = function(e) { - deferred.resolve(e.target.result); - }; - reader.onerror = function(e) { - deferred.reject(e); - }; - reader.readAsDataURL(file); - return deferred.promise; -} +const ALLOWED_BLOB_MIMETYPES = { + 'image/jpeg': true, + 'image/gif': true, + 'image/png': true, + 'video/mp4': true, + 'video/webm': true, + 'video/ogg': true, + + 'audio/mp4': true, + 'audio/webm': true, + 'audio/aac': true, + 'audio/mpeg': true, + 'audio/ogg': true, + 'audio/wave': true, + 'audio/wav': true, + 'audio/x-wav': true, + 'audio/x-pn-wav': true, + 'audio/flac': true, + 'audio/x-flac': true, +} /** * Decrypt a file attached to a matrix event. @@ -61,7 +99,17 @@ export function decryptFile(file) { return encrypt.decryptAttachment(responseData, file); }).then(function(dataArray) { // Turn the array into a Blob and give it the correct MIME-type. - const blob = new Blob([dataArray], {type: file.mimetype}); + + // IMPORTANT: we must not allow scriptable mime-types into Blobs otherwise + // they introduce XSS attacks if the Blob URI is viewed directly in the + // browser (e.g. by copying the URI into a new tab or window.) + // See warning at top of file. + const mimetype = file.mimetype ? file.mimetype.split(";")[0].trim() : ''; + if (!ALLOWED_BLOB_MIMETYPES[mimetype]) { + mimetype = 'application/octet-stream'; + } + + const blob = new Blob([dataArray], {type: mimetype}); return blob; }); }