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>
This commit is contained in:
Anders Kaseorg 2022-08-16 21:37:53 -07:00
parent 4ac4573c52
commit 4a61e36def
2 changed files with 34 additions and 15 deletions

View File

@ -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

View File

@ -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;