Merge branch 'features/staticfe-sanitization' into 'develop'

static_fe: Sanitize HTML Closes #1614 See merge request pleroma/pleroma!2299
2020-03-15 19:53:52 +00:00 · 2020-03-15 19:53:52 +00:00 · fcf51a77ba
commit fcf51a77ba
parent 8096bfb891
7 changed files with 56 additions and 29 deletions
--- a/lib/pleroma/user.ex
+++ b/lib/pleroma/user.ex
@ -16,6 +16,7 @@ defmodule Pleroma.User do
  alias Pleroma.Conversation.Participation
  alias Pleroma.Delivery
  alias Pleroma.FollowingRelationship
+  alias Pleroma.HTML
  alias Pleroma.Keys
  alias Pleroma.Notification
  alias Pleroma.Object
@ -2062,4 +2063,27 @@ defmodule Pleroma.User do
    |> validate_required([:invisible])
    |> update_and_set_cache()
  end
+
+  def sanitize_html(%User{} = user) do
+    sanitize_html(user, nil)
+  end
+
+  # User data that mastodon isn't filtering (treated as plaintext):
+  # - field name
+  # - display name
+  def sanitize_html(%User{} = user, filter) do
+    fields =
+      user
+      |> User.fields()
+      |> Enum.map(fn %{"name" => name, "value" => value} ->
+        %{
+          "name" => name,
+          "value" => HTML.filter_tags(value, Pleroma.HTML.Scrubber.LinksOnly)
+        }
+      end)
+
+    user
+    |> Map.put(:bio, HTML.filter_tags(user.bio, filter))
+    |> Map.put(:fields, fields)
+  end
 end
--- a/lib/pleroma/web/activity_pub/views/user_view.ex
+++ b/lib/pleroma/web/activity_pub/views/user_view.ex
@ -73,6 +73,7 @@ defmodule Pleroma.Web.ActivityPub.UserView do
    {:ok, _, public_key} = Keys.keys_from_pem(user.keys)
    public_key = :public_key.pem_entry_encode(:SubjectPublicKeyInfo, public_key)
    public_key = :public_key.pem_encode([public_key])
+    user = User.sanitize_html(user)

    endpoints = render("endpoints.json", %{user: user})

@ -81,12 +82,6 @@ defmodule Pleroma.Web.ActivityPub.UserView do
    fields =
      user
      |> User.fields()
-      |> Enum.map(fn %{"name" => name, "value" => value} ->
-        %{
-          "name" => Pleroma.HTML.strip_tags(name),
-          "value" => Pleroma.HTML.filter_tags(value, Pleroma.HTML.Scrubber.LinksOnly)
-        }
-      end)
      |> Enum.map(&Map.put(&1, "type", "PropertyValue"))

    %{
--- a/lib/pleroma/web/admin_api/views/account_view.ex
+++ b/lib/pleroma/web/admin_api/views/account_view.ex
@ -5,7 +5,6 @@
 defmodule Pleroma.Web.AdminAPI.AccountView do
  use Pleroma.Web, :view

-  alias Pleroma.HTML
  alias Pleroma.User
  alias Pleroma.Web.AdminAPI.AccountView
  alias Pleroma.Web.MediaProxy
@ -26,7 +25,8 @@ defmodule Pleroma.Web.AdminAPI.AccountView do

  def render("show.json", %{user: user}) do
    avatar = User.avatar_url(user) |> MediaProxy.url()
-    display_name = HTML.strip_tags(user.name || user.nickname)
+    display_name = Pleroma.HTML.strip_tags(user.name || user.nickname)
+    user = User.sanitize_html(user, FastSanitize.Sanitizer.StripTags)

    %{
      "id" => user.id,
--- a/lib/pleroma/web/mastodon_api/views/account_view.ex
+++ b/lib/pleroma/web/mastodon_api/views/account_view.ex
@ -5,7 +5,6 @@
 defmodule Pleroma.Web.MastodonAPI.AccountView do
  use Pleroma.Web, :view

-  alias Pleroma.HTML
  alias Pleroma.User
  alias Pleroma.Web.CommonAPI.Utils
  alias Pleroma.Web.MastodonAPI.AccountView
@ -67,6 +66,7 @@ defmodule Pleroma.Web.MastodonAPI.AccountView do
  end

  defp do_render("show.json", %{user: user} = opts) do
+    user = User.sanitize_html(user, User.html_filter_policy(opts[:for]))
    display_name = user.name || user.nickname

    image = User.avatar_url(user) |> MediaProxy.url()
@ -100,17 +100,6 @@ defmodule Pleroma.Web.MastodonAPI.AccountView do
        }
      end)

-    fields =
-      user
-      |> User.fields()
-      |> Enum.map(fn %{"name" => name, "value" => value} ->
-        %{
-          "name" => name,
-          "value" => Pleroma.HTML.filter_tags(value, Pleroma.HTML.Scrubber.LinksOnly)
-        }
-      end)
-
-    bio = HTML.filter_tags(user.bio, User.html_filter_policy(opts[:for]))
    relationship = render("relationship.json", %{user: opts[:for], target: user})

    %{
@ -123,17 +112,17 @@ defmodule Pleroma.Web.MastodonAPI.AccountView do
      followers_count: followers_count,
      following_count: following_count,
      statuses_count: user.note_count,
-      note: bio || "",
+      note: user.bio || "",
      url: User.profile_url(user),
      avatar: image,
      avatar_static: image,
      header: header,
      header_static: header,
      emojis: emojis,
-      fields: fields,
+      fields: user.fields,
      bot: bot,
      source: %{
-        note: HTML.strip_tags((user.bio || "") |> String.replace("<br>", "\n")),
+        note: Pleroma.HTML.strip_tags((user.bio || "") |> String.replace("<br>", "\n")),
        sensitive: false,
        fields: user.raw_fields,
        pleroma: %{
--- a/lib/pleroma/web/static_fe/static_fe_controller.ex
+++ b/lib/pleroma/web/static_fe/static_fe_controller.ex
@ -54,10 +54,17 @@ defmodule Pleroma.Web.StaticFE.StaticFEController do
        _ -> data["url"] || data["external_url"] || data["id"]
      end

+    content =
+      if data["content"] do
+        Pleroma.HTML.filter_tags(data["content"])
+      else
+        nil
+      end
+
    %{
-      user: user,
+      user: User.sanitize_html(user),
      title: get_title(activity.object),
-      content: data["content"] || nil,
+      content: content,
      attachment: data["attachment"],
      link: link,
      published: data["published"],
@ -109,7 +116,7 @@ defmodule Pleroma.Web.StaticFE.StaticFEController do
        next_page_id = List.last(timeline) && List.last(timeline).id

        render(conn, "profile.html", %{
-          user: user,
+          user: User.sanitize_html(user),
          timeline: timeline,
          prev_page_id: prev_page_id,
          next_page_id: next_page_id,
--- a/mix.lock
+++ b/mix.lock
@ -4,10 +4,10 @@
  "base62": {:hex, :base62, "1.2.1", "4866763e08555a7b3917064e9eef9194c41667276c51b59de2bc42c6ea65f806", [:mix], [{:custom_base, "~> 0.2.1", [hex: :custom_base, repo: "hexpm", optional: false]}], "hexpm", "3b29948de2013d3f93aa898c884a9dff847e7aec75d9d6d8c1dc4c61c2716c42"},
  "base64url": {:hex, :base64url, "0.0.1", "36a90125f5948e3afd7be97662a1504b934dd5dac78451ca6e9abf85a10286be", [:rebar], [], "hexpm"},
  "bbcode": {:git, "https://git.pleroma.social/pleroma/elixir-libraries/bbcode.git", "f2d267675e9a7e1ad1ea9beb4cc23382762b66c2", [ref: "v0.2.0"]},
-  "bbcode_pleroma": {:hex, :bbcode_pleroma, "0.2.0", "d36f5bca6e2f62261c45be30fa9b92725c0655ad45c99025cb1c3e28e25803ef", [:mix], [{:nimble_parsec, "~> 0.5", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm"},
+  "bbcode_pleroma": {:hex, :bbcode_pleroma, "0.2.0", "d36f5bca6e2f62261c45be30fa9b92725c0655ad45c99025cb1c3e28e25803ef", [:mix], [{:nimble_parsec, "~> 0.5", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "19851074419a5fedb4ef49e1f01b30df504bb5dbb6d6adfc135238063bebd1c3"},
  "benchee": {:hex, :benchee, "1.0.1", "66b211f9bfd84bd97e6d1beaddf8fc2312aaabe192f776e8931cb0c16f53a521", [:mix], [{:deep_merge, "~> 1.0", [hex: :deep_merge, repo: "hexpm", optional: false]}], "hexpm", "3ad58ae787e9c7c94dd7ceda3b587ec2c64604563e049b2a0e8baafae832addb"},
  "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm", "7af5c7e09fe1d40f76c8e4f9dd2be7cebd83909f31fee7cd0e9eadc567da8353"},
-  "cachex": {:hex, :cachex, "3.2.0", "a596476c781b0646e6cb5cd9751af2e2974c3e0d5498a8cab71807618b74fe2f", [:mix], [{:eternal, "~> 1.2", [hex: :eternal, repo: "hexpm", optional: false]}, {:jumper, "~> 1.0", [hex: :jumper, repo: "hexpm", optional: false]}, {:sleeplocks, "~> 1.1", [hex: :sleeplocks, repo: "hexpm", optional: false]}, {:unsafe, "~> 1.0", [hex: :unsafe, repo: "hexpm", optional: false]}], "hexpm"},
+  "cachex": {:hex, :cachex, "3.2.0", "a596476c781b0646e6cb5cd9751af2e2974c3e0d5498a8cab71807618b74fe2f", [:mix], [{:eternal, "~> 1.2", [hex: :eternal, repo: "hexpm", optional: false]}, {:jumper, "~> 1.0", [hex: :jumper, repo: "hexpm", optional: false]}, {:sleeplocks, "~> 1.1", [hex: :sleeplocks, repo: "hexpm", optional: false]}, {:unsafe, "~> 1.0", [hex: :unsafe, repo: "hexpm", optional: false]}], "hexpm", "aef93694067a43697ae0531727e097754a9e992a1e7946296f5969d6dd9ac986"},
  "calendar": {:hex, :calendar, "0.17.6", "ec291cb2e4ba499c2e8c0ef5f4ace974e2f9d02ae9e807e711a9b0c7850b9aee", [:mix], [{:tzdata, "~> 0.5.20 or ~> 0.1.201603 or ~> 1.0", [hex: :tzdata, repo: "hexpm", optional: false]}], "hexpm", "738d0e17a93c2ccfe4ddc707bdc8e672e9074c8569498483feb1c4530fb91b2b"},
  "captcha": {:git, "https://git.pleroma.social/pleroma/elixir-libraries/elixir-captcha.git", "e0f16822d578866e186a0974d65ad58cddc1e2ab", [ref: "e0f16822d578866e186a0974d65ad58cddc1e2ab"]},
  "certifi": {:hex, :certifi, "2.5.1", "867ce347f7c7d78563450a18a6a28a8090331e77fa02380b4a21962a65d36ee5", [:rebar3], [{:parse_trans, "~>3.3", [hex: :parse_trans, repo: "hexpm", optional: false]}], "hexpm", "805abd97539caf89ec6d4732c91e62ba9da0cda51ac462380bbd28ee697a8c42"},
--- a/test/web/static_fe/static_fe_controller_test.exs
+++ b/test/web/static_fe/static_fe_controller_test.exs
@ -110,8 +110,20 @@ defmodule Pleroma.Web.StaticFE.StaticFEControllerTest do
      assert html =~ "testing a thing!"
    end

-    test "shows the whole thread", %{conn: conn} do
+    test "filters HTML tags", %{conn: conn} do
      user = insert(:user)
+      {:ok, activity} = CommonAPI.post(user, %{"status" => "<script>alert('xss')</script>"})
+
+      conn =
+        conn
+        |> put_req_header("accept", "text/html")
+        |> get("/notice/#{activity.id}")
+
+      html = html_response(conn, 200)
+      assert html =~ ~s[&lt;script&gt;alert(&#39;xss&#39;)&lt;/script&gt;]
+    end
+
+    test "shows the whole thread", %{conn: conn, user: user} do
      {:ok, activity} = CommonAPI.post(user, %{"status" => "space: the final frontier"})

      CommonAPI.post(user, %{