From 40b8db84e32a8d66a448fac620f3c4962fe4f229 Mon Sep 17 00:00:00 2001
From: Michael Telatynski <7t3chguy@gmail.com>
Date: Fri, 7 Feb 2020 22:07:29 +0000
Subject: [PATCH 1/5] Get rid of dependence on usercontent.riot.im
---
src/components/views/messages/MFileBody.js | 99 ++--------------------
1 file changed, 6 insertions(+), 93 deletions(-)
diff --git a/src/components/views/messages/MFileBody.js b/src/components/views/messages/MFileBody.js
index 737c229afe..aee7d903aa 100644
--- a/src/components/views/messages/MFileBody.js
+++ b/src/components/views/messages/MFileBody.js
@@ -26,7 +26,7 @@ import {decryptFile} from '../../../utils/DecryptFile';
import Tinter from '../../../Tinter';
import request from 'browser-request';
import Modal from '../../../Modal';
-import SdkConfig from "../../../SdkConfig";
+import AccessibleButton from "../elements/AccessibleButton";
// A cached tinted copy of require("../../../../res/img/download.svg")
@@ -94,84 +94,6 @@ Tinter.registerTintable(updateTintedDownloadImage);
// The downside of using a second domain is that it complicates hosting,
// the downside of using a sandboxed iframe is that the browers are overly
// restrictive in what you are allowed to do with the generated URL.
-//
-// For now given how unusable the blobs generated in sandboxed iframes are we
-// default to using a renderer hosted on "usercontent.riot.im". This is
-// overridable so that people running their own version of the client can
-// choose a different renderer.
-//
-// To that end the current version of the blob generation is the following
-// html:
-//
-//
-//
-// This waits to receive a message event sent using the window.postMessage API.
-// When it receives the event it evals a javascript function in data.code and
-// runs the function passing the event as an argument. This version adds
-// support for a query parameter controlling the origin from which messages
-// will be processed as an extra layer of security (note that the default URL
-// is still 'v1' since it is backwards compatible).
-//
-// In particular it means that the rendering function can be written as a
-// ordinary javascript function which then is turned into a string using
-// toString().
-//
-const DEFAULT_CROSS_ORIGIN_RENDERER = "https://usercontent.riot.im/v1.html";
-
-/**
- * Render the attachment inside the iframe.
- * We can't use imported libraries here so this has to be vanilla JS.
- */
-function remoteRender(event) {
- const data = event.data;
-
- const img = document.createElement("img");
- img.id = "img";
- img.src = data.imgSrc;
-
- const a = document.createElement("a");
- a.id = "a";
- a.rel = data.rel;
- a.target = data.target;
- a.download = data.download;
- a.style = data.style;
- a.style.fontFamily = "Arial, Helvetica, Sans-Serif";
- a.href = window.URL.createObjectURL(data.blob);
- a.appendChild(img);
- a.appendChild(document.createTextNode(data.textContent));
-
- const body = document.body;
- // Don't display scrollbars if the link takes more than one line
- // to display.
- body.style = "margin: 0px; overflow: hidden";
- body.appendChild(a);
-}
-
-/**
- * Update the tint inside the iframe.
- * We can't use imported libraries here so this has to be vanilla JS.
- */
-function remoteSetTint(event) {
- const data = event.data;
-
- const img = document.getElementById("img");
- img.src = data.imgSrc;
- img.style = data.imgStyle;
-
- const a = document.getElementById("a");
- a.style = data.style;
-}
-
/**
* Get the current CSS style for a DOMElement.
@@ -283,7 +205,6 @@ export default createReactClass({
// will be inside the iframe so we wont be able to update
// it directly.
this._iframe.current.contentWindow.postMessage({
- code: remoteSetTint.toString(),
imgSrc: tintedDownloadImageURL,
style: computedStyle(this._dummyLink.current),
}, "*");
@@ -306,7 +227,7 @@ export default createReactClass({
// Wait for the user to click on the link before downloading
// and decrypting the attachment.
let decrypting = false;
- const decrypt = () => {
+ const decrypt = (e) => {
if (decrypting) {
return false;
}
@@ -323,16 +244,15 @@ export default createReactClass({
});
}).finally(() => {
decrypting = false;
- return;
});
};
return (
);
@@ -341,7 +261,6 @@ export default createReactClass({
// When the iframe loads we tell it to render a download link
const onIframeLoad = (ev) => {
ev.target.contentWindow.postMessage({
- code: remoteRender.toString(),
imgSrc: tintedDownloadImageURL,
style: computedStyle(this._dummyLink.current),
blob: this.state.decryptedBlob,
@@ -355,13 +274,7 @@ export default createReactClass({
}, "*");
};
- // If the attachment is encryped then put the link inside an iframe.
- let renderer_url = DEFAULT_CROSS_ORIGIN_RENDERER;
- const appConfig = SdkConfig.get();
- if (appConfig && appConfig.cross_origin_renderer_url) {
- renderer_url = appConfig.cross_origin_renderer_url;
- }
- renderer_url += "?origin=" + encodeURIComponent(window.location.origin);
+ // If the attachment is encrypted then put the link inside an iframe.
return (
-
+
);
From 627a4d4ea44da90b3fa8ec611d90815e00272024 Mon Sep 17 00:00:00 2001
From: Michael Telatynski <7t3chguy@gmail.com>
Date: Thu, 13 Feb 2020 16:53:45 +0000
Subject: [PATCH 2/5] Update comments and such
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
---
src/components/views/messages/MFileBody.js | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/components/views/messages/MFileBody.js b/src/components/views/messages/MFileBody.js
index aee7d903aa..f67cd1b2b0 100644
--- a/src/components/views/messages/MFileBody.js
+++ b/src/components/views/messages/MFileBody.js
@@ -268,12 +268,12 @@ export default createReactClass({
// will have the correct name when the user tries to download it.
// We can't provide a Content-Disposition header like we would for HTTP.
download: fileName,
- rel: "noopener",
- target: "_blank",
textContent: _t("Download %(text)s", { text: text }),
}, "*");
};
+ const url = "usercontent/"; // XXX: this path should probably be passed from the skin
+
// If the attachment is encrypted then put the link inside an iframe.
return (
@@ -286,7 +286,11 @@ export default createReactClass({
*/ }
-
+
);
From 4ed27a4ba65ad5cd5b27d26b45f0a319bef87eae Mon Sep 17 00:00:00 2001
From: Michael Telatynski <7t3chguy@gmail.com>
Date: Thu, 13 Feb 2020 16:58:27 +0000
Subject: [PATCH 3/5] Move bulk to react-sdk and reference it from riot-web
land
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
---
docs/usercontent.md | 27 +++++++++++++++++++++
src/usercontent/index.html | 12 ++++++++++
src/usercontent/index.js | 49 ++++++++++++++++++++++++++++++++++++++
3 files changed, 88 insertions(+)
create mode 100644 docs/usercontent.md
create mode 100644 src/usercontent/index.html
create mode 100644 src/usercontent/index.js
diff --git a/docs/usercontent.md b/docs/usercontent.md
new file mode 100644
index 0000000000..e54851dd0d
--- /dev/null
+++ b/docs/usercontent.md
@@ -0,0 +1,27 @@
+# Usercontent
+
+While decryption itself is safe to be done without a sandbox,
+letting the browser and user interact with the resulting data may be dangerous,
+previously `usercontent.riot.im` was used to act as a sandbox on a different origin to close the attack surface,
+it is now possible to do by using a combination of a sandboxed iframe and some code written into the app which consumes this SDK.
+
+Usercontent is an iframe sandbox target for allowing a user to safely download a decrypted attachment from a sandboxed origin where it cannot be used to XSS your riot session out from under you.
+
+Its function is to create an Object URL for the user/browser to use but bound to an origin different to that of the riot instance to protect against XSS.
+
+It exposes a function over a postMessage API, when sent an object with the matching fields to render a download link with the Object URL:
+
+```json5
+{
+ "imgSrc": "", // the src of the image to display in the download link
+ "imgStyle": "", // the style to apply to the image
+ "style": "", // the style to apply to the download link
+ "download": "", // download attribute to pass to the tag
+ "textContent": "", // the text to put inside the download link
+ "blob": "", // the data blob to wrap in an object url and allow the user to download
+}
+```
+
+If only imgSrc, imgStyle and style are passed then just update the existing link without overwriting other things about it.
+
+It is expected that this target be available at `usercontent/` relative to the root of the app, this can be seen in riot-web's webpack config.
diff --git a/src/usercontent/index.html b/src/usercontent/index.html
new file mode 100644
index 0000000000..90a0fe7c16
--- /dev/null
+++ b/src/usercontent/index.html
@@ -0,0 +1,12 @@
+
+
+
+
+
+
diff --git a/src/usercontent/index.js b/src/usercontent/index.js
new file mode 100644
index 0000000000..8e77f6860e
--- /dev/null
+++ b/src/usercontent/index.js
@@ -0,0 +1,49 @@
+var params = window.location.search.substring(1).split('&');
+var lockOrigin;
+for (var i = 0; i < params.length; ++i) {
+ var parts = params[i].split('=');
+ if (parts[0] === 'origin') lockOrigin = decodeURIComponent(parts[1]);
+}
+
+function remoteRender(event) {
+ const data = event.data;
+
+ const img = document.createElement("img");
+ img.id = "img";
+ img.src = data.imgSrc;
+ img.style = data.imgStyle;
+
+ const a = document.createElement("a");
+ a.id = "a";
+ a.rel = "noopener";
+ a.target = "_blank";
+ a.download = data.download;
+ a.style = data.style;
+ a.style.fontFamily = "Arial, Helvetica, Sans-Serif";
+ a.href = window.URL.createObjectURL(data.blob);
+ a.appendChild(img);
+ a.appendChild(document.createTextNode(data.textContent));
+
+ const body = document.body;
+ // Don't display scrollbars if the link takes more than one line to display.
+ body.style = "margin: 0px; overflow: hidden";
+ body.appendChild(a);
+}
+
+function remoteSetTint(event) {
+ const data = event.data;
+
+ const img = document.getElementById("img");
+ img.src = data.imgSrc;
+ img.style = data.imgStyle;
+
+ const a = document.getElementById("a");
+ a.style = data.style;
+}
+
+window.onmessage = function(e) {
+ if (lockOrigin === undefined || e.origin === lockOrigin) {
+ if (e.data.blob) remoteRender(e);
+ else remoteSetTint(e);
+ }
+};
From 8ea13d9b57ec3be24b45db98079fb2b49bd92a78 Mon Sep 17 00:00:00 2001
From: Michael Telatynski <7t3chguy@gmail.com>
Date: Thu, 13 Feb 2020 17:05:48 +0000
Subject: [PATCH 4/5] delint prefer let/const over var
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
---
src/usercontent/index.js | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/usercontent/index.js b/src/usercontent/index.js
index 8e77f6860e..0648cbb238 100644
--- a/src/usercontent/index.js
+++ b/src/usercontent/index.js
@@ -1,7 +1,7 @@
-var params = window.location.search.substring(1).split('&');
-var lockOrigin;
-for (var i = 0; i < params.length; ++i) {
- var parts = params[i].split('=');
+const params = window.location.search.substring(1).split('&');
+let lockOrigin;
+for (let i = 0; i < params.length; ++i) {
+ const parts = params[i].split('=');
if (parts[0] === 'origin') lockOrigin = decodeURIComponent(parts[1]);
}
From 4da5f3276478872404796a3e127983f2d4077f69 Mon Sep 17 00:00:00 2001
From: Michael Telatynski <7t3chguy@gmail.com>
Date: Wed, 19 Feb 2020 12:44:46 +0000
Subject: [PATCH 5/5] get rid of lockOrigin backwards compatibility
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
---
src/usercontent/index.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/usercontent/index.js b/src/usercontent/index.js
index 0648cbb238..b87ccb9dbb 100644
--- a/src/usercontent/index.js
+++ b/src/usercontent/index.js
@@ -42,7 +42,7 @@ function remoteSetTint(event) {
}
window.onmessage = function(e) {
- if (lockOrigin === undefined || e.origin === lockOrigin) {
+ if (e.origin === lockOrigin) {
if (e.data.blob) remoteRender(e);
else remoteSetTint(e);
}