From a85af47b0a5ed701dfc5fa4d660a94e22ef91e5a Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Thu, 16 Jul 2020 17:46:49 -0400 Subject: [PATCH 1/2] use a proper HTML sanitizer to strip , rather than a regexp --- src/components/views/elements/ReplyThread.js | 31 ++++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/components/views/elements/ReplyThread.js b/src/components/views/elements/ReplyThread.js index e96d9ced117..56b3c3ff8b8 100644 --- a/src/components/views/elements/ReplyThread.js +++ b/src/components/views/elements/ReplyThread.js @@ -27,6 +27,7 @@ import SettingsStore from "../../../settings/SettingsStore"; import escapeHtml from "escape-html"; import MatrixClientContext from "../../../contexts/MatrixClientContext"; import {Action} from "../../../dispatcher/actions"; +import sanitizeHtml from "sanitize-html"; // This component does no cycle detection, simply because the only way to make such a cycle would be to // craft event_id's, using a homeserver that generates predictable event IDs; even then the impact would @@ -92,7 +93,21 @@ export default class ReplyThread extends React.Component { // Part of Replies fallback support static stripHTMLReply(html) { - return html.replace(/^[\s\S]+?<\/mx-reply>/, ''); + // Sanitize the original HTML for inclusion in . We allow + // any HTML, since the original sender could use special tags that we + // don't recognize, but want to pass along to any recipients who do + // recognize them -- recipients should be sanitizing before displaying + // anyways. However, we sanitize to 1) remove any mx-reply, so that we + // don't generate a nested mx-reply, and 2) make sure that the HTML is + // properly formatted (e.g. tags are closed where necessary) + return sanitizeHtml( + html, + { + allowedTags: false, // false means allow everything + allowedAttributes: false, + exclusiveFilter: (frame) => frame.tag === "mx-reply", + } + ); } // Part of Replies fallback support @@ -102,15 +117,19 @@ export default class ReplyThread extends React.Component { let {body, formatted_body: html} = ev.getContent(); if (this.getParentEventId(ev)) { if (body) body = this.stripPlainReply(body); - if (html) html = this.stripHTMLReply(html); } if (!body) body = ""; // Always ensure we have a body, for reasons. - // Escape the body to use as HTML below. - // We also run a nl2br over the result to fix the fallback representation. We do this - // after converting the text to safe HTML to avoid user-provided BR's from being converted. - if (!html) html = escapeHtml(body).replace(/\n/g, '
'); + if (html) { + // sanitize the HTML before we put it in an + html = this.stripHTMLReply(html); + } else { + // Escape the body to use as HTML below. + // We also run a nl2br over the result to fix the fallback representation. We do this + // after converting the text to safe HTML to avoid user-provided BR's from being converted. + html = escapeHtml(body).replace(/\n/g, '
'); + } // dev note: do not rely on `body` being safe for HTML usage below. From b05a19ef13d19b7525fefff01b74bdd61b681fc2 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Thu, 16 Jul 2020 18:07:51 -0400 Subject: [PATCH 2/2] lint --- src/components/views/elements/ReplyThread.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/ReplyThread.js b/src/components/views/elements/ReplyThread.js index 56b3c3ff8b8..409bf9e01fe 100644 --- a/src/components/views/elements/ReplyThread.js +++ b/src/components/views/elements/ReplyThread.js @@ -106,7 +106,7 @@ export default class ReplyThread extends React.Component { allowedTags: false, // false means allow everything allowedAttributes: false, exclusiveFilter: (frame) => frame.tag === "mx-reply", - } + }, ); }