From 3a6268f0c1d2ba7dfecf5266ac3ca0224b97ff0c Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Fri, 23 Oct 2020 17:18:11 +0200
Subject: [PATCH 01/22] basic PoC of image decryption working

needs looooaaads of cleanup still
---
 .../session/room/timeline/TilesCollection.js  |  3 ++
 .../session/room/timeline/tiles/ImageTile.js  | 39 ++++++++++++++++++-
 src/matrix/SessionContainer.js                |  7 +++-
 src/matrix/net/HomeServerApi.js               |  3 +-
 src/matrix/net/MediaRepository.js             | 18 +++++++--
 src/matrix/net/request/fetch.js               | 25 +++++++-----
 src/matrix/ssss/SecretStorage.js              |  7 +++-
 src/ui/web/dom/CryptoDriver.js                | 34 +++++++++++-----
 src/ui/web/session/room/timeline/ImageView.js |  4 +-
 9 files changed, 109 insertions(+), 31 deletions(-)

diff --git a/src/domain/session/room/timeline/TilesCollection.js b/src/domain/session/room/timeline/TilesCollection.js
index cf46899a..57eb75f0 100644
--- a/src/domain/session/room/timeline/TilesCollection.js
+++ b/src/domain/session/room/timeline/TilesCollection.js
@@ -95,6 +95,9 @@ export class TilesCollection extends BaseObservableList {
 
     onUnsubscribeLast() {
         this._entrySubscription = this._entrySubscription();
+        for(let i = 0; i < this._tiles.length; i+= 1) {
+            this._tiles[i].dispose();
+        }
         this._tiles = null;
     }
 
diff --git a/src/domain/session/room/timeline/tiles/ImageTile.js b/src/domain/session/room/timeline/tiles/ImageTile.js
index e72d28e4..670ec774 100644
--- a/src/domain/session/room/timeline/tiles/ImageTile.js
+++ b/src/domain/session/room/timeline/tiles/ImageTile.js
@@ -20,20 +20,47 @@ const MAX_HEIGHT = 300;
 const MAX_WIDTH = 400;
 
 export class ImageTile extends MessageTile {
+
+    constructor(options) {
+        super(options);
+        this._decryptedUrl = null;
+        this.load();
+    }
+
+    async load() {
+        const thumbnailFile = this._getContent().file;
+        if (thumbnailFile) {
+            const buffer = await this._mediaRepository.downloadEncryptedFile(thumbnailFile);
+            // TODO: fix XSS bug here by not checking mimetype
+            const blob = new Blob([buffer], {type: thumbnailFile.mimetype});
+            if (this.isDisposed) {
+                return;
+            }
+            this._decryptedUrl = URL.createObjectURL(blob);
+            this.emitChange("thumbnailUrl");
+        }
+    }
+
     get thumbnailUrl() {
+        if (this._decryptedUrl) {
+            return this._decryptedUrl;
+        }
         const mxcUrl = this._getContent()?.url;
         if (typeof mxcUrl === "string") {
             return this._mediaRepository.mxcUrlThumbnail(mxcUrl, this.thumbnailWidth, this.thumbnailHeight, "scale");
         }
-        return null;
+        return "";
     }
 
     get url() {
+        if (this._decryptedUrl) {
+            return this._decryptedUrl;
+        }
         const mxcUrl = this._getContent()?.url;
         if (typeof mxcUrl === "string") {
             return this._mediaRepository.mxcUrl(mxcUrl);
         }
-        return null;
+        return "";
     }
 
     _scaleFactor() {
@@ -62,4 +89,12 @@ export class ImageTile extends MessageTile {
     get shape() {
         return "image";
     }
+
+    dispose() {
+        if (this._decryptedUrl) {
+            URL.revokeObjectURL(this._decryptedUrl);
+            this._decryptedUrl = null;
+        }
+        super.dispose();
+    }
 }
diff --git a/src/matrix/SessionContainer.js b/src/matrix/SessionContainer.js
index d59d6c49..fdcd0a96 100644
--- a/src/matrix/SessionContainer.js
+++ b/src/matrix/SessionContainer.js
@@ -168,6 +168,11 @@ export class SessionContainer {
         }
         this._requestScheduler = new RequestScheduler({hsApi, clock: this._clock});
         this._requestScheduler.start();
+        const mediaRepository = new MediaRepository({
+            homeServer: sessionInfo.homeServer,
+            cryptoDriver: this._cryptoDriver,
+            request: this._request,
+        });
         this._session = new Session({
             storage: this._storage,
             sessionInfo: filteredSessionInfo,
@@ -176,7 +181,7 @@ export class SessionContainer {
             clock: this._clock,
             olmWorker,
             cryptoDriver: this._cryptoDriver,
-            mediaRepository: new MediaRepository(sessionInfo.homeServer)
+            mediaRepository
         });
         await this._session.load();
         if (isNewLogin) {
diff --git a/src/matrix/net/HomeServerApi.js b/src/matrix/net/HomeServerApi.js
index 3c5eda8a..354423b8 100644
--- a/src/matrix/net/HomeServerApi.js
+++ b/src/matrix/net/HomeServerApi.js
@@ -75,7 +75,8 @@ export class HomeServerApi {
             method,
             headers,
             body: bodyString,
-            timeout: options?.timeout
+            timeout: options?.timeout,
+            format: "json"
         });
 
         const wrapper = new RequestWrapper(method, url, requestResult);
diff --git a/src/matrix/net/MediaRepository.js b/src/matrix/net/MediaRepository.js
index 63fde496..1bba287d 100644
--- a/src/matrix/net/MediaRepository.js
+++ b/src/matrix/net/MediaRepository.js
@@ -15,17 +15,20 @@ limitations under the License.
 */
 
 import {encodeQueryParams} from "./common.js";
+import {decryptAttachment} from "../e2ee/attachment.js";
 
 export class MediaRepository {
-    constructor(homeserver) {
-        this._homeserver = homeserver;
+    constructor({homeServer, cryptoDriver, request}) {
+        this._homeServer = homeServer;
+        this._cryptoDriver = cryptoDriver;
+        this._request = request;
     }
 
     mxcUrlThumbnail(url, width, height, method) {
         const parts = this._parseMxcUrl(url);
         if (parts) {
             const [serverName, mediaId] = parts;
-            const httpUrl = `${this._homeserver}/_matrix/media/r0/thumbnail/${encodeURIComponent(serverName)}/${encodeURIComponent(mediaId)}`;
+            const httpUrl = `${this._homeServer}/_matrix/media/r0/thumbnail/${encodeURIComponent(serverName)}/${encodeURIComponent(mediaId)}`;
             return httpUrl + "?" + encodeQueryParams({width, height, method});
         }
         return null;
@@ -35,7 +38,7 @@ export class MediaRepository {
         const parts = this._parseMxcUrl(url);
         if (parts) {
             const [serverName, mediaId] = parts;
-            return `${this._homeserver}/_matrix/media/r0/download/${encodeURIComponent(serverName)}/${encodeURIComponent(mediaId)}`;
+            return `${this._homeServer}/_matrix/media/r0/download/${encodeURIComponent(serverName)}/${encodeURIComponent(mediaId)}`;
         } else {
             return null;
         }
@@ -49,4 +52,11 @@ export class MediaRepository {
             return null;
         }
     }
+
+    async downloadEncryptedFile(fileEntry) {
+        const url = this.mxcUrl(fileEntry.url);
+        const {body: encryptedBuffer} = await this._request(url, {format: "buffer"}).response();
+        const decryptedBuffer = await decryptAttachment(this._cryptoDriver, encryptedBuffer, fileEntry);
+        return decryptedBuffer;
+    }
 }
diff --git a/src/matrix/net/request/fetch.js b/src/matrix/net/request/fetch.js
index eaa891ed..fa7f8727 100644
--- a/src/matrix/net/request/fetch.js
+++ b/src/matrix/net/request/fetch.js
@@ -51,8 +51,9 @@ class RequestResult {
 }
 
 export function createFetchRequest(createTimeout) {
-    return function fetchRequest(url, options) {
+    return function fetchRequest(url, {method, headers, body, timeout, format}) {
         const controller = typeof AbortController === "function" ? new AbortController() : null;
+        let options = {method, body};
         if (controller) {
             options = Object.assign(options, {
                 signal: controller.signal
@@ -76,18 +77,22 @@ export function createFetchRequest(createTimeout) {
             // cache: "no-store",
             cache: "default",
         });
-        if (options.headers) {
-            const headers = new Headers();
-            for(const [name, value] of options.headers.entries()) {
-                headers.append(name, value);
+        if (headers) {
+            const fetchHeaders = new Headers();
+            for(const [name, value] of headers.entries()) {
+                fetchHeaders.append(name, value);
             }
-            options.headers = headers;
+            options.headers = fetchHeaders;
         }
         const promise = fetch(url, options).then(async response => {
             const {status} = response;
             let body;
             try {
-                body = await response.json();
+                if (format === "json") {
+                    body = await response.json();
+                } else if (format === "buffer") {
+                    body = await response.arrayBuffer();
+                }
             } catch (err) {
                 // some error pages return html instead of json, ignore error
                 if (!(err.name === "SyntaxError" && status >= 400)) {
@@ -105,14 +110,14 @@ export function createFetchRequest(createTimeout) {
                 // 
                 // One could check navigator.onLine to rule out the first
                 // but the 2 latter ones are indistinguishable from javascript.
-                throw new ConnectionError(`${options.method} ${url}: ${err.message}`);
+                throw new ConnectionError(`${method} ${url}: ${err.message}`);
             }
             throw err;
         });
         const result = new RequestResult(promise, controller);
 
-        if (options.timeout) {
-            result.promise = abortOnTimeout(createTimeout, options.timeout, result, result.promise);
+        if (timeout) {
+            result.promise = abortOnTimeout(createTimeout, timeout, result, result.promise);
         }
 
         return result;
diff --git a/src/matrix/ssss/SecretStorage.js b/src/matrix/ssss/SecretStorage.js
index a94c2e19..fdea7187 100644
--- a/src/matrix/ssss/SecretStorage.js
+++ b/src/matrix/ssss/SecretStorage.js
@@ -64,8 +64,11 @@ export class SecretStorage {
             throw new Error("Bad MAC");
         }
 
-        const plaintextBytes = await this._cryptoDriver.aes.decrypt(
-            aesKey, base64.decode(encryptedData.iv), ciphertextBytes);
+        const plaintextBytes = await this._cryptoDriver.aes.decryptCTR({
+            key: aesKey,
+            iv: base64.decode(encryptedData.iv),
+            data: ciphertextBytes
+        });
 
         return textDecoder.decode(plaintextBytes);
     }
diff --git a/src/ui/web/dom/CryptoDriver.js b/src/ui/web/dom/CryptoDriver.js
index 66bb35c0..5d392c03 100644
--- a/src/ui/web/dom/CryptoDriver.js
+++ b/src/ui/web/dom/CryptoDriver.js
@@ -158,22 +158,26 @@ class CryptoAESDriver {
     }
     /**
      * [decrypt description]
-     * @param  {BufferSource} key        [description]
+     * @param  {string} keyFormat        "raw" or "jwk"
+     * @param  {BufferSource | Object} key        [description]
      * @param  {BufferSource} iv         [description]
-     * @param  {BufferSource} ciphertext [description]
+     * @param  {BufferSource} data [description]
+     * @param  {Number}       counterLength the size of the counter, in bits
      * @return {BufferSource}            [description]
      */
-    async decrypt(key, iv, ciphertext) {
+    async decryptCTR({key, jwkKey, iv, data, counterLength = 64}) {
         const opts = {
             name: "AES-CTR",
             counter: iv,
-            length: 64,
+            length: counterLength,
         };
         let aesKey;
         try {
+            const selectedKey = key || jwkKey;
+            const format = jwkKey ? "jwk" : "raw";
             aesKey = await subtleCryptoResult(this._subtleCrypto.importKey(
-                'raw',
-                key,
+                format,
+                selectedKey,
                 opts,
                 false,
                 ['decrypt'],
@@ -186,7 +190,7 @@ class CryptoAESDriver {
                 // see https://developer.mozilla.org/en-US/docs/Web/API/AesCtrParams
                 opts,
                 aesKey,
-                ciphertext,
+                data,
             ), "decrypt");
             return new Uint8Array(plaintext);
         } catch (err) {
@@ -205,12 +209,24 @@ class CryptoLegacyAESDriver {
      * @param  {BufferSource} key        [description]
      * @param  {BufferSource} iv         [description]
      * @param  {BufferSource} ciphertext [description]
+     * @param  {Number}       counterLength the size of the counter, in bits
      * @return {BufferSource}            [description]
      */
-    async decrypt(key, iv, ciphertext) {
+    async decryptCTR({key, jwkKey, iv, data, counterLength = 64}) {
+        // TODO: support counterLength and jwkKey
         const aesjs = this._aesjs;
+        // This won't work as aesjs throws with iv.length !== 16
+        // const nonceLength = 8;
+        // const expectedIVLength = (counterLength / 8) + nonceLength;
+        // if (iv.length < expectedIVLength) {
+        //     const newIV = new Uint8Array(expectedIVLength);
+        //     for(let i = 0; i < iv.length; ++i) {
+        //         newIV[i] = iv[i];
+        //     }
+        //     iv = newIV;
+        // }
         var aesCtr = new aesjs.ModeOfOperation.ctr(new Uint8Array(key), new aesjs.Counter(new Uint8Array(iv)));
-        return aesCtr.decrypt(new Uint8Array(ciphertext));
+        return aesCtr.decrypt(new Uint8Array(data));
     }
 }
 
diff --git a/src/ui/web/session/room/timeline/ImageView.js b/src/ui/web/session/room/timeline/ImageView.js
index 69360b75..0cc918f7 100644
--- a/src/ui/web/session/room/timeline/ImageView.js
+++ b/src/ui/web/session/room/timeline/ImageView.js
@@ -23,14 +23,14 @@ export class ImageView extends TemplateView {
         const heightRatioPercent = (vm.thumbnailHeight / vm.thumbnailWidth) * 100;
         const image = t.img({
             className: "picture",
-            src: vm.thumbnailUrl,
+            src: vm => vm.thumbnailUrl,
             width: vm.thumbnailWidth,
             height: vm.thumbnailHeight,
             loading: "lazy",
             alt: vm.label,
         });
         const linkContainer = t.a({
-            href: vm.url,
+            href: vm => vm.url,
             target: "_blank",
             style: `padding-top: ${heightRatioPercent}%; width: ${vm.thumbnailWidth}px;`
         }, image);

From 6a468a08833c2547a64cb4484e3cd68755e790bd Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Fri, 23 Oct 2020 17:45:15 +0200
Subject: [PATCH 02/22] decrypt attachment code

---
 src/matrix/e2ee/attachment.js | 58 +++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 src/matrix/e2ee/attachment.js

diff --git a/src/matrix/e2ee/attachment.js b/src/matrix/e2ee/attachment.js
new file mode 100644
index 00000000..26560ca6
--- /dev/null
+++ b/src/matrix/e2ee/attachment.js
@@ -0,0 +1,58 @@
+/*
+Copyright 2020 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.
+*/
+
+import base64 from "../../../lib/base64-arraybuffer/index.js";
+
+/**
+ * Decrypt an attachment.
+ * @param {ArrayBuffer} ciphertextBuffer The encrypted attachment data buffer.
+ * @param {Object} info The information needed to decrypt the attachment.
+ * @param {Object} info.key AES-CTR JWK key object.
+ * @param {string} info.iv Base64 encoded 16 byte AES-CTR IV.
+ * @param {string} info.hashes.sha256 Base64 encoded SHA-256 hash of the ciphertext.
+ * @return {Promise} A promise that resolves with an ArrayBuffer when the attachment is decrypted.
+ */
+export async function decryptAttachment(cryptoDriver, ciphertextBuffer, info) {
+    if (info === undefined || info.key === undefined || info.iv === undefined
+        || info.hashes === undefined || info.hashes.sha256 === undefined) {
+       throw new Error("Invalid info. Missing info.key, info.iv or info.hashes.sha256 key");
+    }
+
+    var ivArray = base64.decode(info.iv);
+    // re-encode to not deal with padded vs unpadded
+    var expectedSha256base64 = base64.encode(base64.decode(info.hashes.sha256));
+    // Check the sha256 hash
+    const digestResult = await cryptoDriver.digest("SHA-256", ciphertextBuffer);
+    if (base64.encode(new Uint8Array(digestResult)) != expectedSha256base64) {
+        throw new Error("Mismatched SHA-256 digest");
+    }
+    var counterLength;
+    if (info.v == "v1" || info.v == "v2") {
+        // Version 1 and 2 use a 64 bit counter.
+        counterLength = 64;
+    } else {
+        // Version 0 uses a 128 bit counter.
+        counterLength = 128;
+    }
+
+    const decryptedBuffer = await cryptoDriver.aes.decryptCTR({
+        jwkKey: info.key,
+        iv: ivArray,
+        data: ciphertextBuffer,
+        counterLength
+    });
+    return decryptedBuffer;
+}

From cbd48aa5286197d4ef4cdf86c27f3054e6e954f3 Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Mon, 26 Oct 2020 09:49:42 +0100
Subject: [PATCH 03/22] only load main image when clicking thumbnail

---
 .../session/room/timeline/tiles/ImageTile.js  | 48 ++++++++++++-------
 src/ui/web/session/room/timeline/ImageView.js | 13 ++++-
 2 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/src/domain/session/room/timeline/tiles/ImageTile.js b/src/domain/session/room/timeline/tiles/ImageTile.js
index 670ec774..55dc5acb 100644
--- a/src/domain/session/room/timeline/tiles/ImageTile.js
+++ b/src/domain/session/room/timeline/tiles/ImageTile.js
@@ -23,26 +23,37 @@ export class ImageTile extends MessageTile {
 
     constructor(options) {
         super(options);
+        this._decryptedThumbailUrl = null;
         this._decryptedUrl = null;
         this.load();
     }
 
+    async _loadEncryptedFile(file) {
+        const buffer = await this._mediaRepository.downloadEncryptedFile(file);
+        // TODO: fix XSS bug here by not checking mimetype
+        const blob = new Blob([buffer], {type: file.mimetype});
+        if (this.isDisposed) {
+            return;
+        }
+        return URL.createObjectURL(blob);
+    }
+
     async load() {
-        const thumbnailFile = this._getContent().file;
+        const thumbnailFile = this._getContent().info?.thumbnail_file;
+        const file = this._getContent().file;
         if (thumbnailFile) {
-            const buffer = await this._mediaRepository.downloadEncryptedFile(thumbnailFile);
-            // TODO: fix XSS bug here by not checking mimetype
-            const blob = new Blob([buffer], {type: thumbnailFile.mimetype});
-            if (this.isDisposed) {
-                return;
-            }
-            this._decryptedUrl = URL.createObjectURL(blob);
+            this._decryptedThumbailUrl = await this._loadEncryptedFile(thumbnailFile);
+            this.emitChange("thumbnailUrl");
+        } else if (file) {
+            this._decryptedUrl = await this._loadEncryptedFile(file);
             this.emitChange("thumbnailUrl");
         }
     }
 
     get thumbnailUrl() {
-        if (this._decryptedUrl) {
+        if (this._decryptedThumbailUrl) {
+            return this._decryptedThumbailUrl;
+        } else if (this._decryptedUrl) {
             return this._decryptedUrl;
         }
         const mxcUrl = this._getContent()?.url;
@@ -52,15 +63,14 @@ export class ImageTile extends MessageTile {
         return "";
     }
 
-    get url() {
-        if (this._decryptedUrl) {
-            return this._decryptedUrl;
+    async loadImageUrl() {
+        if (!this._decryptedUrl) {
+            const file = this._getContent().file;
+            if (file) {
+                this._decryptedUrl = await this._loadEncryptedFile(file);
+            }
         }
-        const mxcUrl = this._getContent()?.url;
-        if (typeof mxcUrl === "string") {
-            return this._mediaRepository.mxcUrl(mxcUrl);
-        }
-        return "";
+        return this._decryptedUrl || "";
     }
 
     _scaleFactor() {
@@ -91,6 +101,10 @@ export class ImageTile extends MessageTile {
     }
 
     dispose() {
+        if (this._decryptedThumbailUrl) {
+            URL.revokeObjectURL(this._decryptedThumbailUrl);
+            this._decryptedThumbailUrl = null;
+        }
         if (this._decryptedUrl) {
             URL.revokeObjectURL(this._decryptedUrl);
             this._decryptedUrl = null;
diff --git a/src/ui/web/session/room/timeline/ImageView.js b/src/ui/web/session/room/timeline/ImageView.js
index 0cc918f7..06d479f3 100644
--- a/src/ui/web/session/room/timeline/ImageView.js
+++ b/src/ui/web/session/room/timeline/ImageView.js
@@ -30,7 +30,8 @@ export class ImageView extends TemplateView {
             alt: vm.label,
         });
         const linkContainer = t.a({
-            href: vm => vm.url,
+            href: "#",
+            onClick: evt => this.openImage(evt),
             target: "_blank",
             style: `padding-top: ${heightRatioPercent}%; width: ${vm.thumbnailWidth}px;`
         }, image);
@@ -39,4 +40,14 @@ export class ImageView extends TemplateView {
             [t.div(linkContainer), t.p(t.time(vm.date + " " + vm.time))]
         );
     }
+
+    async openImage(evt) {
+        const link = evt.currentTarget;
+        if (link.getAttribute("href") === "#") {
+            evt.preventDefault();
+            const url = await this.value.loadImageUrl();
+            link.setAttribute("href", url);
+            link.click();
+        }
+    }
 }

From 4fd71279cfb320884219782b316708084ae9eb4e Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Mon, 26 Oct 2020 09:58:39 +0100
Subject: [PATCH 04/22] don't disable cache for media repository downloads

---
 src/matrix/net/MediaRepository.js | 2 +-
 src/matrix/net/request/fetch.js   | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/matrix/net/MediaRepository.js b/src/matrix/net/MediaRepository.js
index 1bba287d..cc7721ab 100644
--- a/src/matrix/net/MediaRepository.js
+++ b/src/matrix/net/MediaRepository.js
@@ -55,7 +55,7 @@ export class MediaRepository {
 
     async downloadEncryptedFile(fileEntry) {
         const url = this.mxcUrl(fileEntry.url);
-        const {body: encryptedBuffer} = await this._request(url, {format: "buffer"}).response();
+        const {body: encryptedBuffer} = await this._request(url, {format: "buffer", cache: true}).response();
         const decryptedBuffer = await decryptAttachment(this._cryptoDriver, encryptedBuffer, fileEntry);
         return decryptedBuffer;
     }
diff --git a/src/matrix/net/request/fetch.js b/src/matrix/net/request/fetch.js
index fa7f8727..0f8f9b52 100644
--- a/src/matrix/net/request/fetch.js
+++ b/src/matrix/net/request/fetch.js
@@ -51,7 +51,7 @@ class RequestResult {
 }
 
 export function createFetchRequest(createTimeout) {
-    return function fetchRequest(url, {method, headers, body, timeout, format}) {
+    return function fetchRequest(url, {method, headers, body, timeout, format, cache = false}) {
         const controller = typeof AbortController === "function" ? new AbortController() : null;
         let options = {method, body};
         if (controller) {
@@ -59,7 +59,9 @@ export function createFetchRequest(createTimeout) {
                 signal: controller.signal
             });
         }
-        url = addCacheBuster(url);
+        if (!cache) {
+            url = addCacheBuster(url);
+        }
         options = Object.assign(options, {
             mode: "cors",
             credentials: "omit",

From a6224135e33b7d24e0c9e1e95bed28a5b0cb4069 Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Mon, 26 Oct 2020 10:14:46 +0100
Subject: [PATCH 05/22] extract blob url code to platform (WIP)

---
 .../session/room/timeline/tiles/ImageTile.js  | 40 +++++++------------
 1 file changed, 14 insertions(+), 26 deletions(-)

diff --git a/src/domain/session/room/timeline/tiles/ImageTile.js b/src/domain/session/room/timeline/tiles/ImageTile.js
index 55dc5acb..e1c5c229 100644
--- a/src/domain/session/room/timeline/tiles/ImageTile.js
+++ b/src/domain/session/room/timeline/tiles/ImageTile.js
@@ -23,38 +23,38 @@ export class ImageTile extends MessageTile {
 
     constructor(options) {
         super(options);
-        this._decryptedThumbailUrl = null;
-        this._decryptedUrl = null;
+        this._decryptedThumbail = null;
+        this._decryptedImage = null;
         this.load();
     }
 
     async _loadEncryptedFile(file) {
         const buffer = await this._mediaRepository.downloadEncryptedFile(file);
-        // TODO: fix XSS bug here by not checking mimetype
-        const blob = new Blob([buffer], {type: file.mimetype});
         if (this.isDisposed) {
             return;
         }
-        return URL.createObjectURL(blob);
+        // TODO: fix XSS bug here by not checking mimetype
+        // const blob = new Blob([buffer], {type: file.mimetype});
+        return this.track(this._platform.createBufferURI(buffer, file.mimetype));
     }
 
     async load() {
         const thumbnailFile = this._getContent().info?.thumbnail_file;
         const file = this._getContent().file;
         if (thumbnailFile) {
-            this._decryptedThumbailUrl = await this._loadEncryptedFile(thumbnailFile);
+            this._decryptedThumbail = await this._loadEncryptedFile(thumbnailFile);
             this.emitChange("thumbnailUrl");
         } else if (file) {
-            this._decryptedUrl = await this._loadEncryptedFile(file);
+            this._decryptedImage = await this._loadEncryptedFile(file);
             this.emitChange("thumbnailUrl");
         }
     }
 
     get thumbnailUrl() {
-        if (this._decryptedThumbailUrl) {
-            return this._decryptedThumbailUrl;
-        } else if (this._decryptedUrl) {
-            return this._decryptedUrl;
+        if (this._decryptedThumbail) {
+            return this._decryptedThumbail.uri;
+        } else if (this._decryptedImage) {
+            return this._decryptedImage.uri;
         }
         const mxcUrl = this._getContent()?.url;
         if (typeof mxcUrl === "string") {
@@ -64,13 +64,13 @@ export class ImageTile extends MessageTile {
     }
 
     async loadImageUrl() {
-        if (!this._decryptedUrl) {
+        if (!this._decryptedImage) {
             const file = this._getContent().file;
             if (file) {
-                this._decryptedUrl = await this._loadEncryptedFile(file);
+                this._decryptedImage = await this._loadEncryptedFile(file);
             }
         }
-        return this._decryptedUrl || "";
+        return this._decryptedImage || "";
     }
 
     _scaleFactor() {
@@ -99,16 +99,4 @@ export class ImageTile extends MessageTile {
     get shape() {
         return "image";
     }
-
-    dispose() {
-        if (this._decryptedThumbailUrl) {
-            URL.revokeObjectURL(this._decryptedThumbailUrl);
-            this._decryptedThumbailUrl = null;
-        }
-        if (this._decryptedUrl) {
-            URL.revokeObjectURL(this._decryptedUrl);
-            this._decryptedUrl = null;
-        }
-        super.dispose();
-    }
 }

From 2e0d1363c7bfee06cee107778b0bf7bdcbb37db3 Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Mon, 26 Oct 2020 10:16:23 +0100
Subject: [PATCH 06/22] fix

---
 src/domain/session/room/timeline/tiles/ImageTile.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/domain/session/room/timeline/tiles/ImageTile.js b/src/domain/session/room/timeline/tiles/ImageTile.js
index e1c5c229..2d8d912e 100644
--- a/src/domain/session/room/timeline/tiles/ImageTile.js
+++ b/src/domain/session/room/timeline/tiles/ImageTile.js
@@ -70,7 +70,7 @@ export class ImageTile extends MessageTile {
                 this._decryptedImage = await this._loadEncryptedFile(file);
             }
         }
-        return this._decryptedImage || "";
+        return this._decryptedImage?.uri || "";
     }
 
     _scaleFactor() {

From ee1e62207c3471e6f41ad6e8beda4f3e3a9456b2 Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Mon, 26 Oct 2020 17:18:17 +0100
Subject: [PATCH 07/22] apply platform changes to decrypting images

---
 .../session/room/timeline/tiles/ImageTile.js  | 10 +++----
 .../session/room/timeline/tiles/SimpleTile.js |  6 ++---
 src/platform/web/Platform.js                  |  5 ++++
 src/platform/web/dom/BufferURL.js             | 27 +++++++++++++++++++
 4 files changed, 39 insertions(+), 9 deletions(-)
 create mode 100644 src/platform/web/dom/BufferURL.js

diff --git a/src/domain/session/room/timeline/tiles/ImageTile.js b/src/domain/session/room/timeline/tiles/ImageTile.js
index 2d8d912e..ea876b51 100644
--- a/src/domain/session/room/timeline/tiles/ImageTile.js
+++ b/src/domain/session/room/timeline/tiles/ImageTile.js
@@ -20,7 +20,6 @@ const MAX_HEIGHT = 300;
 const MAX_WIDTH = 400;
 
 export class ImageTile extends MessageTile {
-
     constructor(options) {
         super(options);
         this._decryptedThumbail = null;
@@ -34,8 +33,7 @@ export class ImageTile extends MessageTile {
             return;
         }
         // TODO: fix XSS bug here by not checking mimetype
-        // const blob = new Blob([buffer], {type: file.mimetype});
-        return this.track(this._platform.createBufferURI(buffer, file.mimetype));
+        return this.track(this.platform.createBufferURL(buffer, file.mimetype));
     }
 
     async load() {
@@ -52,9 +50,9 @@ export class ImageTile extends MessageTile {
 
     get thumbnailUrl() {
         if (this._decryptedThumbail) {
-            return this._decryptedThumbail.uri;
+            return this._decryptedThumbail.url;
         } else if (this._decryptedImage) {
-            return this._decryptedImage.uri;
+            return this._decryptedImage.url;
         }
         const mxcUrl = this._getContent()?.url;
         if (typeof mxcUrl === "string") {
@@ -70,7 +68,7 @@ export class ImageTile extends MessageTile {
                 this._decryptedImage = await this._loadEncryptedFile(file);
             }
         }
-        return this._decryptedImage?.uri || "";
+        return this._decryptedImage?.url || "";
     }
 
     _scaleFactor() {
diff --git a/src/domain/session/room/timeline/tiles/SimpleTile.js b/src/domain/session/room/timeline/tiles/SimpleTile.js
index 12acd4c5..ec60bbba 100644
--- a/src/domain/session/room/timeline/tiles/SimpleTile.js
+++ b/src/domain/session/room/timeline/tiles/SimpleTile.js
@@ -18,9 +18,9 @@ import {UpdateAction} from "../UpdateAction.js";
 import {ViewModel} from "../../../../ViewModel.js";
 
 export class SimpleTile extends ViewModel {
-    constructor({entry}) {
-        super();
-        this._entry = entry;
+    constructor(options) {
+        super(options);
+        this._entry = options.entry;
     }
     // view model props for all subclasses
     // hmmm, could also do instanceof ... ?
diff --git a/src/platform/web/Platform.js b/src/platform/web/Platform.js
index 1ecae5be..dc299ef8 100644
--- a/src/platform/web/Platform.js
+++ b/src/platform/web/Platform.js
@@ -27,6 +27,7 @@ import {OnlineStatus} from "./dom/OnlineStatus.js";
 import {Crypto} from "./dom/Crypto.js";
 import {estimateStorageUsage} from "./dom/StorageEstimate.js";
 import {WorkerPool} from "./dom/WorkerPool.js";
+import {BufferURL} from "./dom/BufferURL.js";
 
 function addScript(src) {
     return new Promise(function (resolve, reject) {
@@ -127,4 +128,8 @@ export class Platform {
     setNavigation(navigation) {
         this._serviceWorkerHandler?.setNavigation(navigation);
     }
+
+    createBufferURL(buffer, mimetype) {
+        return new BufferURL(buffer, mimetype);
+    }
 }
diff --git a/src/platform/web/dom/BufferURL.js b/src/platform/web/dom/BufferURL.js
new file mode 100644
index 00000000..144acf35
--- /dev/null
+++ b/src/platform/web/dom/BufferURL.js
@@ -0,0 +1,27 @@
+/*
+Copyright 2020 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.
+*/
+
+export class BufferURL {
+    constructor(buffer, mimetype) {
+        const blob = new Blob([buffer], {type: mimetype});
+        this.url = URL.createObjectURL(blob);
+    }
+
+    dispose() {
+        URL.revokeObjectURL(this.url);
+        this.url = null;
+    }
+}

From a3aa25449ba506015314ec9cb2b8a3646e1e77ed Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Mon, 26 Oct 2020 17:37:32 +0100
Subject: [PATCH 08/22] make it work with xhr

---
 src/matrix/net/MediaRepository.js   |  2 +-
 src/platform/web/dom/request/xhr.js | 26 +++++++++++++++++---------
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/matrix/net/MediaRepository.js b/src/matrix/net/MediaRepository.js
index ac953448..856c6657 100644
--- a/src/matrix/net/MediaRepository.js
+++ b/src/matrix/net/MediaRepository.js
@@ -55,7 +55,7 @@ export class MediaRepository {
 
     async downloadEncryptedFile(fileEntry) {
         const url = this.mxcUrl(fileEntry.url);
-        const {body: encryptedBuffer} = await this._request(url, {format: "buffer", cache: true}).response();
+        const {body: encryptedBuffer} = await this._request(url, {method: "GET", format: "buffer", cache: true}).response();
         const decryptedBuffer = await decryptAttachment(this._crypto, encryptedBuffer, fileEntry);
         return decryptedBuffer;
     }
diff --git a/src/platform/web/dom/request/xhr.js b/src/platform/web/dom/request/xhr.js
index 38a189fd..e397c336 100644
--- a/src/platform/web/dom/request/xhr.js
+++ b/src/platform/web/dom/request/xhr.js
@@ -35,19 +35,22 @@ class RequestResult {
     }
 }
 
-function send(url, options) {
+function send(url, {method, headers, timeout, body, format}) {
     const xhr = new XMLHttpRequest();
-    xhr.open(options.method, url);
-    if (options.headers) {
-        for(const [name, value] of options.headers.entries()) {
+    if (format === "buffer") {
+        xhr.responseType = "arraybuffer";
+    }
+    xhr.open(method, url);
+    if (headers) {
+        for(const [name, value] of headers.entries()) {
             xhr.setRequestHeader(name, value);
         }
     }
-    if (options.timeout) {
-        xhr.timeout = options.timeout;
+    if (timeout) {
+        xhr.timeout = timeout;
     }
 
-    xhr.send(options.body || null);
+    xhr.send(body || null);
 
     return xhr;
 }
@@ -62,12 +65,17 @@ function xhrAsPromise(xhr, method, url) {
 }
 
 export function xhrRequest(url, options) {
-    url = addCacheBuster(url);
+    const {cache, format} = options;
+    if (!cache) {
+        url = addCacheBuster(url);
+    }
     const xhr = send(url, options);
     const promise = xhrAsPromise(xhr, options.method, url).then(xhr => {
         const {status} = xhr;
         let body = null;
-        if (xhr.getResponseHeader("Content-Type") === "application/json") {
+        if (format === "buffer") {
+            body = xhr.response;
+        } else if (xhr.getResponseHeader("Content-Type") === "application/json") {
             body = JSON.parse(xhr.responseText);
         }
         return {status, body};

From a61d7fc68adfd0e329d03aa41b778f99992d97dc Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Tue, 27 Oct 2020 13:21:12 +0100
Subject: [PATCH 09/22] jwk key support for aesjs

---
 src/platform/web/dom/Crypto.js | 35 +++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/src/platform/web/dom/Crypto.js b/src/platform/web/dom/Crypto.js
index bc02dbdc..18c0ed66 100644
--- a/src/platform/web/dom/Crypto.js
+++ b/src/platform/web/dom/Crypto.js
@@ -158,8 +158,8 @@ class AESCrypto {
     }
     /**
      * [decrypt description]
-     * @param  {string} keyFormat        "raw" or "jwk"
-     * @param  {BufferSource | Object} key        [description]
+     * @param  {BufferSource} key        [description]
+     * @param  {Object} jwkKey        [description]
      * @param  {BufferSource} iv         [description]
      * @param  {BufferSource} data [description]
      * @param  {Number}       counterLength the size of the counter, in bits
@@ -200,6 +200,8 @@ class AESCrypto {
 }
 
 
+import base64 from "../../../../lib/base64-arraybuffer/index.js";
+
 class AESLegacyCrypto {
     constructor(aesjs) {
         this._aesjs = aesjs;
@@ -213,18 +215,25 @@ class AESLegacyCrypto {
      * @return {BufferSource}            [description]
      */
     async decryptCTR({key, jwkKey, iv, data, counterLength = 64}) {
-        // TODO: support counterLength and jwkKey
         const aesjs = this._aesjs;
-        // This won't work as aesjs throws with iv.length !== 16
-        // const nonceLength = 8;
-        // const expectedIVLength = (counterLength / 8) + nonceLength;
-        // if (iv.length < expectedIVLength) {
-        //     const newIV = new Uint8Array(expectedIVLength);
-        //     for(let i = 0; i < iv.length; ++i) {
-        //         newIV[i] = iv[i];
-        //     }
-        //     iv = newIV;
-        // }
+        if (counterLength !== 64) {
+            throw new Error(`Unsupported counter length: ${counterLength}`);
+        }
+        if (jwkKey) {
+            if (jwkKey.alg !== "A256CTR") {
+                throw new Error(`Unknown algorithm: ${jwkKey.alg}`);
+            }
+            if (!jwkKey.key_ops.includes("decrypt")) {
+                throw new Error(`decrypt missing from key_ops`);
+            }
+            if (jwkKey.kty !== "oct") {
+                throw new Error(`Invalid key type, "oct" expected: ${jwkKey.kty}`);
+            }
+            // need   //var chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_";
+            const base64Key = jwkKey.k.replace(/-/g, "+").replace(/_/g, "/");
+            key = base64.decode(base64Key);
+        }
+
         var aesCtr = new aesjs.ModeOfOperation.ctr(new Uint8Array(key), new aesjs.Counter(new Uint8Array(iv)));
         return aesCtr.decrypt(new Uint8Array(data));
     }

From 2ebce8eb7b22fec3ccd7bd45abf999883294acc7 Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Tue, 27 Oct 2020 13:49:36 +0100
Subject: [PATCH 10/22] set responseType after calling open

---
 src/platform/web/dom/request/xhr.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/platform/web/dom/request/xhr.js b/src/platform/web/dom/request/xhr.js
index e397c336..ab5ac9f5 100644
--- a/src/platform/web/dom/request/xhr.js
+++ b/src/platform/web/dom/request/xhr.js
@@ -37,10 +37,11 @@ class RequestResult {
 
 function send(url, {method, headers, timeout, body, format}) {
     const xhr = new XMLHttpRequest();
+    xhr.open(method, url);
     if (format === "buffer") {
+        // important to call this after calling open
         xhr.responseType = "arraybuffer";
     }
-    xhr.open(method, url);
     if (headers) {
         for(const [name, value] of headers.entries()) {
             xhr.setRequestHeader(name, value);

From ade01f6cf77ea560b731d68185af6818b76dbe2e Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Tue, 27 Oct 2020 13:51:40 +0100
Subject: [PATCH 11/22] space

---
 src/platform/web/dom/request/xhr.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/platform/web/dom/request/xhr.js b/src/platform/web/dom/request/xhr.js
index ab5ac9f5..9574c8cf 100644
--- a/src/platform/web/dom/request/xhr.js
+++ b/src/platform/web/dom/request/xhr.js
@@ -38,6 +38,7 @@ class RequestResult {
 function send(url, {method, headers, timeout, body, format}) {
     const xhr = new XMLHttpRequest();
     xhr.open(method, url);
+    
     if (format === "buffer") {
         // important to call this after calling open
         xhr.responseType = "arraybuffer";

From fcc4c21ad2aadd2d63553c6afe127f981a8ea73f Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Tue, 27 Oct 2020 14:35:33 +0100
Subject: [PATCH 12/22] filter mime types for blob urls

---
 .../session/room/timeline/tiles/ImageTile.js  |  1 -
 src/platform/web/dom/BufferURL.js             | 59 +++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/src/domain/session/room/timeline/tiles/ImageTile.js b/src/domain/session/room/timeline/tiles/ImageTile.js
index ea876b51..27e68250 100644
--- a/src/domain/session/room/timeline/tiles/ImageTile.js
+++ b/src/domain/session/room/timeline/tiles/ImageTile.js
@@ -32,7 +32,6 @@ export class ImageTile extends MessageTile {
         if (this.isDisposed) {
             return;
         }
-        // TODO: fix XSS bug here by not checking mimetype
         return this.track(this.platform.createBufferURL(buffer, file.mimetype));
     }
 
diff --git a/src/platform/web/dom/BufferURL.js b/src/platform/web/dom/BufferURL.js
index 144acf35..28730022 100644
--- a/src/platform/web/dom/BufferURL.js
+++ b/src/platform/web/dom/BufferURL.js
@@ -14,8 +14,67 @@ 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.
+//
+// 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': 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,
+};
+
 export class BufferURL {
     constructor(buffer, mimetype) {
+        mimetype = mimetype ? mimetype.split(";")[0].trim() : '';
+        if (!ALLOWED_BLOB_MIMETYPES[mimetype]) {
+            mimetype = 'application/octet-stream';
+        }
         const blob = new Blob([buffer], {type: mimetype});
         this.url = URL.createObjectURL(blob);
     }

From 932542bea00c0b3ca4c182a1306fc255bba65059 Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Tue, 27 Oct 2020 14:35:47 +0100
Subject: [PATCH 13/22] some more cleanup

---
 src/platform/web/dom/Crypto.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/platform/web/dom/Crypto.js b/src/platform/web/dom/Crypto.js
index 18c0ed66..f80c1a40 100644
--- a/src/platform/web/dom/Crypto.js
+++ b/src/platform/web/dom/Crypto.js
@@ -215,7 +215,6 @@ class AESLegacyCrypto {
      * @return {BufferSource}            [description]
      */
     async decryptCTR({key, jwkKey, iv, data, counterLength = 64}) {
-        const aesjs = this._aesjs;
         if (counterLength !== 64) {
             throw new Error(`Unsupported counter length: ${counterLength}`);
         }
@@ -229,11 +228,12 @@ class AESLegacyCrypto {
             if (jwkKey.kty !== "oct") {
                 throw new Error(`Invalid key type, "oct" expected: ${jwkKey.kty}`);
             }
-            // need   //var chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_";
-            const base64Key = jwkKey.k.replace(/-/g, "+").replace(/_/g, "/");
+            // convert base64-url to normal base64
+            const base64UrlKey = jwkKey.k;
+            const base64Key = base64UrlKey.replace(/-/g, "+").replace(/_/g, "/");
             key = base64.decode(base64Key);
         }
-
+        const aesjs = this._aesjs;
         var aesCtr = new aesjs.ModeOfOperation.ctr(new Uint8Array(key), new aesjs.Counter(new Uint8Array(iv)));
         return aesCtr.decrypt(new Uint8Array(data));
     }

From 77dca5dd558f3a6e80684daadd7e09f3371711e3 Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Tue, 27 Oct 2020 16:19:36 +0100
Subject: [PATCH 14/22] add update parameter when replacing tile

---
 src/domain/session/room/timeline/TilesCollection.js        | 7 ++++---
 src/domain/session/room/timeline/UpdateAction.js           | 4 ++--
 .../session/room/timeline/tiles/EncryptedEventTile.js      | 3 ++-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/domain/session/room/timeline/TilesCollection.js b/src/domain/session/room/timeline/TilesCollection.js
index 57eb75f0..2a53c9d4 100644
--- a/src/domain/session/room/timeline/TilesCollection.js
+++ b/src/domain/session/room/timeline/TilesCollection.js
@@ -150,7 +150,8 @@ export class TilesCollection extends BaseObservableList {
             if (action.shouldReplace) {
                 const newTile = this._tileCreator(entry);
                 if (newTile) {
-                    this._replaceTile(tileIdx, tile, newTile);
+                    this._replaceTile(tileIdx, tile, newTile, action.updateParams);
+                    newTile.setUpdateEmit(this._emitSpontanousUpdate);
                 } else {
                     this._removeTile(tileIdx, tile);
                 }
@@ -175,7 +176,7 @@ export class TilesCollection extends BaseObservableList {
         //   merge with neighbours? ... hard to imagine use case for this  ...
     }
 
-    _replaceTile(tileIdx, existingTile, newTile) {
+    _replaceTile(tileIdx, existingTile, newTile, updateParams) {
         existingTile.dispose();
         const prevTile = this._getTileAtIdx(tileIdx - 1);
         const nextTile = this._getTileAtIdx(tileIdx + 1);
@@ -184,7 +185,7 @@ export class TilesCollection extends BaseObservableList {
         newTile.updatePreviousSibling(prevTile);
         newTile.updateNextSibling(nextTile);
         nextTile?.updatePreviousSibling(newTile);
-        this.emitUpdate(tileIdx, newTile, null);
+        this.emitUpdate(tileIdx, newTile, updateParams);
     }
 
     _removeTile(tileIdx, tile) {
diff --git a/src/domain/session/room/timeline/UpdateAction.js b/src/domain/session/room/timeline/UpdateAction.js
index 33a5dbe6..0bd90d7c 100644
--- a/src/domain/session/room/timeline/UpdateAction.js
+++ b/src/domain/session/room/timeline/UpdateAction.js
@@ -50,7 +50,7 @@ export class UpdateAction {
         return new UpdateAction(false, false, false, null);
     }
 
-    static Replace() {
-        return new UpdateAction(false, false, true, null);
+    static Replace(params) {
+        return new UpdateAction(false, false, true, params);
     }
 }
diff --git a/src/domain/session/room/timeline/tiles/EncryptedEventTile.js b/src/domain/session/room/timeline/tiles/EncryptedEventTile.js
index bc4f8feb..d4dbb632 100644
--- a/src/domain/session/room/timeline/tiles/EncryptedEventTile.js
+++ b/src/domain/session/room/timeline/tiles/EncryptedEventTile.js
@@ -22,7 +22,8 @@ export class EncryptedEventTile extends MessageTile {
         const parentResult = super.updateEntry(entry, params);
         // event got decrypted, recreate the tile and replace this one with it
         if (entry.eventType !== "m.room.encrypted") {
-            return UpdateAction.Replace();
+            // the "shape" parameter trigger tile recreation in TimelineList
+            return UpdateAction.Replace("shape");
         } else {
             return parentResult;
         }

From c9efee77f2a677d87f07af7681edd786f2f6eab6 Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Tue, 27 Oct 2020 16:20:04 +0100
Subject: [PATCH 15/22] if shape is update and item should be different view,
 recreate the tile

---
 src/platform/web/ui/general/ListView.js       | 15 ++++++++
 .../web/ui/session/room/TimelineList.js       | 38 +++++++++++++++----
 2 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/src/platform/web/ui/general/ListView.js b/src/platform/web/ui/general/ListView.js
index cb5a3298..ccbf0d92 100644
--- a/src/platform/web/ui/general/ListView.js
+++ b/src/platform/web/ui/general/ListView.js
@@ -144,6 +144,21 @@ export class ListView {
         }
     }
 
+    recreateItem(index, value) {
+        if (this._childInstances) {
+            const child = this._childCreator(value);
+            let oldChild;
+            if (child) {
+                oldChild = this._childInstances.splice(index, 1, child)[0];
+                this._root.replaceChild(child.mount(this._mountArgs), oldChild.root());
+            } else {
+                oldChild = this._childInstances.splice(index, 1)[0];
+                oldChild.root().remove();
+            }
+            oldChild.unmount();
+        }
+    }
+
     onBeforeListChanged() {}
     onListChanged() {}
 }
diff --git a/src/platform/web/ui/session/room/TimelineList.js b/src/platform/web/ui/session/room/TimelineList.js
index 2072b453..0cadbb71 100644
--- a/src/platform/web/ui/session/room/TimelineList.js
+++ b/src/platform/web/ui/session/room/TimelineList.js
@@ -20,6 +20,17 @@ import {TextMessageView} from "./timeline/TextMessageView.js";
 import {ImageView} from "./timeline/ImageView.js";
 import {AnnouncementView} from "./timeline/AnnouncementView.js";
 
+function viewClassForEntry(entry) {
+    switch (entry.shape) {
+        case "gap": return GapView;
+        case "announcement": return AnnouncementView;
+        case "message":
+        case "message-status":
+            return TextMessageView;
+        case "image": return ImageView;
+    }
+}
+
 export class TimelineList extends ListView {
     constructor(viewModel) {
         const options = {
@@ -27,13 +38,9 @@ export class TimelineList extends ListView {
             list: viewModel.tiles,
         }
         super(options, entry => {
-            switch (entry.shape) {
-                case "gap": return new GapView(entry);
-                case "announcement": return new AnnouncementView(entry);
-                case "message":
-                case "message-status":
-                    return new TextMessageView(entry);
-                case "image": return new ImageView(entry);
+            const View = viewClassForEntry(entry);
+            if (View) {
+                return new View(entry);
             }
         });
         this._atBottom = false;
@@ -127,4 +134,21 @@ export class TimelineList extends ListView {
             root.scrollTop = root.scrollHeight;
         }
     }
+
+    onUpdate(index, value, param) {
+        if (param === "shape") {
+            if (this._childInstances) {
+                const ExpectedClass = viewClassForEntry(value);
+                const child = this._childInstances[index];
+                if (!ExpectedClass || !(child instanceof ExpectedClass)) {
+                    // shape was updated, so we need to recreate the tile view,
+                    // the shape parameter is set in EncryptedEventTile.updateEntry
+                    // (and perhaps elsewhere by the time you read this)
+                    super.recreateItem(index, value);
+                    return;
+                }
+            }
+        }
+        super.onUpdate(index, value, param);
+    }
 }

From e7ff6decbf0e5e0b5ffc72c3e60fa5205a4bb262 Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Tue, 27 Oct 2020 16:20:41 +0100
Subject: [PATCH 16/22] remove unneeded quotes

---
 src/domain/session/room/timeline/tiles/EncryptedEventTile.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/domain/session/room/timeline/tiles/EncryptedEventTile.js b/src/domain/session/room/timeline/tiles/EncryptedEventTile.js
index d4dbb632..23476ebb 100644
--- a/src/domain/session/room/timeline/tiles/EncryptedEventTile.js
+++ b/src/domain/session/room/timeline/tiles/EncryptedEventTile.js
@@ -39,7 +39,7 @@ export class EncryptedEventTile extends MessageTile {
         if (code === "MEGOLM_NO_SESSION") {
             return this.i18n`The sender hasn't sent us the key for this message yet.`;
         } else {
-            return decryptionError?.message || this.i18n`"Could not decrypt message because of unknown reason."`;
+            return decryptionError?.message || this.i18n`Could not decrypt message because of unknown reason.`;
         }
     }
 }

From 97c3a4b8f369a3563a3b032e5ebd7e704288cf7e Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Tue, 27 Oct 2020 16:21:08 +0100
Subject: [PATCH 17/22] store error when loading encrypted images

---
 .../session/room/timeline/tiles/ImageTile.js  | 21 ++++++++++++-------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/domain/session/room/timeline/tiles/ImageTile.js b/src/domain/session/room/timeline/tiles/ImageTile.js
index 27e68250..e86902d9 100644
--- a/src/domain/session/room/timeline/tiles/ImageTile.js
+++ b/src/domain/session/room/timeline/tiles/ImageTile.js
@@ -24,6 +24,7 @@ export class ImageTile extends MessageTile {
         super(options);
         this._decryptedThumbail = null;
         this._decryptedImage = null;
+        this._error = null;
         this.load();
     }
 
@@ -36,14 +37,18 @@ export class ImageTile extends MessageTile {
     }
 
     async load() {
-        const thumbnailFile = this._getContent().info?.thumbnail_file;
-        const file = this._getContent().file;
-        if (thumbnailFile) {
-            this._decryptedThumbail = await this._loadEncryptedFile(thumbnailFile);
-            this.emitChange("thumbnailUrl");
-        } else if (file) {
-            this._decryptedImage = await this._loadEncryptedFile(file);
-            this.emitChange("thumbnailUrl");
+        try {
+            const thumbnailFile = this._getContent().info?.thumbnail_file;
+            const file = this._getContent().file;
+            if (thumbnailFile) {
+                this._decryptedThumbail = await this._loadEncryptedFile(thumbnailFile);
+                this.emitChange("thumbnailUrl");
+            } else if (file) {
+                this._decryptedImage = await this._loadEncryptedFile(file);
+                this.emitChange("thumbnailUrl");
+            }
+        } catch (err) {
+            this._error = err;
         }
     }
 

From d55f38a9e92af35f6f91f495f9f225ebdfdc45dc Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Tue, 27 Oct 2020 16:26:45 +0100
Subject: [PATCH 18/22] don't reimplement removing an item from the list

---
 src/platform/web/ui/general/ListView.js | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/src/platform/web/ui/general/ListView.js b/src/platform/web/ui/general/ListView.js
index ccbf0d92..3ec7207c 100644
--- a/src/platform/web/ui/general/ListView.js
+++ b/src/platform/web/ui/general/ListView.js
@@ -147,15 +147,13 @@ export class ListView {
     recreateItem(index, value) {
         if (this._childInstances) {
             const child = this._childCreator(value);
-            let oldChild;
-            if (child) {
-                oldChild = this._childInstances.splice(index, 1, child)[0];
-                this._root.replaceChild(child.mount(this._mountArgs), oldChild.root());
+            if (!child) {
+                this.onRemove(index, value);
             } else {
-                oldChild = this._childInstances.splice(index, 1)[0];
-                oldChild.root().remove();
+                const [oldChild] = this._childInstances.splice(index, 1, child);
+                this._root.replaceChild(child.mount(this._mountArgs), oldChild.root());
+                oldChild.unmount();
             }
-            oldChild.unmount();
         }
     }
 

From b69464b87da7aba5a10b88219290f71bf3998e87 Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Tue, 27 Oct 2020 16:47:22 +0100
Subject: [PATCH 19/22] improve picture styling

---
 src/platform/web/ui/css/themes/element/theme.css | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/platform/web/ui/css/themes/element/theme.css b/src/platform/web/ui/css/themes/element/theme.css
index b880c83d..d35e318f 100644
--- a/src/platform/web/ui/css/themes/element/theme.css
+++ b/src/platform/web/ui/css/themes/element/theme.css
@@ -505,6 +505,11 @@ ul.Timeline > li.messageStatus .message-container > p {
     --avatar-size: 25px;
 }
 
+.message-container img.picture {
+    margin-top: 4px;
+    border-radius: 4px;
+}
+
 .TextMessageView.continuation .message-container {
     margin-top: 0;
     margin-bottom: 0;

From 0405af011655126f6d71e551710ca9cef2d85f4a Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Thu, 29 Oct 2020 10:17:19 +0100
Subject: [PATCH 20/22] disable click-to-zoom on image

so we can release before the lightbox ix ready
---
 .../web/ui/session/room/timeline/ImageView.js    | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/src/platform/web/ui/session/room/timeline/ImageView.js b/src/platform/web/ui/session/room/timeline/ImageView.js
index 06d479f3..fb540f4b 100644
--- a/src/platform/web/ui/session/room/timeline/ImageView.js
+++ b/src/platform/web/ui/session/room/timeline/ImageView.js
@@ -27,12 +27,10 @@ export class ImageView extends TemplateView {
             width: vm.thumbnailWidth,
             height: vm.thumbnailHeight,
             loading: "lazy",
-            alt: vm.label,
+            alt: vm => vm.label,
+            title: vm => vm.label,
         });
         const linkContainer = t.a({
-            href: "#",
-            onClick: evt => this.openImage(evt),
-            target: "_blank",
             style: `padding-top: ${heightRatioPercent}%; width: ${vm.thumbnailWidth}px;`
         }, image);
 
@@ -40,14 +38,4 @@ export class ImageView extends TemplateView {
             [t.div(linkContainer), t.p(t.time(vm.date + " " + vm.time))]
         );
     }
-
-    async openImage(evt) {
-        const link = evt.currentTarget;
-        if (link.getAttribute("href") === "#") {
-            evt.preventDefault();
-            const url = await this.value.loadImageUrl();
-            link.setAttribute("href", url);
-            link.click();
-        }
-    }
 }

From d1e78a735aad04cc6a39f8c8fb88d82e6fa7288e Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Thu, 29 Oct 2020 10:18:05 +0100
Subject: [PATCH 21/22] show error in label for now

---
 src/domain/session/room/timeline/tiles/ImageTile.js | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/domain/session/room/timeline/tiles/ImageTile.js b/src/domain/session/room/timeline/tiles/ImageTile.js
index e86902d9..6c67d3d8 100644
--- a/src/domain/session/room/timeline/tiles/ImageTile.js
+++ b/src/domain/session/room/timeline/tiles/ImageTile.js
@@ -1,5 +1,6 @@
 /*
 Copyright 2020 Bruno Windels <bruno@windels.cloud>
+Copyright 2020 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.
@@ -49,6 +50,7 @@ export class ImageTile extends MessageTile {
             }
         } catch (err) {
             this._error = err;
+            this.emitChange("label");
         }
     }
 
@@ -95,6 +97,9 @@ export class ImageTile extends MessageTile {
     }
 
     get label() {
+        if (this._error) {
+            return `Could not decrypt image: ${this._error.message}`;
+        }
         return this._getContent().body;
     }
 

From 862a0ba56bfd720cf9a667cb569250d1afe034d0 Mon Sep 17 00:00:00 2001
From: Bruno Windels <brunow@matrix.org>
Date: Thu, 29 Oct 2020 10:29:08 +0100
Subject: [PATCH 22/22] show image decryption error

---
 src/domain/session/room/timeline/tiles/ImageTile.js    | 8 ++++++--
 src/platform/web/ui/css/themes/element/theme.css       | 2 +-
 src/platform/web/ui/session/room/timeline/ImageView.js | 5 ++++-
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/domain/session/room/timeline/tiles/ImageTile.js b/src/domain/session/room/timeline/tiles/ImageTile.js
index 6c67d3d8..04cff3c9 100644
--- a/src/domain/session/room/timeline/tiles/ImageTile.js
+++ b/src/domain/session/room/timeline/tiles/ImageTile.js
@@ -50,7 +50,7 @@ export class ImageTile extends MessageTile {
             }
         } catch (err) {
             this._error = err;
-            this.emitChange("label");
+            this.emitChange("error");
         }
     }
 
@@ -97,10 +97,14 @@ export class ImageTile extends MessageTile {
     }
 
     get label() {
+        return this._getContent().body;
+    }
+
+    get error() {
         if (this._error) {
             return `Could not decrypt image: ${this._error.message}`;
         }
-        return this._getContent().body;
+        return null;
     }
 
     get shape() {
diff --git a/src/platform/web/ui/css/themes/element/theme.css b/src/platform/web/ui/css/themes/element/theme.css
index d35e318f..9473b307 100644
--- a/src/platform/web/ui/css/themes/element/theme.css
+++ b/src/platform/web/ui/css/themes/element/theme.css
@@ -613,7 +613,7 @@ ul.Timeline > li.messageStatus .message-container > p {
     flex: 0 0 200px;
 }
 
-.Settings .error {
+.error {
     color: red;
     font-weight: 600;
 }
diff --git a/src/platform/web/ui/session/room/timeline/ImageView.js b/src/platform/web/ui/session/room/timeline/ImageView.js
index fb540f4b..113fb1e4 100644
--- a/src/platform/web/ui/session/room/timeline/ImageView.js
+++ b/src/platform/web/ui/session/room/timeline/ImageView.js
@@ -32,7 +32,10 @@ export class ImageView extends TemplateView {
         });
         const linkContainer = t.a({
             style: `padding-top: ${heightRatioPercent}%; width: ${vm.thumbnailWidth}px;`
-        }, image);
+        }, [
+            image,
+            t.if(vm => vm.error, t.createTemplate((t, vm) => t.p({className: "error"}, vm.error)))
+        ]);
 
         return renderMessage(t, vm,
             [t.div(linkContainer), t.p(t.time(vm.date + " " + vm.time))]