From 0c28a505dddd13e2773cd3d5e0beef76a21eb415 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 27 Feb 2020 12:32:54 +0100 Subject: [PATCH 1/2] Fix leak of arbitrary statuses through unfavourite action in REST API (#13161) --- .../api/v1/statuses/bookmarks_controller.rb | 27 +++--- .../favourited_by_accounts_controller.rb | 3 +- .../api/v1/statuses/favourites_controller.rb | 26 ++---- .../reblogged_by_accounts_controller.rb | 3 +- .../api/v1/statuses/reblogs_controller.rb | 27 +++--- .../v1/statuses/bookmarks_controller_spec.rb | 67 +++++++++++---- .../v1/statuses/favourites_controller_spec.rb | 86 +++++++++++++------ .../v1/statuses/reblogs_controller_spec.rb | 86 +++++++++++++------ 8 files changed, 202 insertions(+), 123 deletions(-) diff --git a/app/controllers/api/v1/statuses/bookmarks_controller.rb b/app/controllers/api/v1/statuses/bookmarks_controller.rb index bb9729cf5..a7f1eed00 100644 --- a/app/controllers/api/v1/statuses/bookmarks_controller.rb +++ b/app/controllers/api/v1/statuses/bookmarks_controller.rb @@ -5,35 +5,28 @@ 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) render json: @status, serializer: REST::StatusSerializer 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) + render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_account.id, bookmarks_map: { @status.id => false }) 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) - - 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 end 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 99eff360e..05f4acc33 100644 --- a/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb +++ b/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb @@ -69,8 +69,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 cceee9060..f18ace996 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 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) + 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 }) 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 cc285ad23..fa60e7d84 100644 --- a/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb +++ b/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb @@ -66,8 +66,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 42381a37f..67106ccbe 100644 --- a/app/controllers/api/v1/statuses/reblogs_controller.rb +++ b/app/controllers/api/v1/statuses/reblogs_controller.rb @@ -5,33 +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 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? - status_for_destroy.discard - 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) + render json: @reblog, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_account.id, reblogs_map: { @reblog.id => false }) end private - def status_for_reblog - Status.find params[:status_id] - end - - def status_for_destroy - @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 diff --git a/spec/controllers/api/v1/statuses/bookmarks_controller_spec.rb b/spec/controllers/api/v1/statuses/bookmarks_controller_spec.rb index b79853718..aa5ca433f 100644 --- a/spec/controllers/api/v1/statuses/bookmarks_controller_spec.rb +++ b/spec/controllers/api/v1/statuses/bookmarks_controller_spec.rb @@ -21,36 +21,67 @@ describe Api::V1::Statuses::BookmarksController do post :create, params: { status_id: status.id } end - it 'returns http success' do - expect(response).to have_http_status(:success) - end + context 'with public status' do + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'updates the bookmarked attribute' do + expect(user.account.bookmarked?(status)).to be true + end + + it 'returns json with updated attributes' do + hash_body = body_as_json - it 'updates the bookmarked attribute' do - expect(user.account.bookmarked?(status)).to be true + expect(hash_body[:id]).to eq status.id.to_s + expect(hash_body[:bookmarked]).to be true + end end - it 'return json with updated attributes' do - hash_body = body_as_json + context 'with private status of not-followed account' do + let(:status) { Fabricate(:status, visibility: :private) } - expect(hash_body[:id]).to eq status.id.to_s - expect(hash_body[:bookmarked]).to be true + it 'returns http not found' do + expect(response).to have_http_status(404) + end end end describe 'POST #destroy' do - let(:status) { Fabricate(:status, account: user.account) } + context 'with public status' do + let(:status) { Fabricate(:status, account: user.account) } - before do - Bookmark.find_or_create_by!(account: user.account, status: status) - post :destroy, params: { status_id: status.id } - end + before do + Bookmark.find_or_create_by!(account: user.account, status: status) + post :destroy, params: { status_id: status.id } + end - it 'returns http success' do - expect(response).to have_http_status(:success) + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'updates the bookmarked attribute' do + expect(user.account.bookmarked?(status)).to be false + end + + it 'returns json with updated attributes' do + hash_body = body_as_json + + expect(hash_body[:id]).to eq status.id.to_s + expect(hash_body[:bookmarked]).to be false + end end - it 'updates the bookmarked attribute' do - expect(user.account.bookmarked?(status)).to be false + context 'with private status that was not bookmarked' do + let(:status) { Fabricate(:status, visibility: :private) } + + before do + post :destroy, params: { status_id: status.id } + end + + it 'returns http not found' do + expect(response).to have_http_status(404) + end end end end diff --git a/spec/controllers/api/v1/statuses/favourites_controller_spec.rb b/spec/controllers/api/v1/statuses/favourites_controller_spec.rb index 24a760e20..6e947f5d2 100644 --- a/spec/controllers/api/v1/statuses/favourites_controller_spec.rb +++ b/spec/controllers/api/v1/statuses/favourites_controller_spec.rb @@ -21,45 +21,77 @@ describe Api::V1::Statuses::FavouritesController do post :create, params: { status_id: status.id } end - it 'returns http success' do - expect(response).to have_http_status(200) + context 'with public status' do + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'updates the favourites count' do + expect(status.favourites.count).to eq 1 + end + + it 'updates the favourited attribute' do + expect(user.account.favourited?(status)).to be true + end + + it 'returns json with updated attributes' do + hash_body = body_as_json + + expect(hash_body[:id]).to eq status.id.to_s + expect(hash_body[:favourites_count]).to eq 1 + expect(hash_body[:favourited]).to be true + end end - it 'updates the favourites count' do - expect(status.favourites.count).to eq 1 - end - - it 'updates the favourited attribute' do - expect(user.account.favourited?(status)).to be true - end - - it 'return json with updated attributes' do - hash_body = body_as_json + context 'with private status of not-followed account' do + let(:status) { Fabricate(:status, visibility: :private) } - expect(hash_body[:id]).to eq status.id.to_s - expect(hash_body[:favourites_count]).to eq 1 - expect(hash_body[:favourited]).to be true + it 'returns http not found' do + expect(response).to have_http_status(404) + end end end describe 'POST #destroy' do - let(:status) { Fabricate(:status, account: user.account) } + context 'with public status' do + let(:status) { Fabricate(:status, account: user.account) } - before do - FavouriteService.new.call(user.account, status) - post :destroy, params: { status_id: status.id } - end + before do + FavouriteService.new.call(user.account, status) + post :destroy, params: { status_id: status.id } + end - it 'returns http success' do - expect(response).to have_http_status(200) - end + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'updates the favourites count' do + expect(status.favourites.count).to eq 0 + end + + it 'updates the favourited attribute' do + expect(user.account.favourited?(status)).to be false + end - it 'updates the favourites count' do - expect(status.favourites.count).to eq 0 + it 'returns json with updated attributes' do + hash_body = body_as_json + + expect(hash_body[:id]).to eq status.id.to_s + expect(hash_body[:favourites_count]).to eq 0 + expect(hash_body[:favourited]).to be false + end end - it 'updates the favourited attribute' do - expect(user.account.favourited?(status)).to be false + context 'with private status that was not favourited' do + let(:status) { Fabricate(:status, visibility: :private) } + + before do + post :destroy, params: { status_id: status.id } + end + + it 'returns http not found' do + expect(response).to have_http_status(404) + end end end end diff --git a/spec/controllers/api/v1/statuses/reblogs_controller_spec.rb b/spec/controllers/api/v1/statuses/reblogs_controller_spec.rb index d14ca3e8b..93b244cc3 100644 --- a/spec/controllers/api/v1/statuses/reblogs_controller_spec.rb +++ b/spec/controllers/api/v1/statuses/reblogs_controller_spec.rb @@ -21,45 +21,77 @@ describe Api::V1::Statuses::ReblogsController do post :create, params: { status_id: status.id } end - it 'returns http success' do - expect(response).to have_http_status(200) + context 'with public status' do + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'updates the reblogs count' do + expect(status.reblogs.count).to eq 1 + end + + it 'updates the reblogged attribute' do + expect(user.account.reblogged?(status)).to be true + end + + it 'returns json with updated attributes' do + hash_body = body_as_json + + expect(hash_body[:reblog][:id]).to eq status.id.to_s + expect(hash_body[:reblog][:reblogs_count]).to eq 1 + expect(hash_body[:reblog][:reblogged]).to be true + end end - it 'updates the reblogs count' do - expect(status.reblogs.count).to eq 1 - end - - it 'updates the reblogged attribute' do - expect(user.account.reblogged?(status)).to be true - end - - it 'return json with updated attributes' do - hash_body = body_as_json + context 'with private status of not-followed account' do + let(:status) { Fabricate(:status, visibility: :private) } - expect(hash_body[:reblog][:id]).to eq status.id.to_s - expect(hash_body[:reblog][:reblogs_count]).to eq 1 - expect(hash_body[:reblog][:reblogged]).to be true + it 'returns http not found' do + expect(response).to have_http_status(404) + end end end describe 'POST #destroy' do - let(:status) { Fabricate(:status, account: user.account) } + context 'with public status' do + let(:status) { Fabricate(:status, account: user.account) } - before do - ReblogService.new.call(user.account, status) - post :destroy, params: { status_id: status.id } - end + before do + ReblogService.new.call(user.account, status) + post :destroy, params: { status_id: status.id } + end - it 'returns http success' do - expect(response).to have_http_status(200) - end + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'updates the reblogs count' do + expect(status.reblogs.count).to eq 0 + end + + it 'updates the reblogged attribute' do + expect(user.account.reblogged?(status)).to be false + end - it 'updates the reblogs count' do - expect(status.reblogs.count).to eq 0 + it 'returns json with updated attributes' do + hash_body = body_as_json + + expect(hash_body[:id]).to eq status.id.to_s + expect(hash_body[:reblogs_count]).to eq 0 + expect(hash_body[:reblogged]).to be false + end end - it 'updates the reblogged attribute' do - expect(user.account.reblogged?(status)).to be false + context 'with private status that was not reblogged' do + let(:status) { Fabricate(:status, visibility: :private) } + + before do + post :destroy, params: { status_id: status.id } + end + + it 'returns http not found' do + expect(response).to have_http_status(404) + end end end end From aa8568b7f2f9043056fa7a2bb9b59ef31d63554c Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 27 Feb 2020 12:34:07 +0100 Subject: [PATCH 2/2] Bump version to 3.1.2 (#13162) --- CHANGELOG.md | 26 ++++++++++++++++++++++++++ lib/mastodon/version.rb | 2 +- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ae9fc82c..33663c2ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,32 @@ Changelog All notable changes to this project will be documented in this file. +## [v3.1.2] - 2020-02-27 +### Added + +- Add `--reset-password` option to `tootctl accounts modify` ([ThibG](https://github.com/tootsuite/mastodon/pull/13126)) +- Add source-mapped stacktrace to error message in web UI ([ThibG](https://github.com/tootsuite/mastodon/pull/13082)) + +### Fixed + +- Fix dismissing an announcement twice raising an obscure error ([ThibG](https://github.com/tootsuite/mastodon/pull/13124)) +- Fix misleading error when attempting to re-send a pending follow request ([ThibG](https://github.com/tootsuite/mastodon/pull/13133)) +- Fix backups failing when files are missing from media attachments ([ThibG](https://github.com/tootsuite/mastodon/pull/13146)) +- Fix duplicate accounts being created when fetching an account for its key only ([ThibG](https://github.com/tootsuite/mastodon/pull/13147)) +- Fix `/web` redirecting to `/web/web` in web UI ([ThibG](https://github.com/tootsuite/mastodon/pull/13128)) +- Fix previously OStatus-based accounts not being detected as ActivityPub ([ThibG](https://github.com/tootsuite/mastodon/pull/13129)) +- Fix account JSON/RSS not being cacheable due to wrong mime type comparison ([ThibG](https://github.com/tootsuite/mastodon/pull/13116)) +- Fix old browsers crashing because of missing `finally` polyfill in web UI ([ThibG](https://github.com/tootsuite/mastodon/pull/13115)) +- Fix account's bio not being shown if there are no proofs/fields in admin UI ([ThibG](https://github.com/tootsuite/mastodon/pull/13075)) +- Fix sign-ups without checked user agreement being accepted through the web form ([ThibG](https://github.com/tootsuite/mastodon/pull/13088)) +- Fix non-x64 architectures not being able to build Docker image because of hardcoded Node.js architecture ([SaraSmiseth](https://github.com/tootsuite/mastodon/pull/13081)) +- Fix invite request input not being shown on sign-up error if left empty ([ThibG](https://github.com/tootsuite/mastodon/pull/13089)) +- Fix some migration hints mentioning GitLab instead of Mastodon ([saper](https://github.com/tootsuite/mastodon/pull/13084)) + +### Security + +- Fix leak of arbitrary statuses through unfavourite action in REST API ([Gargron](https://github.com/tootsuite/mastodon/pull/13161)) + ## [3.1.1] - 2020-02-10 ### Fixed diff --git a/lib/mastodon/version.rb b/lib/mastodon/version.rb index 28eca740a..ed8bc96cd 100644 --- a/lib/mastodon/version.rb +++ b/lib/mastodon/version.rb @@ -13,7 +13,7 @@ module Mastodon end def patch - 1 + 2 end def flags