Nothing much just a build up of things
[anni] / 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")