Merge branch 'features/staticfe-sanitization' into 'develop'
authorrinpatch <rinpatch@sdf.org>
Sun, 15 Mar 2020 19:53:52 +0000 (19:53 +0000)
committerrinpatch <rinpatch@sdf.org>
Sun, 15 Mar 2020 21:14:04 +0000 (00:14 +0300)
static_fe: Sanitize HTML

Closes #1614

See merge request pleroma/pleroma!2299

lib/pleroma/user.ex
lib/pleroma/web/activity_pub/views/user_view.ex
lib/pleroma/web/admin_api/views/account_view.ex
lib/pleroma/web/mastodon_api/views/account_view.ex
lib/pleroma/web/static_fe/static_fe_controller.ex
mix.lock
test/web/static_fe/static_fe_controller_test.exs

index 7531757f5b53e18ab968bea682b0eec24ddeea17..c686163854c237d17ba8c98ef32b4ca1973732c8 100644 (file)
@@ -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
index c0358b6787b55f262398ee57d2fc39cda2c2097e..bc21ac6c72804e0480bd3188902fd72285dc22a6 100644 (file)
@@ -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"))
 
     %{
index 619390ef4e51ce0518bf52882ea4363ab5adfac1..1e03849de035f950d73ee99bf91011b1201ba892 100644 (file)
@@ -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,
index 6dc19125022e1576fa430a91603c96dd39d0345a..341dc2c91e2f059a7ced3c4c50d4aca387796138 100644 (file)
@@ -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: %{
index 5ac75f1c458b9587f0ff5a5e62927abf5e31dec6..98977bc19bf679e30bb2341f797154dfcfa926b8 100644 (file)
@@ -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,
index 1b4fbc92708944987e664d693382c698c3ff1bad..62e14924a648604d46120a3c4e78fe5582b24c58 100644 (file)
--- 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"},
index 2ce8f9fa30e7516f0d4cb199112bba201da547d2..a3be908887c3c407a7745a0095268bac99f0f93f 100644 (file)
@@ -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, %{