From b1be9415effadf81e557eddee3f60bdf0fa359af Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sun, 2 Sep 2018 00:14:25 +0000 Subject: [PATCH 1/6] Revert "Merge branch 'revert-a26d5e6b' into 'develop'" This reverts commit d31bbb1cfe04ca6073a322bcf77239e7d4b79839, reversing changes made to 340ab3cb9068d444b77213e07beb8c2c3ca128b9. --- lib/pleroma/formatter.ex | 6 +++- lib/pleroma/web/common_api/common_api.ex | 9 ++++- lib/pleroma/web/common_api/utils.ex | 34 ++++++++++++++++--- .../web/twitter_api/twitter_api_controller.ex | 2 +- mix.exs | 1 + mix.lock | 1 + test/web/common_api/common_api_test.exs | 32 +++++++++++++++++ 7 files changed, 78 insertions(+), 7 deletions(-) diff --git a/lib/pleroma/formatter.ex b/lib/pleroma/formatter.ex index d5565a2ca..c0a176184 100644 --- a/lib/pleroma/formatter.ex +++ b/lib/pleroma/formatter.ex @@ -192,7 +192,11 @@ def get_custom_emoji() do ] # TODO: make it use something other than @link_regex - def html_escape(text) do + def html_escape(text, "text/html") do + HtmlSanitizeEx.basic_html(text) + end + + def html_escape(text, "text/plain") do Regex.split(@link_regex, text, include_captures: true) |> Enum.map_every(2, fn chunk -> {:safe, part} = Phoenix.HTML.html_escape(chunk) diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 125c57d05..2ab50c968 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -85,7 +85,14 @@ def post(user, %{"status" => status} = data) do {to, cc} <- to_for_user_and_mentions(user, mentions, inReplyTo, visibility), tags <- Formatter.parse_tags(status, data), content_html <- - make_content_html(status, mentions, attachments, tags, data["no_attachment_links"]), + make_content_html( + status, + mentions, + attachments, + tags, + data["content_type"] || "text/plain", + data["no_attachment_links"] + ), context <- make_context(inReplyTo), cw <- data["spoiler_text"], object <- diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex index 358ca22ac..667027c02 100644 --- a/lib/pleroma/web/common_api/utils.ex +++ b/lib/pleroma/web/common_api/utils.ex @@ -63,9 +63,16 @@ def to_for_user_and_mentions(_user, mentions, inReplyTo, "direct") do end end - def make_content_html(status, mentions, attachments, tags, no_attachment_links \\ false) do + def make_content_html( + status, + mentions, + attachments, + tags, + content_type, + no_attachment_links \\ false + ) do status - |> format_input(mentions, tags) + |> format_input(mentions, tags, content_type) |> maybe_add_attachments(attachments, no_attachment_links) end @@ -92,9 +99,9 @@ def add_attachments(text, attachments) do Enum.join([text | attachment_text], "
") end - def format_input(text, mentions, tags) do + def format_input(text, mentions, tags, "text/plain") do text - |> Formatter.html_escape() + |> Formatter.html_escape("text/plain") |> String.replace(~r/\r?\n/, "
") |> (&{[], &1}).() |> Formatter.add_links() @@ -103,6 +110,25 @@ def format_input(text, mentions, tags) do |> Formatter.finalize() end + def format_input(text, mentions, tags, "text/html") do + text + |> Formatter.html_escape("text/html") + |> String.replace(~r/\r?\n/, "
") + |> (&{[], &1}).() + |> Formatter.add_user_links(mentions) + |> Formatter.finalize() + end + + def format_input(text, mentions, tags, "text/markdown") do + text + |> Earmark.as_html!() + |> Formatter.html_escape("text/html") + |> String.replace(~r/\r?\n/, "") + |> (&{[], &1}).() + |> Formatter.add_user_links(mentions) + |> Formatter.finalize() + end + def add_tag_links(text, tags) do tags = tags diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index cd2bb5b57..c6637e38d 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -423,7 +423,7 @@ def update_profile(%{assigns: %{user: user}} = conn, params) do {String.trim(name, ":"), url} end) - bio_html = CommonUtils.format_input(bio, mentions, tags) + bio_html = CommonUtils.format_input(bio, mentions, tags, "text/plain") Map.put(params, "bio", bio_html |> Formatter.emojify(emoji)) else params diff --git a/mix.exs b/mix.exs index 24c7108a0..885df0094 100644 --- a/mix.exs +++ b/mix.exs @@ -48,6 +48,7 @@ defp deps do {:mogrify, "~> 0.6.1"}, {:ex_aws, "~> 2.0"}, {:ex_aws_s3, "~> 2.0"}, + {:earmark, "~> 1.2"}, {:ex_machina, "~> 2.2", only: :test}, {:credo, "~> 0.9.3", only: [:dev, :test]}, {:mock, "~> 0.3.1", only: :test}, diff --git a/mix.lock b/mix.lock index 1da8e7b0c..105eaf665 100644 --- a/mix.lock +++ b/mix.lock @@ -11,6 +11,7 @@ "crypt": {:git, "https://github.com/msantos/crypt", "1f2b58927ab57e72910191a7ebaeff984382a1d3", [ref: "1f2b58927ab57e72910191a7ebaeff984382a1d3"]}, "db_connection": {:hex, :db_connection, "1.1.3", "89b30ca1ef0a3b469b1c779579590688561d586694a3ce8792985d4d7e575a61", [:mix], [{:connection, "~> 1.0.2", [hex: :connection, repo: "hexpm", optional: false]}, {:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, repo: "hexpm", optional: true]}], "hexpm"}, "decimal": {:hex, :decimal, "1.5.0", "b0433a36d0e2430e3d50291b1c65f53c37d56f83665b43d79963684865beab68", [:mix], [], "hexpm"}, + "earmark": {:hex, :earmark, "1.2.6", "b6da42b3831458d3ecc57314dff3051b080b9b2be88c2e5aa41cd642a5b044ed", [:mix], [], "hexpm"}, "ecto": {:hex, :ecto, "2.2.10", "e7366dc82f48f8dd78fcbf3ab50985ceeb11cb3dc93435147c6e13f2cda0992e", [:mix], [{:db_connection, "~> 1.1", [hex: :db_connection, repo: "hexpm", optional: true]}, {:decimal, "~> 1.2", [hex: :decimal, repo: "hexpm", optional: false]}, {:mariaex, "~> 0.8.0", [hex: :mariaex, repo: "hexpm", optional: true]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: true]}, {:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: false]}, {:postgrex, "~> 0.13.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, repo: "hexpm", optional: true]}], "hexpm"}, "eternal": {:hex, :eternal, "1.2.0", "e2a6b6ce3b8c248f7dc31451aefca57e3bdf0e48d73ae5043229380a67614c41", [:mix], [], "hexpm"}, "ex_aws": {:hex, :ex_aws, "2.1.0", "b92651527d6c09c479f9013caa9c7331f19cba38a650590d82ebf2c6c16a1d8a", [:mix], [{:configparser_ex, "~> 2.0", [hex: :configparser_ex, repo: "hexpm", optional: true]}, {:hackney, "1.6.3 or 1.6.5 or 1.7.1 or 1.8.6 or ~> 1.9", [hex: :hackney, repo: "hexpm", optional: true]}, {:jsx, "~> 2.8", [hex: :jsx, repo: "hexpm", optional: true]}, {:poison, ">= 1.2.0", [hex: :poison, repo: "hexpm", optional: true]}, {:sweet_xml, "~> 0.6", [hex: :sweet_xml, repo: "hexpm", optional: true]}, {:xml_builder, "~> 0.1.0", [hex: :xml_builder, repo: "hexpm", optional: true]}], "hexpm"}, diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index 2a2c40833..cd5aca961 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -21,4 +21,36 @@ test "it adds emoji when updating profiles" do assert karjalanpiirakka["name"] == ":karjalanpiirakka:" end + + describe "posting" do + test "it filters out obviously bad tags when accepting a post as HTML" do + user = insert(:user) + + post = "

2hu

" + + {:ok, activity} = + CommonAPI.post(user, %{ + "status" => post, + "content_type" => "text/html" + }) + + content = activity.data["object"]["content"] + assert content == "

2hu

alert('xss')" + end + + test "it filters out obviously bad tags when accepting a post as Markdown" do + user = insert(:user) + + post = "

2hu

" + + {:ok, activity} = + CommonAPI.post(user, %{ + "status" => post, + "content_type" => "text/markdown" + }) + + content = activity.data["object"]["content"] + assert content == "

2hu

alert('xss')" + end + end end From 16307da3115a840163be149c3847fc600b260bc6 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sun, 9 Sep 2018 12:12:31 +0000 Subject: [PATCH 2/6] twitterapi: frontend config: add formattingOptionsEnabled --- config/config.exs | 1 + lib/pleroma/web/twitter_api/controllers/util_controller.ex | 1 + 2 files changed, 2 insertions(+) diff --git a/config/config.exs b/config/config.exs index 2290119f7..c3094eb2b 100644 --- a/config/config.exs +++ b/config/config.exs @@ -98,6 +98,7 @@ redirect_root_login: "/main/friends", show_instance_panel: true, scope_options_enabled: false, + formatting_options_enabled: false, collapse_message_with_subject: false config :pleroma, :activitypub, diff --git a/lib/pleroma/web/twitter_api/controllers/util_controller.ex b/lib/pleroma/web/twitter_api/controllers/util_controller.ex index 886b70f5f..4aaf28869 100644 --- a/lib/pleroma/web/twitter_api/controllers/util_controller.ex +++ b/lib/pleroma/web/twitter_api/controllers/util_controller.ex @@ -176,6 +176,7 @@ def config(conn, _params) do chatDisabled: !Keyword.get(@instance_chat, :enabled), showInstanceSpecificPanel: Keyword.get(@instance_fe, :show_instance_panel), scopeOptionsEnabled: Keyword.get(@instance_fe, :scope_options_enabled), + formattingOptionsEnabled: Keyword.get(@instance_fe, :formatting_options_enabled), collapseMessageWithSubject: Keyword.get(@instance_fe, :collapse_message_with_subject) } From 52b05137c5800186fffee83950c83194a3468057 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sun, 9 Sep 2018 23:40:24 +0000 Subject: [PATCH 3/6] formatter: use Pleroma.HTML module instead of HtmlSanitizeEx directly --- lib/pleroma/formatter.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/formatter.ex b/lib/pleroma/formatter.ex index c0a176184..5b63fb795 100644 --- a/lib/pleroma/formatter.ex +++ b/lib/pleroma/formatter.ex @@ -193,7 +193,7 @@ def get_custom_emoji() do # TODO: make it use something other than @link_regex def html_escape(text, "text/html") do - HtmlSanitizeEx.basic_html(text) + HTML.filter_tags(text) end def html_escape(text, "text/plain") do From 285ac80c36cbd943b16eb5e1ee4447376f8f555f Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Fri, 5 Oct 2018 21:02:17 +0000 Subject: [PATCH 4/6] config: allow for accepted post formats to be configured --- config/config.exs | 7 ++++++- lib/pleroma/web/common_api/common_api.ex | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/config/config.exs b/config/config.exs index c3094eb2b..608c035b0 100644 --- a/config/config.exs +++ b/config/config.exs @@ -74,7 +74,12 @@ rewrite_policy: Pleroma.Web.ActivityPub.MRF.NoOpPolicy, public: true, quarantined_instances: [], - managed_config: true + managed_config: true, + allowed_post_formats: [ + "text/plain", + "text/html", + "text/markdown" + ] config :pleroma, :markup, # XXX - unfortunately, inline images must be enabled by default right now, because diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 2ab50c968..d4a973e36 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -73,6 +73,11 @@ def get_visibility(%{"in_reply_to_status_id" => status_id}) when not is_nil(stat def get_visibility(_), do: "public" @instance Application.get_env(:pleroma, :instance) + @allowed_post_formats Keyword.get(@instance, :allowed_post_formats) + + defp get_content_type(content_type) when content_type in @allowed_post_formats, do: content_type + defp get_content_type(_), do: "text/plain" + @limit Keyword.get(@instance, :limit) def post(user, %{"status" => status} = data) do visibility = get_visibility(data) @@ -90,7 +95,7 @@ def post(user, %{"status" => status} = data) do mentions, attachments, tags, - data["content_type"] || "text/plain", + get_content_type(data["content_type"]), data["no_attachment_links"] ), context <- make_context(inReplyTo), From bd76d9cee6f166c20af9194d4d83f5276041ef75 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Fri, 5 Oct 2018 21:05:37 +0000 Subject: [PATCH 5/6] nodeinfo: add accepted post formats to metadata --- lib/pleroma/web/nodeinfo/nodeinfo_controller.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/nodeinfo/nodeinfo_controller.ex b/lib/pleroma/web/nodeinfo/nodeinfo_controller.ex index 860468506..a14000c61 100644 --- a/lib/pleroma/web/nodeinfo/nodeinfo_controller.ex +++ b/lib/pleroma/web/nodeinfo/nodeinfo_controller.ex @@ -92,7 +92,8 @@ def nodeinfo(conn, %{"version" => "2.0"}) do mrf_policies: mrf_policies, mrf_simple: mrf_simple, quarantined_instances: quarantined - } + }, + postFormats: Keyword.get(instance, :allowed_post_formats) } } From 497814cbbb4baea91b2882fbddd5cd8d5ad6e170 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Fri, 5 Oct 2018 21:11:22 +0000 Subject: [PATCH 6/6] test: update test for new html scrub policy --- test/web/common_api/common_api_test.exs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index cd5aca961..cd36e409c 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -26,7 +26,7 @@ test "it adds emoji when updating profiles" do test "it filters out obviously bad tags when accepting a post as HTML" do user = insert(:user) - post = "

2hu

" + post = "

2hu

" {:ok, activity} = CommonAPI.post(user, %{ @@ -35,13 +35,13 @@ test "it filters out obviously bad tags when accepting a post as HTML" do }) content = activity.data["object"]["content"] - assert content == "

2hu

alert('xss')" + assert content == "

2hu

alert('xss')" end test "it filters out obviously bad tags when accepting a post as Markdown" do user = insert(:user) - post = "

2hu

" + post = "

2hu

" {:ok, activity} = CommonAPI.post(user, %{ @@ -50,7 +50,7 @@ test "it filters out obviously bad tags when accepting a post as Markdown" do }) content = activity.data["object"]["content"] - assert content == "

2hu

alert('xss')" + assert content == "

2hu

alert('xss')" end end end