From 4a61e36def4ae7349dc7b39f2a9848505389cf6a Mon Sep 17 00:00:00 2001
From: Anders Kaseorg <anders@zulip.com>
Date: Tue, 16 Aug 2022 21:37:53 -0700
Subject: [PATCH] CVE-2022-36048: Rewrite only specific local links to
 relative.

Due to mismatches between the URL parsers in Python and browsers, it
was possible to hoodwink rewrite_local_links_to_relative into
generating links that browsers would interpret as absolute.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
---
 zerver/lib/markdown/__init__.py | 21 ++++++++-------------
 zerver/tests/test_markdown.py   | 28 ++++++++++++++++++++++++++--
 2 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/zerver/lib/markdown/__init__.py b/zerver/lib/markdown/__init__.py
index e2ebc2ff12..126787268b 100644
--- a/zerver/lib/markdown/__init__.py
+++ b/zerver/lib/markdown/__init__.py
@@ -292,10 +292,7 @@ def rewrite_local_links_to_relative(db_data: Optional[DbData], link: str) -> str
 
     if db_data:
         realm_uri_prefix = db_data.realm_uri + "/"
-        if (
-            link.startswith(realm_uri_prefix)
-            and urllib.parse.urljoin(realm_uri_prefix, link[len(realm_uri_prefix) :]) == link
-        ):
+        if link.startswith((realm_uri_prefix + "#", realm_uri_prefix + "user_uploads/")):
             return link[len(realm_uri_prefix) :]
 
     return link
@@ -1221,9 +1218,6 @@ class InlineInterestingLinkProcessor(markdown.treeprocessors.Treeprocessor):
             if uncle_link not in parent_links:
                 return insertion_index
 
-    def is_absolute_url(self, url: str) -> bool:
-        return bool(urllib.parse.urlparse(url).netloc)
-
     def run(self, root: Element) -> None:
         # Get all URLs from the blob
         found_urls = walk_tree_with_family(root, self.get_url_data)
@@ -1274,12 +1268,6 @@ class InlineInterestingLinkProcessor(markdown.treeprocessors.Treeprocessor):
             else:
                 continue
 
-            if not self.is_absolute_url(url):
-                if self.is_image(url):
-                    self.handle_image_inlining(root, found_url)
-                # We don't have a strong use case for doing URL preview for relative links.
-                continue
-
             dropbox_image = self.dropbox_image(url)
             if dropbox_image is not None:
                 class_attr = "message_inline_ref"
@@ -1308,6 +1296,13 @@ class InlineInterestingLinkProcessor(markdown.treeprocessors.Treeprocessor):
                 self.handle_image_inlining(root, found_url)
                 continue
 
+            netloc = urlsplit(url).netloc
+            if netloc == "" or (
+                self.md.zulip_realm is not None and netloc == self.md.zulip_realm.host
+            ):
+                # We don't have a strong use case for doing URL preview for relative links.
+                continue
+
             if get_tweet_id(url) is not None:
                 if rendered_tweet_count >= self.TWITTER_MAX_TO_PREVIEW:
                     # Only render at most one tweet per message
diff --git a/zerver/tests/test_markdown.py b/zerver/tests/test_markdown.py
index c378e25356..7407568f59 100644
--- a/zerver/tests/test_markdown.py
+++ b/zerver/tests/test_markdown.py
@@ -2760,13 +2760,25 @@ class MarkdownTest(ZulipTestCase):
         realm = get_realm("zulip")
         sender_user_profile = self.example_user("othello")
         message = Message(sender=sender_user_profile, sending_client=get_client("test"))
-        msg = "http://zulip.testserver/#narrow/stream/999-hello"
 
+        msg = "http://zulip.testserver/#narrow/stream/999-hello"
         self.assertEqual(
             markdown_convert(msg, message_realm=realm, message=message).rendered_content,
             '<p><a href="#narrow/stream/999-hello">http://zulip.testserver/#narrow/stream/999-hello</a></p>',
         )
 
+        msg = f"http://zulip.testserver/user_uploads/{realm.id}/ff/file.txt"
+        self.assertEqual(
+            markdown_convert(msg, message_realm=realm, message=message).rendered_content,
+            f'<p><a href="user_uploads/{realm.id}/ff/file.txt">http://zulip.testserver/user_uploads/{realm.id}/ff/file.txt</a></p>',
+        )
+
+        msg = "http://zulip.testserver/not:relative"
+        self.assertEqual(
+            markdown_convert(msg, message_realm=realm, message=message).rendered_content,
+            '<p><a href="http://zulip.testserver/not:relative">http://zulip.testserver/not:relative</a></p>',
+        )
+
     def test_relative_link_streams_page(self) -> None:
         realm = get_realm("zulip")
         sender_user_profile = self.example_user("othello")
@@ -2782,13 +2794,25 @@ class MarkdownTest(ZulipTestCase):
         realm = get_realm("zulip")
         sender_user_profile = self.example_user("othello")
         message = Message(sender=sender_user_profile, sending_client=get_client("test"))
-        msg = "[hello](http://zulip.testserver/#narrow/stream/999-hello)"
 
+        msg = "[hello](http://zulip.testserver/#narrow/stream/999-hello)"
         self.assertEqual(
             markdown_convert(msg, message_realm=realm, message=message).rendered_content,
             '<p><a href="#narrow/stream/999-hello">hello</a></p>',
         )
 
+        msg = f"[hello](http://zulip.testserver/user_uploads/{realm.id}/ff/file.txt)"
+        self.assertEqual(
+            markdown_convert(msg, message_realm=realm, message=message).rendered_content,
+            f'<p><a href="user_uploads/{realm.id}/ff/file.txt">hello</a></p>',
+        )
+
+        msg = "[hello](http://zulip.testserver/not:relative)"
+        self.assertEqual(
+            markdown_convert(msg, message_realm=realm, message=message).rendered_content,
+            '<p><a href="http://zulip.testserver/not:relative">hello</a></p>',
+        )
+
     def test_html_entity_conversion(self) -> None:
         msg = """\
             Test raw: Hello, &copy;