**MAJOR**: port tootsuite#13161 to monsterfork: Fix leak of arbitrary statuses through unfavourite action in REST API

5 files changed, 36 insertions, 52 deletions

diff --git a/app/controllers/api/v1/statuses/bookmarks_controller.rb b/app/controllers/api/v1/statuses/bookmarks_controller.rb
index d7adda285..c1dfba499 100644
--- a/app/controllers/api/v1/statuses/bookmarks_controller.rb
+++ b/app/controllers/api/v1/statuses/bookmarks_controller.rb
@@ -5,38 +5,30 @@ class Api::V1::Statuses::BookmarksController < Api::BaseController
 
   before_action -> { doorkeeper_authorize! :write, :'write:bookmarks' }
   before_action :require_user!
+  before_action :set_status
 
   respond_to :json
 
   def create
-    @status = bookmarked_status
+    current_account.bookmarks.find_or_create_by!(account: current_account, status: @status)
+    curate_status(@status)
     render json: @status, serializer: REST::StatusSerializer, monsterfork_api: monsterfork_api
   end
 
   def destroy
-    @status = requested_status
-    @bookmarks_map = { @status.id => false }
+    bookmark = current_account.bookmarks.find_by(status: @status)
+    bookmark&.destroy!
 
-    bookmark = Bookmark.find_by!(account: current_user.account, status: @status)
-    bookmark.destroy!
-
-    render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_user&.account_id, bookmarks_map: @bookmarks_map), monsterfork_api: monsterfork_api
+    render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_account.id, bookmarks_map: { @status.id => false }), monsterfork_api: monsterfork_api
   end
 
   private
 
-  def bookmarked_status
-    authorize_with current_user.account, requested_status, :show?
-
-    bookmark = Bookmark.find_or_create_by!(account: current_user.account, status: requested_status)
-
-    curate_status(requested_status)
-
-    bookmark.status.reload
-  end
-
-  def requested_status
-    Status.find(params[:status_id])
+  def set_status
+    @status = Status.find(params[:status_id])
+    authorize @status, :show?
+  rescue Mastodon::NotPermittedError
+    not_found
   end
 
   def curate_status(status)
diff --git a/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb b/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb
index 80621881a..1686608db 100644
--- a/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb
+++ b/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb
@@ -68,8 +68,7 @@ class Api::V1::Statuses::FavouritedByAccountsController < Api::BaseController
     @status = Status.find(params[:status_id])
     authorize @status, :show?
   rescue Mastodon::NotPermittedError
-    # Reraise in order to get a 404 instead of a 403 error code
-    raise ActiveRecord::RecordNotFound
+    not_found
   end
 
   def pagination_params(core_params)
diff --git a/app/controllers/api/v1/statuses/favourites_controller.rb b/app/controllers/api/v1/statuses/favourites_controller.rb
index 8529bcd03..11c98609b 100644
--- a/app/controllers/api/v1/statuses/favourites_controller.rb
+++ b/app/controllers/api/v1/statuses/favourites_controller.rb
@@ -5,34 +5,26 @@ class Api::V1::Statuses::FavouritesController < Api::BaseController
 
   before_action -> { doorkeeper_authorize! :write, :'write:favourites' }
   before_action :require_user!
+  before_action :set_status
 
   respond_to :json
 
   def create
-    @status = favourited_status
+    FavouriteService.new.call(current_account, @status)
     render json: @status, serializer: REST::StatusSerializer, monsterfork_api: monsterfork_api
   end
 
   def destroy
-    @status = requested_status
-    @favourites_map = { @status.id => false }
-
-    UnfavouriteWorker.perform_async(current_user.account_id, @status.id)
-
-    render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_user&.account_id, favourites_map: @favourites_map), monsterfork_api: monsterfork_api
+    UnfavouriteWorker.perform_async(current_account.id, @status.id)
+    render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_account.id, favourites_map: { @status.id => false }), monsterfork_api: monsterfork_api
   end
 
   private
 
-  def favourited_status
-    service_result.status.reload
-  end
-
-  def service_result
-    FavouriteService.new.call(current_user.account, requested_status)
-  end
-
-  def requested_status
-    Status.find(params[:status_id])
+  def set_status
+    @status = Status.find(params[:status_id])
+    authorize @status, :show?
+  rescue Mastodon::NotPermittedError
+    not_found
   end
 end
diff --git a/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb b/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb
index 58faa05b1..22aa66c31 100644
--- a/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb
+++ b/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb
@@ -65,8 +65,7 @@ class Api::V1::Statuses::RebloggedByAccountsController < Api::BaseController
     @status = Status.find(params[:status_id])
     authorize @status, :show?
   rescue Mastodon::NotPermittedError
-    # Reraise in order to get a 404 instead of a 403 error code
-    raise ActiveRecord::RecordNotFound
+    not_found
   end
 
   def pagination_params(core_params)
diff --git a/app/controllers/api/v1/statuses/reblogs_controller.rb b/app/controllers/api/v1/statuses/reblogs_controller.rb
index 986ce120b..d76055cfa 100644
--- a/app/controllers/api/v1/statuses/reblogs_controller.rb
+++ b/app/controllers/api/v1/statuses/reblogs_controller.rb
@@ -5,32 +5,34 @@ class Api::V1::Statuses::ReblogsController < Api::BaseController
 
   before_action -> { doorkeeper_authorize! :write, :'write:statuses' }
   before_action :require_user!
+  before_action :set_reblog
 
   respond_to :json
 
   def create
-    @status = ReblogService.new.call(current_user.account, status_for_reblog, reblog_params)
+    @status = ReblogService.new.call(current_account, @reblog, reblog_params)
     render json: @status, serializer: REST::StatusSerializer, monsterfork_api: monsterfork_api
   end
 
   def destroy
-    @status = status_for_destroy.reblog
-    @reblogs_map = { @status.id => false }
+    @status = current_account.statuses.find_by(reblog_of_id: @reblog.id)
 
-    authorize status_for_destroy, :unreblog?
-    RemovalWorker.perform_async(status_for_destroy.id)
+    if @status
+      authorize @status, :unreblog?
+      @status.discard
+      RemovalWorker.perform_async(@status.id)
+    end
 
-    render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_user&.account_id, reblogs_map: @reblogs_map), monsterfork_api: monsterfork_api
+    render json: @reblog, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_account.id, reblogs_map: { @reblog.id => false }), monsterfork_api: monsterfork_api
   end
 
   private
 
-  def status_for_reblog
-    Status.find params[:status_id]
-  end
-
-  def status_for_destroy
-    current_user.account.statuses.where(reblog_of_id: params[:status_id]).first!
+  def set_reblog
+    @reblog = Status.find(params[:status_id])
+    authorize @reblog, :show?
+  rescue Mastodon::NotPermittedError
+    not_found
   end
 
   def reblog_params