From 5e76adb07e225a1f5934b44ad474158dfcd52233 Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Wed, 30 May 2018 20:00:27 +0200
Subject: [PATCH] Fix url guessing attacks.
---
.../activity_pub/activity_pub_controller.ex | 8 ++++-
lib/pleroma/web/ostatus/ostatus_controller.ex | 21 +++++++++--
.../activity_pub_controller_test.exs | 13 +++++++
test/web/ostatus/ostatus_controller_test.exs | 36 +++++++++++++++++++
4 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex
index c7d50893f..a6a9b99ef 100644
--- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex
+++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex
@@ -20,10 +20,16 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
def object(conn, %{"uuid" => uuid}) do
with ap_id <- o_status_url(conn, :object, uuid),
- %Object{} = object <- Object.get_cached_by_ap_id(ap_id) do
+ %Object{} = object <- Object.get_cached_by_ap_id(ap_id),
+ {_, true} <- {:public?, ActivityPub.is_public?(object)} do
conn
|> put_resp_header("content-type", "application/activity+json")
|> json(ObjectView.render("object.json", %{object: object}))
+ else
+ {:public?, false} ->
+ conn
+ |> put_status(404)
+ |> json("Not found")
end
end
diff --git a/lib/pleroma/web/ostatus/ostatus_controller.ex b/lib/pleroma/web/ostatus/ostatus_controller.ex
index f39ebaf2b..53278431e 100644
--- a/lib/pleroma/web/ostatus/ostatus_controller.ex
+++ b/lib/pleroma/web/ostatus/ostatus_controller.ex
@@ -68,37 +68,47 @@ defmodule Pleroma.Web.OStatus.OStatusController do
|> send_resp(200, "")
end
- # TODO: Data leak
def object(conn, %{"uuid" => uuid} = params) do
if get_format(conn) == "activity+json" do
ActivityPubController.object(conn, params)
else
with id <- o_status_url(conn, :object, uuid),
%Activity{} = activity <- Activity.get_create_activity_by_object_ap_id(id),
+ {_, true} <- {:public?, ActivityPub.is_public?(activity)},
%User{} = user <- User.get_cached_by_ap_id(activity.data["actor"]) do
case get_format(conn) do
"html" -> redirect(conn, to: "/notice/#{activity.id}")
_ -> represent_activity(conn, activity, user)
end
+ else
+ {:public?, false} ->
+ conn
+ |> put_status(404)
+ |> json("Not found")
end
end
end
- # TODO: Data leak
def activity(conn, %{"uuid" => uuid}) do
with id <- o_status_url(conn, :activity, uuid),
%Activity{} = activity <- Activity.get_by_ap_id(id),
+ {_, true} <- {:public?, ActivityPub.is_public?(activity)},
%User{} = user <- User.get_cached_by_ap_id(activity.data["actor"]) do
case get_format(conn) do
"html" -> redirect(conn, to: "/notice/#{activity.id}")
_ -> represent_activity(conn, activity, user)
end
+ else
+ {:public?, false} ->
+ conn
+ |> put_status(404)
+ |> json("Not found")
end
end
- # TODO: Data leak
def notice(conn, %{"id" => id}) do
with %Activity{} = activity <- Repo.get(Activity, id),
+ {_, true} <- {:public?, ActivityPub.is_public?(activity)},
%User{} = user <- User.get_cached_by_ap_id(activity.data["actor"]) do
case get_format(conn) do
"html" ->
@@ -109,6 +119,11 @@ defmodule Pleroma.Web.OStatus.OStatusController do
_ ->
represent_activity(conn, activity, user)
end
+ else
+ {:public?, false} ->
+ conn
+ |> put_status(404)
+ |> json("Not found")
end
end
diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs
index 25b47ee31..305f9d0e0 100644
--- a/test/web/activity_pub/activity_pub_controller_test.exs
+++ b/test/web/activity_pub/activity_pub_controller_test.exs
@@ -4,6 +4,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
alias Pleroma.Web.ActivityPub.{UserView, ObjectView}
alias Pleroma.{Repo, User}
alias Pleroma.Activity
+ alias Pleroma.Web.CommonAPI
describe "/users/:nickname" do
test "it returns a json representation of the user", %{conn: conn} do
@@ -32,6 +33,18 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
assert json_response(conn, 200) == ObjectView.render("object.json", %{object: note})
end
+
+ test "it returns 404 for non-public messages", %{conn: conn} do
+ note = insert(:direct_note)
+ uuid = String.split(note.data["id"], "/") |> List.last()
+
+ conn =
+ conn
+ |> put_req_header("accept", "application/activity+json")
+ |> get("/objects/#{uuid}")
+
+ assert json_response(conn, 404)
+ end
end
describe "/users/:nickname/inbox" do
diff --git a/test/web/ostatus/ostatus_controller_test.exs b/test/web/ostatus/ostatus_controller_test.exs
index ba8855093..faee4fc3e 100644
--- a/test/web/ostatus/ostatus_controller_test.exs
+++ b/test/web/ostatus/ostatus_controller_test.exs
@@ -77,6 +77,19 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest do
assert response(conn, 200) == expected
end
+ test "404s on private objects", %{conn: conn} do
+ note_activity = insert(:direct_note_activity)
+ user = User.get_by_ap_id(note_activity.data["actor"])
+ [_, uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, note_activity.data["object"]["id"]))
+ url = "/objects/#{uuid}"
+
+ conn =
+ conn
+ |> get(url)
+
+ assert response(conn, 404)
+ end
+
test "gets an activity", %{conn: conn} do
note_activity = insert(:note_activity)
[_, uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, note_activity.data["id"]))
@@ -89,6 +102,18 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest do
assert response(conn, 200)
end
+ test "404s on private activities", %{conn: conn} do
+ note_activity = insert(:direct_note_activity)
+ [_, uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, note_activity.data["id"]))
+ url = "/activities/#{uuid}"
+
+ conn =
+ conn
+ |> get(url)
+
+ assert response(conn, 404)
+ end
+
test "gets a notice", %{conn: conn} do
note_activity = insert(:note_activity)
url = "/notice/#{note_activity.id}"
@@ -99,4 +124,15 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest do
assert response(conn, 200)
end
+
+ test "404s a private notice", %{conn: conn} do
+ note_activity = insert(:direct_note_activity)
+ url = "/notice/#{note_activity.id}"
+
+ conn =
+ conn
+ |> get(url)
+
+ assert response(conn, 404)
+ end
end