From 52f74bbd9b2fafd4e11dd09d0714d59fda244dd3 Mon Sep 17 00:00:00 2001
From: Alex Vandiver <alexmv@zulip.com>
Date: Wed, 20 Oct 2021 17:48:28 -0700
Subject: [PATCH] markdown: Run URL preview links through camo.

Not proxying these requests through camo is a security concern.
Furthermore, on the desktop client, any embed image which is hosted on
a server with an expired or otherwise invalid certificate will trigger
a blocking modal window with no clear source and a confusing error
message; see zulip/zulip-desktop#1119.

Rewrite all `message_embed_image` URLs through camo, if it is enabled.
---
 zerver/lib/markdown/__init__.py |  1 +
 zerver/tests/test_link_embed.py | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/zerver/lib/markdown/__init__.py b/zerver/lib/markdown/__init__.py
index 1d6fa00a86..675470764a 100644
--- a/zerver/lib/markdown/__init__.py
+++ b/zerver/lib/markdown/__init__.py
@@ -712,6 +712,7 @@ class InlineInterestingLinkProcessor(markdown.treeprocessors.Treeprocessor):
         container = SubElement(root, "div")
         container.set("class", "message_embed")
 
+        img_link = get_camo_url(img_link)
         img = SubElement(container, "a")
         img.set("style", "background-image: url(" + img_link + ")")
         img.set("href", link)
diff --git a/zerver/tests/test_link_embed.py b/zerver/tests/test_link_embed.py
index ae1473c0c6..dc437c8aa3 100644
--- a/zerver/tests/test_link_embed.py
+++ b/zerver/tests/test_link_embed.py
@@ -522,6 +522,7 @@ class PreviewTestCase(ZulipTestCase):
         msg = self._send_message_with_test_org_url(sender=self.example_user("prospero"))
         self.assertIn(embedded_link, msg.rendered_content)
 
+    @override_settings(CAMO_URI="")
     def test_inline_url_embed_preview(self) -> None:
         with_preview = '<p><a href="http://test.org/">http://test.org/</a></p>\n<div class="message_embed"><a class="message_embed_image" href="http://test.org/" style="background-image: url(http://ia.media-imdb.com/images/rock.jpg)"></a><div class="data-container"><div class="message_embed_title"><a href="http://test.org/" title="The Rock">The Rock</a></div><div class="message_embed_description">Description text</div></div></div>'
         without_preview = '<p><a href="http://test.org/">http://test.org/</a></p>'
@@ -537,6 +538,17 @@ class PreviewTestCase(ZulipTestCase):
         )
         self.assertEqual(msg.rendered_content, without_preview)
 
+    def test_inline_url_embed_preview_with_camo(self) -> None:
+        camo_url = get_camo_url("http://ia.media-imdb.com/images/rock.jpg")
+        with_preview = (
+            '<p><a href="http://test.org/">http://test.org/</a></p>\n<div class="message_embed"><a class="message_embed_image" href="http://test.org/" style="background-image: url('
+            + camo_url
+            + ')"></a><div class="data-container"><div class="message_embed_title"><a href="http://test.org/" title="The Rock">The Rock</a></div><div class="message_embed_description">Description text</div></div></div>'
+        )
+        msg = self._send_message_with_test_org_url(sender=self.example_user("hamlet"))
+        self.assertEqual(msg.rendered_content, with_preview)
+
+    @override_settings(CAMO_URI="")
     @override_settings(INLINE_URL_EMBED_PREVIEW=True)
     def test_inline_relative_url_embed_preview(self) -> None:
         # Relative URLs should not be sent for URL preview.
@@ -548,6 +560,7 @@ class PreviewTestCase(ZulipTestCase):
             )
             patched.assert_not_called()
 
+    @override_settings(CAMO_URI="")
     def test_inline_url_embed_preview_with_relative_image_url(self) -> None:
         with_preview_relative = '<p><a href="http://test.org/">http://test.org/</a></p>\n<div class="message_embed"><a class="message_embed_image" href="http://test.org/" style="background-image: url(http://test.org/images/rock.jpg)"></a><div class="data-container"><div class="message_embed_title"><a href="http://test.org/" title="The Rock">The Rock</a></div><div class="message_embed_description">Description text</div></div></div>'
         # Try case where the Open Graph image is a relative URL.
@@ -708,6 +721,7 @@ class PreviewTestCase(ZulipTestCase):
         )
 
     @responses.activate
+    @override_settings(CAMO_URI="")
     @override_settings(INLINE_URL_EMBED_PREVIEW=True)
     def test_link_preview_no_content_type_header(self) -> None:
         user = self.example_user("hamlet")