Skip to content

Commit e1b664a

Browse files
progvalprogval
committed
Prefer to transform mxc:// URIs to URLs to the HS rather than the sender's server
Otherwise it is too easy to crash M51, eg. by sending a mxc:// URI with a misbehaved server (eg. invalid TLS cert)
1 parent 1b61870 commit e1b664a

File tree

5 files changed

+59
-19
lines changed

5 files changed

+59
-19
lines changed

‎lib/format/common.ex

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,12 @@ defmodule M51.Format do
5858
iex> M51.Format.matrix2irc(~s(foo <font data-mx-color="FF0000">bar</font> baz))
5959
"foo \x04FF0000,FFFFFFbar\x0399,99 baz"
6060
"""
61-
def matrix2irc(html) do
61+
def matrix2irc(html, homeserver \\ nil) do
6262
tree = :mochiweb_html.parse("<html>" <> html <> "</html>")
63-
String.trim(M51.Format.Matrix2Irc.transform(tree, %M51.Format.Matrix2Irc.State{}))
63+
64+
String.trim(
65+
M51.Format.Matrix2Irc.transform(tree, %M51.Format.Matrix2Irc.State{homeserver: homeserver})
66+
)
6467
end
6568

6669
@doc ~S"""

‎lib/format/matrix2irc.ex

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
###
1616

1717
defmodule M51.Format.Matrix2Irc.State do
18-
defstruct preserve_whitespace: false,
18+
defstruct homeserver: nil,
19+
preserve_whitespace: false,
1920
color: {nil, nil}
2021
end
2122

@@ -80,9 +81,9 @@ defmodule M51.Format.Matrix2Irc do
8081
{nil, nil, nil} -> transform_children(children, state)
8182
{nil, nil, title} -> title
8283
{nil, alt, _} -> alt
83-
{link, nil, nil} -> format_url(link)
84-
{link, nil, title} -> "#{title} <#{format_url(link)}>"
85-
{link, alt, _} -> "#{alt} <#{format_url(link)}>"
84+
{link, nil, nil} -> format_url(link, state.homeserver)
85+
{link, nil, title} -> "#{title} <#{format_url(link, state.homeserver)}>"
86+
{link, alt, _} -> "#{alt} <#{format_url(link, state.homeserver)}>"
8687
end
8788
end
8889

@@ -175,10 +176,14 @@ defmodule M51.Format.Matrix2Irc do
175176
end
176177

177178
@doc "Transforms a mxc:// \"URL\" into an actually usable URL."
178-
def format_url(url, filename \\ nil) do
179+
def format_url(url, homeserver \\ nil, filename \\ nil) do
179180
case URI.parse(url) do
180181
%{scheme: "mxc", host: host, path: path} ->
181-
base_url = M51.MatrixClient.Client.get_base_url(host, M51.Config.httpoison())
182+
# prefer the homeserver when available, it is more reliable than arbitrary
183+
# hosts chosen by message senders
184+
homeserver = homeserver || host
185+
186+
base_url = M51.MatrixClient.Client.get_base_url(homeserver, M51.Config.httpoison())
182187

183188
case filename do
184189
nil ->

‎lib/matrix_client/client.ex

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,19 @@ defmodule M51.MatrixClient.Client do
481481
end
482482
end
483483

484+
def hostname(pid) do
485+
case GenServer.call(pid, {:dump_state}) do
486+
%M51.MatrixClient.Client{
487+
state: :connected,
488+
hostname: hostname
489+
} ->
490+
hostname
491+
492+
%M51.MatrixClient.Client{state: :initial_state} ->
493+
nil
494+
end
495+
end
496+
484497
def register(pid, local_name, hostname, password) do
485498
GenServer.call(pid, {:register, local_name, hostname, password}, @timeout)
486499
end

‎lib/matrix_client/poller.ex

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -451,8 +451,11 @@ defmodule M51.MatrixClient.Poller do
451451
state = M51.IrcConn.Supervisor.matrix_state(sup_pid)
452452
channel = M51.MatrixClient.State.room_irc_channel(state, room_id)
453453
member = M51.MatrixClient.State.room_member(state, room_id, sender)
454+
client = M51.IrcConn.Supervisor.matrix_client(sup_pid)
454455
send = make_send_function(sup_pid, event, write)
455456

457+
homeserver = M51.MatrixClient.Client.hostname(client)
458+
456459
tags = %{"account" => sender}
457460

458461
# TODO: dedup this with m.reaction handler
@@ -498,7 +501,7 @@ defmodule M51.MatrixClient.Poller do
498501
body
499502
end
500503

501-
{"PRIVMSG", M51.Format.matrix2irc(formatted_body) || body}
504+
{"PRIVMSG", M51.Format.matrix2irc(formatted_body, homeserver) || body}
502505

503506
%{"msgtype" => "m.text", "body" => body} when is_binary(body) ->
504507
body =
@@ -522,7 +525,8 @@ defmodule M51.MatrixClient.Poller do
522525
"formatted_body" => formatted_body,
523526
"body" => body
524527
} ->
525-
{"PRIVMSG", "\x01ACTION " <> (M51.Format.matrix2irc(formatted_body) || body) <> "\x01"}
528+
{"PRIVMSG",
529+
"\x01ACTION " <> (M51.Format.matrix2irc(formatted_body, homeserver) || body) <> "\x01"}
526530

527531
%{"msgtype" => "m.emote", "body" => body} when is_binary(body) ->
528532
# TODO: ditto
@@ -534,7 +538,7 @@ defmodule M51.MatrixClient.Poller do
534538
"formatted_body" => formatted_body,
535539
"body" => body
536540
} ->
537-
{"NOTICE", M51.Format.matrix2irc(formatted_body) || body}
541+
{"NOTICE", M51.Format.matrix2irc(formatted_body, homeserver) || body}
538542

539543
%{"msgtype" => "m.notice", "body" => body} when is_binary(body) ->
540544
# TODO: ditto
@@ -543,38 +547,39 @@ defmodule M51.MatrixClient.Poller do
543547
%{"msgtype" => "m.image", "body" => body, "url" => url, "filename" => filename}
544548
when is_binary(body) and is_binary(url) and is_binary(filename) ->
545549
if M51.Format.Matrix2Irc.useless_img_alt?(body) or body == filename do
546-
{"PRIVMSG", M51.Format.Matrix2Irc.format_url(url, filename)}
550+
{"PRIVMSG", M51.Format.Matrix2Irc.format_url(url, homeserver, filename)}
547551
else
548-
{"PRIVMSG", body <> " " <> M51.Format.Matrix2Irc.format_url(url, filename)}
552+
{"PRIVMSG",
553+
body <> " " <> M51.Format.Matrix2Irc.format_url(url, homeserver, filename)}
549554
end
550555

551556
%{"msgtype" => "m.image", "body" => body, "url" => url}
552557
when is_binary(body) and is_binary(url) ->
553558
if M51.Format.Matrix2Irc.useless_img_alt?(body) do
554-
{"PRIVMSG", M51.Format.Matrix2Irc.format_url(url)}
559+
{"PRIVMSG", M51.Format.Matrix2Irc.format_url(url, homeserver)}
555560
else
556-
{"PRIVMSG", body <> " " <> M51.Format.Matrix2Irc.format_url(url)}
561+
{"PRIVMSG", body <> " " <> M51.Format.Matrix2Irc.format_url(url, homeserver)}
557562
end
558563

559564
%{"msgtype" => "m.file", "body" => body, "url" => url, "filename" => filename}
560565
when is_binary(body) and is_binary(url) and is_binary(filename) ->
561-
{"PRIVMSG", body <> " " <> M51.Format.Matrix2Irc.format_url(url, filename)}
566+
{"PRIVMSG", body <> " " <> M51.Format.Matrix2Irc.format_url(url, homeserver, filename)}
562567

563568
%{"msgtype" => "m.file", "body" => body, "url" => url}
564569
when is_binary(body) and is_binary(url) ->
565-
{"PRIVMSG", body <> " " <> M51.Format.Matrix2Irc.format_url(url)}
570+
{"PRIVMSG", body <> " " <> M51.Format.Matrix2Irc.format_url(url, homeserver)}
566571

567572
%{"msgtype" => "m.audio", "body" => body, "url" => url}
568573
when is_binary(body) and is_binary(url) ->
569-
{"PRIVMSG", body <> " " <> M51.Format.Matrix2Irc.format_url(url)}
574+
{"PRIVMSG", body <> " " <> M51.Format.Matrix2Irc.format_url(url, homeserver)}
570575

571576
%{"msgtype" => "m.location", "body" => body, "geo_uri" => geo_uri}
572577
when is_binary(body) and is_binary(geo_uri) ->
573578
{"PRIVMSG", body <> " (" <> geo_uri <> ")"}
574579

575580
%{"msgtype" => "m.video", "body" => body, "url" => url}
576581
when is_binary(body) and is_binary(url) ->
577-
{"PRIVMSG", body <> " " <> M51.Format.Matrix2Irc.format_url(url)}
582+
{"PRIVMSG", body <> " " <> M51.Format.Matrix2Irc.format_url(url, homeserver)}
578583

579584
%{"body" => body} when is_binary(body) ->
580585
# fallback

‎test/format/common_test.exs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,14 @@ defmodule M51.FormatTest do
122122
body: ~s({"m.homeserver": {"base_url": "https://api.example.org"}})
123123
}
124124
end)
125+
|> expect(:get!, 1, fn url ->
126+
assert url == "https://homeserver.org/.well-known/matrix/client"
127+
128+
%HTTPoison.Response{
129+
status_code: 200,
130+
body: ~s({"m.homeserver": {"base_url": "https://api.homeserver.org"}})
131+
}
132+
end)
125133

126134
assert M51.Format.matrix2irc(~s(<a href="https://example.org">foo</a>)) ==
127135
"foo <https://example.org>"
@@ -145,6 +153,12 @@ defmodule M51.FormatTest do
145153
) ==
146154
"an image <https://api.example.org/_matrix/media/r0/download/example.org/foo>"
147155

156+
assert M51.Format.matrix2irc(
157+
~s(<img src="mxc://example.org/foo" alt="an image" title="blah"/>),
158+
"homeserver.org"
159+
) ==
160+
"an image <https://api.homeserver.org/_matrix/media/r0/download/example.org/foo>"
161+
148162
assert M51.Format.matrix2irc(~s(<img" />)) ==
149163
""
150164

0 commit comments

Comments
 (0)