diff --git a/changelog.d/mrf-domain-regex-escape.security b/changelog.d/mrf-domain-regex-escape.security new file mode 100644 index 000000000..07274c37b --- /dev/null +++ b/changelog.d/mrf-domain-regex-escape.security @@ -0,0 +1 @@ +MRF domain rules now match dots literally, so a rule for `good.example` no longer also matches `goodxexample`, and wildcard rules no longer backtrack on hostile host names diff --git a/lib/pleroma/web/activity_pub/mrf.ex b/lib/pleroma/web/activity_pub/mrf.ex index 2496d1858..ddcd85624 100644 --- a/lib/pleroma/web/activity_pub/mrf.ex +++ b/lib/pleroma/web/activity_pub/mrf.ex @@ -148,14 +148,13 @@ defmodule Pleroma.Web.ActivityPub.MRF do @spec subdomains_regex([String.t()]) :: [Regex.t()] def subdomains_regex(domains) when is_list(domains) do for domain <- domains do - try do - target = String.replace(domain, "*.", "(.*\\.)*") - ~r<^#{target}$>i - rescue - e -> - Logger.error("MRF: Invalid subdomain Regex: #{domain}") - reraise e, __STACKTRACE__ - end + {prefix, rest} = + case domain do + "*." <> rest -> {"(.*\\.)?", rest} + _ -> {"", domain} + end + + ~r<^#{prefix}#{Regex.escape(rest)}$>i end end diff --git a/test/pleroma/web/activity_pub/mrf_test.exs b/test/pleroma/web/activity_pub/mrf_test.exs index d897e529d..83c0b0ae5 100644 --- a/test/pleroma/web/activity_pub/mrf_test.exs +++ b/test/pleroma/web/activity_pub/mrf_test.exs @@ -6,8 +6,6 @@ defmodule Pleroma.Web.ActivityPub.MRFTest do use ExUnit.Case use Pleroma.Tests.Helpers - import ExUnit.CaptureLog - alias Pleroma.Web.ActivityPub.MRF defp regexes_match!([], []), do: true @@ -21,14 +19,14 @@ defmodule Pleroma.Web.ActivityPub.MRFTest do test "subdomains_regex/1" do regexes = MRF.subdomains_regex(["unsafe.tld", "*.unsafe.tld"]) - assert regexes_match!(regexes, [~r/^unsafe.tld$/i, ~r/^(.*\.)*unsafe.tld$/i]) + assert regexes_match!(regexes, [~r/^unsafe\.tld$/i, ~r/^(.*\.)?unsafe\.tld$/i]) end describe "subdomain_match/2" do test "common domains" do regexes = MRF.subdomains_regex(["unsafe.tld", "unsafe2.tld"]) - assert regexes_match!(regexes, [~r/^unsafe.tld$/i, ~r/^unsafe2.tld$/i]) + assert regexes_match!(regexes, [~r/^unsafe\.tld$/i, ~r/^unsafe2\.tld$/i]) assert MRF.subdomain_match?(regexes, "unsafe.tld") assert MRF.subdomain_match?(regexes, "unsafe2.tld") @@ -39,7 +37,7 @@ defmodule Pleroma.Web.ActivityPub.MRFTest do test "wildcard domains with one subdomain" do regexes = MRF.subdomains_regex(["*.unsafe.tld"]) - assert regexes_match!(regexes, [~r/^(.*\.)*unsafe.tld$/i]) + assert regexes_match!(regexes, [~r/^(.*\.)?unsafe\.tld$/i]) assert MRF.subdomain_match?(regexes, "unsafe.tld") assert MRF.subdomain_match?(regexes, "sub.unsafe.tld") @@ -50,7 +48,7 @@ defmodule Pleroma.Web.ActivityPub.MRFTest do test "wildcard domains with two subdomains" do regexes = MRF.subdomains_regex(["*.unsafe.tld"]) - assert regexes_match!(regexes, [~r/^(.*\.)*unsafe.tld$/i]) + assert regexes_match!(regexes, [~r/^(.*\.)?unsafe\.tld$/i]) assert MRF.subdomain_match?(regexes, "unsafe.tld") assert MRF.subdomain_match?(regexes, "sub.sub.unsafe.tld") @@ -61,7 +59,7 @@ defmodule Pleroma.Web.ActivityPub.MRFTest do test "matches are case-insensitive" do regexes = MRF.subdomains_regex(["UnSafe.TLD", "UnSAFE2.Tld"]) - assert regexes_match!(regexes, [~r/^UnSafe.TLD$/i, ~r/^UnSAFE2.Tld$/i]) + assert regexes_match!(regexes, [~r/^UnSafe\.TLD$/i, ~r/^UnSAFE2\.Tld$/i]) assert MRF.subdomain_match?(regexes, "UNSAFE.TLD") assert MRF.subdomain_match?(regexes, "UNSAFE2.TLD") @@ -72,11 +70,20 @@ defmodule Pleroma.Web.ActivityPub.MRFTest do refute MRF.subdomain_match?(regexes, "example.com") end - test "logs sensible error on accidental wildcard" do - assert_raise Regex.CompileError, fn -> - assert capture_log(MRF.subdomains_regex(["*unsafe.tld"])) =~ - "MRF: Invalid subdomain Regex: *unsafe.tld" - end + test "dots only match dots" do + regexes = MRF.subdomains_regex(["unsafe.tld", "*.other.tld"]) + + refute MRF.subdomain_match?(regexes, "unsafextld") + refute MRF.subdomain_match?(regexes, "otherxtld") + refute MRF.subdomain_match?(regexes, "sub.otherxtld") + end + + test "accidental wildcards and regex syntax match nothing" do + regexes = MRF.subdomains_regex(["*unsafe.tld", "bad(.tld"]) + + refute MRF.subdomain_match?(regexes, "unsafe.tld") + refute MRF.subdomain_match?(regexes, "sub.unsafe.tld") + refute MRF.subdomain_match?(regexes, "bad.tld") end end