From bdc8112e4031a17c39a570bc8fc4bb6b8c35f9aa Mon Sep 17 00:00:00 2001
From: href <href@random.sh>
Date: Fri, 7 Dec 2018 21:44:04 +0100
Subject: [PATCH] Media proxy: fix url encoding

---
 lib/pleroma/web/media_proxy/controller.ex  |  7 +++-
 lib/pleroma/web/media_proxy/media_proxy.ex |  9 +++++-
 test/media_proxy_test.exs                  | 37 ++++++++++++++++++++++
 3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/lib/pleroma/web/media_proxy/controller.ex b/lib/pleroma/web/media_proxy/controller.ex
index d0b92d0c1..f496fc936 100644
--- a/lib/pleroma/web/media_proxy/controller.ex
+++ b/lib/pleroma/web/media_proxy/controller.ex
@@ -24,7 +24,12 @@ def remote(conn, params = %{"sig" => sig64, "url" => url64}) do
   end
 
   def filename_matches(has_filename, path, url) do
-    filename = MediaProxy.filename(url)
+    filename =
+      url
+      |> MediaProxy.filename()
+      |> URI.decode()
+
+    path = URI.decode(path)
 
     cond do
       has_filename && filename && Path.basename(path) != filename -> {:wrong_filename, filename}
diff --git a/lib/pleroma/web/media_proxy/media_proxy.ex b/lib/pleroma/web/media_proxy/media_proxy.ex
index 28aacb0b1..902ab1b77 100644
--- a/lib/pleroma/web/media_proxy/media_proxy.ex
+++ b/lib/pleroma/web/media_proxy/media_proxy.ex
@@ -14,7 +14,14 @@ def url(url) do
       url
     else
       secret = Application.get_env(:pleroma, Pleroma.Web.Endpoint)[:secret_key_base]
-      base64 = Base.url_encode64(url, @base64_opts)
+
+      # The URL is url-decoded and encoded again to ensure it is correctly encoded and not twice.
+      base64 =
+        url
+        |> URI.decode()
+        |> URI.encode()
+        |> Base.url_encode64(@base64_opts)
+
       sig = :crypto.hmac(:sha, secret, base64)
       sig64 = sig |> Base.url_encode64(@base64_opts)
 
diff --git a/test/media_proxy_test.exs b/test/media_proxy_test.exs
index d71f9f13a..cb455ca79 100644
--- a/test/media_proxy_test.exs
+++ b/test/media_proxy_test.exs
@@ -1,6 +1,7 @@
 defmodule Pleroma.MediaProxyTest do
   use ExUnit.Case
   import Pleroma.Web.MediaProxy
+  alias Pleroma.Web.MediaProxy.MediaProxyController
 
   describe "when enabled" do
     setup do
@@ -65,6 +66,14 @@ test "encodes and decodes URL and ignores query params for the path" do
       assert decode_result(encoded) == url
     end
 
+    test "ensures urls are url-encoded" do
+      assert decode_result(url("https://pleroma.social/Hello world.jpg")) ==
+               "https://pleroma.social/Hello%20world.jpg"
+
+      assert decode_result(url("https://pleroma.social/Hello%20world.jpg")) ==
+               "https://pleroma.social/Hello%20world.jpg"
+    end
+
     test "validates signature" do
       secret_key_base = Pleroma.Config.get([Pleroma.Web.Endpoint, :secret_key_base])
 
@@ -83,6 +92,34 @@ test "validates signature" do
       assert decode_url(sig, base64) == {:error, :invalid_signature}
     end
 
+    test "filename_matches matches url encoded paths" do
+      assert MediaProxyController.filename_matches(
+               true,
+               "/Hello%20world.jpg",
+               "http://pleroma.social/Hello world.jpg"
+             ) == :ok
+
+      assert MediaProxyController.filename_matches(
+               true,
+               "/Hello%20world.jpg",
+               "http://pleroma.social/Hello%20world.jpg"
+             ) == :ok
+    end
+
+    test "filename_matches matches non-url encoded paths" do
+      assert MediaProxyController.filename_matches(
+               true,
+               "/Hello world.jpg",
+               "http://pleroma.social/Hello%20world.jpg"
+             ) == :ok
+
+      assert MediaProxyController.filename_matches(
+               true,
+               "/Hello world.jpg",
+               "http://pleroma.social/Hello world.jpg"
+             ) == :ok
+    end
+
     test "uses the configured base_url" do
       base_url = Pleroma.Config.get([:media_proxy, :base_url])