From bdc8112e4031a17c39a570bc8fc4bb6b8c35f9aa Mon Sep 17 00:00:00 2001 From: href 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])