Improve handling of svg files

package.json

@@ -72,6 +72,7 @@
     "counterpart": "^0.18.6",
     "diff-dom": "^4.2.2",
     "diff-match-patch": "^1.0.5",
+    "dompurify": "^2.3.3",
     "emojibase-data": "^6.2.0",
     "emojibase-regex": "^5.1.3",
     "escape-html": "^1.0.3",

src/components/views/messages/MImageBody.tsx

@@ -108,9 +108,9 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
                 this.showImage();
                 return;
             }
-
+            // For encrypted SVGs we can just use the thumbnail url that will link to a sanitized svg Blob.
+            const httpUrl = this.isSvg && this.media.isEncrypted ? this.getThumbUrl() : this.getContentUrl();
             const content = this.props.mxEvent.getContent<IMediaEventContent>();
-            const httpUrl = this.getContentUrl();
             const params: Omit<ComponentProps<typeof ImageView>, "onFinished"> = {
                 src: httpUrl,
                 name: content.body?.length > 0 ? content.body : _t('Attachment'),
@@ -144,6 +144,11 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
         return content.info?.mimetype === "image/gif";
     };
 
+    private isSvg = (): boolean => {
+        const content = this.props.mxEvent.getContent();
+        return content.info?.mimetype === "image/svg+xml";
+    };
+
     private onImageEnter = (e: React.MouseEvent<HTMLImageElement>): void => {
         this.setState({ hover: true });
 
@@ -274,10 +279,9 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
     private async downloadImage() {
         if (this.props.mediaEventHelper.media.isEncrypted && this.state.decryptedUrl === null) {
             try {
-                const thumbnailUrl = await this.props.mediaEventHelper.thumbnailUrl.value;
                 this.setState({
                     decryptedUrl: await this.props.mediaEventHelper.sourceUrl.value,
-                    decryptedThumbnailUrl: thumbnailUrl,
+                    decryptedThumbnailUrl: await this.props.mediaEventHelper.thumbnailUrl.value,
                     decryptedBlob: await this.props.mediaEventHelper.sourceBlob.value,
                 });
             } catch (err) {
@@ -551,7 +555,8 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
 
         const contentUrl = this.getContentUrl();
         let thumbUrl;
-        if (this.props.forExport || (this.isGif() && SettingsStore.getValue("autoplayGifs"))) {
+        if (this.props.forExport || (this.isGif() && SettingsStore.getValue("autoplayGifs"))
+            || (this.isSvg && !this.media.isEncrypted)) {
             thumbUrl = contentUrl;
         } else {
             thumbUrl = this.getThumbUrl();

src/utils/DecryptFile.ts

@@ -16,23 +16,27 @@ limitations under the License.
 
 // Pull in the encryption lib so that we can decrypt attachments.
 import encrypt from 'browser-encrypt-attachment';
+import { decodeBase64 } from 'matrix-js-sdk/src/crypto/olmlib';
 
 import { mediaFromContent } from "../customisations/Media";
 import { IEncryptedFile, IMediaEventInfo } from "../customisations/models/IMediaEventContent";
 import { getBlobSafeMimeType } from "./blobs";
+import { sanitizeSvg } from './FileUtils';
 
 /**
  * Decrypt a file attached to a matrix event.
  * @param {IEncryptedFile} file The encrypted file information taken from the matrix event.
- *   This passed to [link]{@link https://github.com/matrix-org/browser-encrypt-attachments}
- *   as the encryption info object, so will also have the those keys in addition to
+ *   This passed to [link]{@link https://github.com/matrix-org/browser-encrypt-attachment}
+ *   as an encryption info object, so it will also have those keys in addition to
  *   the keys below.
  * @param {IMediaEventInfo} info The info parameter taken from the matrix event.
+ * @param {boolean} isSvgThumbnail Whether file is a encrypted svg used for thumbnails.
  * @returns {Promise<Blob>} Resolves to a Blob of the file.
  */
 export function decryptFile(
     file: IEncryptedFile,
     info?: IMediaEventInfo,
+    isSvgThumbnail = false,
 ): Promise<Blob> {
     const media = mediaFromContent({ file });
     // Download the encrypted file as an array buffer.
@@ -44,14 +48,24 @@ export function decryptFile(
         return encrypt.decryptAttachment(responseData, file);
     }).then((dataArray) => {
         // Turn the array into a Blob and give it the correct MIME-type.
-
-        // IMPORTANT: we must not allow scriptable mime-types into Blobs otherwise
+        //
+        // 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.
+        // browser (e.g. by copying the URI into a new tab or window).
+        // However, image/svg+xml can be allowed, since files of this type can be
+        // sanitized using DOMPurify.
+        // For more information refer to the comment in blobs.ts.
         let mimetype = info?.mimetype ? info.mimetype.split(";")[0].trim() : '';
         mimetype = getBlobSafeMimeType(mimetype);
 
+        // In case of svg thumbnails we generate we create a sanitized Blob.
+        // NOTE: For SVG we can also use the thumbnail for the lightbox.
+        if (mimetype === "image/svg+xml" && isSvgThumbnail) {
+            const encodedSvgFile = dataArray as unknown as string; // Data is always in form of a string
+            const decodedSvgFile = new TextDecoder("utf-8").decode(decodeBase64(encodedSvgFile));
+            return new Blob([sanitizeSvg(decodedSvgFile, true)], { type: mimetype });
+        }
+
         return new Blob([dataArray], { type: mimetype });
     });
 }

src/utils/FileUtils.ts

@@ -16,6 +16,7 @@ limitations under the License.
 */
 
 import filesize from 'filesize';
+import DOMPurify from 'dompurify';
 
 import { IMediaEventContent } from '../customisations/models/IMediaEventContent';
 import { _t } from '../languageHandler';
@@ -70,3 +71,42 @@ export function presentableTextForFile(
     }
     return text;
 }
+
+/**
+ * Sanitizes a svg file with DOMPurify in order to prevent XSS attacks
+ * when viewing svg blobs in the browser.
+ *
+ * @param {string} svgFile The original svg file.
+ * @param {boolean} allowUseTags Whether use tags will be filtered out. Default true.
+ * @return {string} The sanitized svg file.
+ */
+export function sanitizeSvg(
+    svgFile: string,
+    allowUseTags = false,
+): string {
+    let allowedTags = [];
+
+    if (allowUseTags) {
+        // Adapted from https://github.com/cure53/DOMPurify/issues/574 which prevents
+        // this exploit: https://insert-script.blogspot.com/2014/02/svg-fun-time-firefox-svg-vector.html
+        // and enables us to safely allow the popular <use> tag.
+        allowedTags = ['use'];
+
+        DOMPurify.addHook('afterSanitizeAttributes', function(node: Element) {
+            const href = node.getAttribute('xlink:href') || node.getAttribute('href');
+            if (href && !href.startsWith('#')) {
+                node.removeAttribute('xlink:href');
+                node.removeAttribute('href');
+            }
+        });
+    }
+
+    return DOMPurify.sanitize(svgFile, {
+        USE_PROFILES: {
+            svg: true,
+            html: false,
+            MathML: false,
+        },
+        ADD_TAGS: allowedTags,
+    });
+}

src/utils/MediaEventHelper.ts

@@ -85,11 +85,14 @@ export class MediaEventHelper implements IDestroyable {
     };
 
     private fetchThumbnail = () => {
-        if (!this.media.hasThumbnail) return Promise.resolve(null);
+        // For SVGs we don't have thumbnails, so we can create them using the original file content.
+        if (!this.media.hasThumbnail && !this.isSvg()) return Promise.resolve(null);
 
         if (this.media.isEncrypted) {
             const content = this.event.getContent<IMediaEventContent>();
-            if (content.info?.thumbnail_file) {
+            if (this.isSvg) {
+                return decryptFile(content.file, content.info, true);
+            } else if (content.info?.thumbnail_file) {
                 return decryptFile(content.info.thumbnail_file, content.info.thumbnail_info);
             } else {
                 // "Should never happen"
@@ -120,4 +123,9 @@ export class MediaEventHelper implements IDestroyable {
         // Finally, it's probably not media
         return false;
     }
+
+    private isSvg(): boolean {
+        const content = this.event.getContent<IMediaEventContent>();
+        return content.info?.mimetype === "image/svg+xml";
+    }
 }

src/utils/blobs.ts

@@ -14,44 +14,44 @@ 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,
+// 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
+// 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(!)
+// 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
+// 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
+// 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 MIME-type 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
+// 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/pdf, and similar.
 //
-// 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.
+// One exception is image/svg+xml. It is sanitized in DecryptFile.ts using DOMPurify.
+// Therefore, we can allow this MIME-type and don't need to convert it to
+// 'application/octet-stream'.
 
 const ALLOWED_BLOB_MIMETYPES = [
     'image/jpeg',
     'image/gif',
     'image/png',
+    'image/svg+xml',
 
     'video/mp4',
     'video/webm',

yarn.lock

@@ -3471,6 +3471,11 @@ domhandler@^4.0.0, domhandler@^4.2.0, domhandler@^4.3.0:
   dependencies:
     domelementtype "^2.2.0"
 
+dompurify@^2.3.3:
+  version "2.3.3"
+  resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-2.3.3.tgz#c1af3eb88be47324432964d8abc75cf4b98d634c"
+  integrity sha512-dqnqRkPMAjOZE0FogZ+ceJNM2dZ3V/yNOuFB7+39qpO93hHhfRpHw3heYQC7DPK9FqbQTfBKUJhiSfz4MvXYwg==
+
 domutils@^1.5.1:
   version "1.7.0"
   resolved "https://registry.yarnpkg.com/domutils/-/domutils-1.7.0.tgz#56ea341e834e06e6748af7a1cb25da67ea9f8c2a"