CVE-2020-9444: Prevent reverse tabnabbing attacks.

While we could fix this issue by changing the markdown processor,
doing so is not a robust solution, because even a momentary bug in the
markdown processor could allow cached messages that do not follow our
security policy.

This change ensures that even if our markdown processor has bugs that
result in rendered content that does not properly follow our policy of
using rel="noopener noreferrer" on links, we'll still do something
reasonable.

Co-authored-by: Tim Abbott <tabbott@zulipchat.com>
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This commit is contained in:
Anders Kaseorg
2020-02-28 14:59:07 -08:00
committed by Tim Abbott
parent e3a4aeeffa
commit 68cfcd6446
20 changed files with 95 additions and 17 deletions

View File

@@ -279,3 +279,35 @@ exports.convert_message_topic = function (message) {
message.topic = message.subject;
}
};
exports.clean_user_content_links = function (html) {
const content = new DOMParser().parseFromString(html, "text/html").body;
for (const elt of content.getElementsByTagName("a")) {
// Ensure that all external links have target="_blank"
// rel="opener noreferrer". This ensures that external links
// never replace the Zulip webapp while also protecting
// against reverse tabnapping attacks, without relying on the
// correctness of how Zulip's markdown processor generates links.
//
// Fragment links, which we intend to only open within the
// Zulip webapp using our hashchange system, do not require
// these attributes.
let url;
try {
url = new URL(elt.getAttribute("href"), window.location.href);
} catch {
elt.removeAttribute("href");
continue;
}
// We detect URLs that are just fragments by comparing the URL
// against a new URL generated using only the hash.
if (url.hash === "" || url.href !== new URL(url.hash, window.location.href).href) {
elt.setAttribute("target", "_blank");
elt.setAttribute("rel", "noopener noreferrer");
} else {
elt.removeAttribute("target");
}
}
return content.innerHTML;
};