Git of dcc - anni/blob - patches/8(2.5.5).diff

   1 diff --git a/CHANGELOG.md b/CHANGELOG.md
   2 index 9d9aadc6e8a0162d8944622f783a6301fefd6cfa..32ec440de55b707d01be37e21f1517542c9cf7d9 100644
   3 --- a/CHANGELOG.md
   4 +++ b/CHANGELOG.md
   5 @@ -14,7 +14,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   6
   7  ### Removed
   8
   9 -## 2.5.54
  10 +## 2.5.5
  11 +
  12 +## Security
  13 +- Prevent users from accessing media of other users by creating a status with reused attachment ID
  14 +
  15 +## 2.5.4
  16
  17  ## Security
  18  - Fix XML External Entity (XXE) loading vulnerability allowing to fetch arbitary files from the server's filesystem
  19 diff --git a/changelog.d/check-attachment-attribution.security b/changelog.d/check-attachment-attribution.security
  20 new file mode 100644
  21 index 0000000000000000000000000000000000000000..e0e46525b38d15e58423debbf9d799e85276d0ca
  22 --- /dev/null
  23 +++ b/changelog.d/check-attachment-attribution.security
  24 @@ -0,0 +1 @@
  25 +CommonAPI: Prevent users from accessing media of other users by creating a status with reused attachment ID
  26 diff --git a/lib/pleroma/scheduled_activity.ex b/lib/pleroma/scheduled_activity.ex
  27 index a7be585123d4be67e7513626b07c2e33c8b18388..0ed51ad07d43c0cc434e532852ce6a7ec1089d10 100644
  28 --- a/lib/pleroma/scheduled_activity.ex
  29 +++ b/lib/pleroma/scheduled_activity.ex
  30 @@ -40,7 +40,11 @@ defp with_media_attachments(
  31           %{changes: %{params: %{"media_ids" => media_ids} = params}} = changeset
  32         )
  33         when is_list(media_ids) do
  34 -    media_attachments = Utils.attachments_from_ids(%{media_ids: media_ids})
  35 +    media_attachments =
  36 +      Utils.attachments_from_ids(
  37 +        %{media_ids: media_ids},
  38 +        User.get_cached_by_id(changeset.data.user_id)
  39 +      )
  40
  41      params =
  42        params
  43 diff --git a/lib/pleroma/web/common_api.ex b/lib/pleroma/web/common_api.ex
  44 index 89cc0d6fe82acfd418e8e5928bc4b44d72b17f79..44eb00075ee694005241c1eabfbb846e4668d665 100644
  45 --- a/lib/pleroma/web/common_api.ex
  46 +++ b/lib/pleroma/web/common_api.ex
  47 @@ -33,6 +33,7 @@ def block(blocker, blocked) do
  48
  49    def post_chat_message(%User{} = user, %User{} = recipient, content, opts \\ []) do
  50      with maybe_attachment <- opts[:media_id] && Object.get_by_id(opts[:media_id]),
  51 +         :ok <- validate_chat_attachment_attribution(maybe_attachment, user),
  52           :ok <- validate_chat_content_length(content, !!maybe_attachment),
  53           {_, {:ok, chat_message_data, _meta}} <-
  54             {:build_object,
  55 @@ -71,6 +72,17 @@ defp format_chat_content(content) do
  56      text
  57    end
  58
  59 +  defp validate_chat_attachment_attribution(nil, _), do: :ok
  60 +
  61 +  defp validate_chat_attachment_attribution(attachment, user) do
  62 +    with :ok <- Object.authorize_access(attachment, user) do
  63 +      :ok
  64 +    else
  65 +      e ->
  66 +        e
  67 +    end
  68 +  end
  69 +
  70    defp validate_chat_content_length(_, true), do: :ok
  71    defp validate_chat_content_length(nil, false), do: {:error, :no_content}
  72
  73 diff --git a/lib/pleroma/web/common_api/activity_draft.ex b/lib/pleroma/web/common_api/activity_draft.ex
  74 index 9af635da8abafb9d9a4c1b59c00cc63261955d48..63ed48a27bbd4a4f2fdfd97776acadcf0fa72c74 100644
  75 --- a/lib/pleroma/web/common_api/activity_draft.ex
  76 +++ b/lib/pleroma/web/common_api/activity_draft.ex
  77 @@ -111,7 +111,7 @@ defp full_payload(%{status: status, summary: summary} = draft) do
  78    end
  79
  80    defp attachments(%{params: params} = draft) do
  81 -    attachments = Utils.attachments_from_ids(params)
  82 +    attachments = Utils.attachments_from_ids(params, draft.user)
  83      draft = %__MODULE__{draft | attachments: attachments}
  84
  85      case Utils.validate_attachments_count(attachments) do
  86 diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex
  87 index ff0814329583726d43ad52e6082662aee1212b45..6410815ea89153a65f20bacc24a5a5aed82a1aa0 100644
  88 --- a/lib/pleroma/web/common_api/utils.ex
  89 +++ b/lib/pleroma/web/common_api/utils.ex
  90 @@ -23,21 +23,21 @@ defmodule Pleroma.Web.CommonAPI.Utils do
  91    require Logger
  92    require Pleroma.Constants
  93
  94 -  def attachments_from_ids(%{media_ids: ids, descriptions: desc}) do
  95 -    attachments_from_ids_descs(ids, desc)
  96 +  def attachments_from_ids(%{media_ids: ids, descriptions: desc}, user) do
  97 +    attachments_from_ids_descs(ids, desc, user)
  98    end
  99
 100 -  def attachments_from_ids(%{media_ids: ids}) do
 101 -    attachments_from_ids_no_descs(ids)
 102 +  def attachments_from_ids(%{media_ids: ids}, user) do
 103 +    attachments_from_ids_no_descs(ids, user)
 104    end
 105
 106 -  def attachments_from_ids(_), do: []
 107 +  def attachments_from_ids(_, _), do: []
 108
 109 -  def attachments_from_ids_no_descs([]), do: []
 110 +  def attachments_from_ids_no_descs([], _), do: []
 111
 112 -  def attachments_from_ids_no_descs(ids) do
 113 +  def attachments_from_ids_no_descs(ids, user) do
 114      Enum.map(ids, fn media_id ->
 115 -      case get_attachment(media_id) do
 116 +      case get_attachment(media_id, user) do
 117          %Object{data: data} -> data
 118          _ -> nil
 119        end
 120 @@ -45,21 +45,26 @@ def attachments_from_ids_no_descs(ids) do
 121      |> Enum.reject(&is_nil/1)
 122    end
 123
 124 -  def attachments_from_ids_descs([], _), do: []
 125 +  def attachments_from_ids_descs([], _, _), do: []
 126
 127 -  def attachments_from_ids_descs(ids, descs_str) do
 128 +  def attachments_from_ids_descs(ids, descs_str, user) do
 129      {_, descs} = Jason.decode(descs_str)
 130
 131      Enum.map(ids, fn media_id ->
 132 -      with %Object{data: data} <- get_attachment(media_id) do
 133 +      with %Object{data: data} <- get_attachment(media_id, user) do
 134          Map.put(data, "name", descs[media_id])
 135        end
 136      end)
 137      |> Enum.reject(&is_nil/1)
 138    end
 139
 140 -  defp get_attachment(media_id) do
 141 -    Repo.get(Object, media_id)
 142 +  defp get_attachment(media_id, user) do
 143 +    with %Object{data: _data} = object <- Repo.get(Object, media_id),
 144 +         :ok <- Object.authorize_access(object, user) do
 145 +      object
 146 +    else
 147 +      _ -> nil
 148 +    end
 149    end
 150
 151    @spec get_to_and_cc(ActivityDraft.t()) :: {list(String.t()), list(String.t())}
 152 diff --git a/mix.exs b/mix.exs
 153 index 12f721364dd75744651e5044936d195684d8cf08..e2aac0fc541f2820841a16feff06b058b18ed2ec 100644
 154 --- a/mix.exs
 155 +++ b/mix.exs
 156 @@ -4,7 +4,7 @@ defmodule Pleroma.Mixfile do
 157    def project do
 158      [
 159        app: :pleroma,
 160 -      version: version("2.5.4"),
 161 +      version: version("2.5.5"),
 162        elixir: "~> 1.11",
 163        elixirc_paths: elixirc_paths(Mix.env()),
 164        compilers: [:phoenix, :gettext] ++ Mix.compilers(),
 165 diff --git a/test/pleroma/web/common_api/utils_test.exs b/test/pleroma/web/common_api/utils_test.exs
 166 index d309c6deda789fd7818b29e95425b77413b06f7c..c52d3e9c557b6040bb457f40d9ebd02c31975a64 100644
 167 --- a/test/pleroma/web/common_api/utils_test.exs
 168 +++ b/test/pleroma/web/common_api/utils_test.exs
 169 @@ -586,41 +586,56 @@ test "returns recipients when object not found" do
 170      end
 171    end
 172
 173 -  describe "attachments_from_ids_descs/2" do
 174 +  describe "attachments_from_ids_descs/3" do
 175      test "returns [] when attachment ids is empty" do
 176 -      assert Utils.attachments_from_ids_descs([], "{}") == []
 177 +      assert Utils.attachments_from_ids_descs([], "{}", nil) == []
 178      end
 179
 180      test "returns list attachments with desc" do
 181 -      object = insert(:note)
 182 +      user = insert(:user)
 183 +      object = insert(:note, %{user: user})
 184        desc = Jason.encode!(%{object.id => "test-desc"})
 185
 186 -      assert Utils.attachments_from_ids_descs(["#{object.id}", "34"], desc) == [
 187 +      assert Utils.attachments_from_ids_descs(["#{object.id}", "34"], desc, user) == [
 188                 Map.merge(object.data, %{"name" => "test-desc"})
 189               ]
 190      end
 191    end
 192
 193 -  describe "attachments_from_ids/1" do
 194 +  describe "attachments_from_ids/2" do
 195      test "returns attachments with descs" do
 196 -      object = insert(:note)
 197 +      user = insert(:user)
 198 +      object = insert(:note, %{user: user})
 199        desc = Jason.encode!(%{object.id => "test-desc"})
 200
 201 -      assert Utils.attachments_from_ids(%{
 202 -               media_ids: ["#{object.id}"],
 203 -               descriptions: desc
 204 -             }) == [
 205 +      assert Utils.attachments_from_ids(
 206 +               %{
 207 +                 media_ids: ["#{object.id}"],
 208 +                 descriptions: desc
 209 +               },
 210 +               user
 211 +             ) == [
 212                 Map.merge(object.data, %{"name" => "test-desc"})
 213               ]
 214      end
 215
 216      test "returns attachments without descs" do
 217 -      object = insert(:note)
 218 -      assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}) == [object.data]
 219 +      user = insert(:user)
 220 +      object = insert(:note, %{user: user})
 221 +      assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}, user) == [object.data]
 222      end
 223
 224      test "returns [] when not pass media_ids" do
 225 -      assert Utils.attachments_from_ids(%{}) == []
 226 +      assert Utils.attachments_from_ids(%{}, nil) == []
 227 +    end
 228 +
 229 +    test "returns [] when media_ids not belong to current user" do
 230 +      user = insert(:user)
 231 +      user2 = insert(:user)
 232 +
 233 +      object = insert(:attachment, %{user: user})
 234 +
 235 +      assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}, user2) == []
 236      end
 237    end
 238
 239 diff --git a/test/pleroma/web/common_api_test.exs b/test/pleroma/web/common_api_test.exs
 240 index 5c9103e9fc742b8082a432d8ac9ca2ce1929ec32..e60691995c51bdd829f0f67a3abbddc9fe919db9 100644
 241 --- a/test/pleroma/web/common_api_test.exs
 242 +++ b/test/pleroma/web/common_api_test.exs
 243 @@ -279,6 +279,24 @@ test "it reject messages via MRF" do
 244        assert {:reject, "[KeywordPolicy] Matches with rejected keyword"} ==
 245                 CommonAPI.post_chat_message(author, recipient, "GNO/Linux")
 246      end
 247 +
 248 +    test "it reject messages with attachments not belonging to user" do
 249 +      author = insert(:user)
 250 +      not_author = insert(:user)
 251 +      recipient = author
 252 +
 253 +      attachment = insert(:attachment, %{user: not_author})
 254 +
 255 +      {:error, message} =
 256 +        CommonAPI.post_chat_message(
 257 +          author,
 258 +          recipient,
 259 +          "123",
 260 +          media_id: attachment.id
 261 +        )
 262 +
 263 +      assert message == :forbidden
 264 +    end
 265    end
 266
 267    describe "unblocking" do
 268 diff --git a/test/pleroma/web/mastodon_api/views/scheduled_activity_view_test.exs b/test/pleroma/web/mastodon_api/views/scheduled_activity_view_test.exs
 269 index e5e510d33b88244b9b088ed93b98bfb2c411dd8b..07a65a3bc1bd09ece6afa2557a719b7550dfae2a 100644
 270 --- a/test/pleroma/web/mastodon_api/views/scheduled_activity_view_test.exs
 271 +++ b/test/pleroma/web/mastodon_api/views/scheduled_activity_view_test.exs
 272 @@ -48,7 +48,7 @@ test "A scheduled activity with a media attachment" do
 273        id: to_string(scheduled_activity.id),
 274        media_attachments:
 275          %{media_ids: [upload.id]}
 276 -        |> Utils.attachments_from_ids()
 277 +        |> Utils.attachments_from_ids(user)
 278          |> Enum.map(&StatusView.render("attachment.json", %{attachment: &1})),
 279        params: %{
 280          in_reply_to_id: to_string(activity.id),
 281 diff --git a/test/pleroma/web/pleroma_api/views/chat_message_reference_view_test.exs b/test/pleroma/web/pleroma_api/views/chat_message_reference_view_test.exs
 282 index 017c9c5c0d2fdf4686d920ea7f05084a6ddf44d7..7ab3f5acdc7971f9ac324db07be30dbff60d680a 100644
 283 --- a/test/pleroma/web/pleroma_api/views/chat_message_reference_view_test.exs
 284 +++ b/test/pleroma/web/pleroma_api/views/chat_message_reference_view_test.exs
 285 @@ -24,7 +24,7 @@ test "it displays a chat message" do
 286        filename: "an_image.jpg"
 287      }
 288
 289 -    {:ok, upload} = ActivityPub.upload(file, actor: user.ap_id)
 290 +    {:ok, upload} = ActivityPub.upload(file, actor: recipient.ap_id)
 291
 292      {:ok, activity} =
 293        CommonAPI.post_chat_message(user, recipient, "kippis :firefox:", idempotency_key: "123")