From 39e84d174d84e4bb909b152858c40ad737ec8299 Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 17 Sep 2018 20:24:46 +0200 Subject: [PATCH 01/11] Unconditionally re-encode locally-uploaded images to strip metadata (#8714) This strips metadata on file upload by re-encoding the files, at the cost of possible slight image quality decrease and processing resources. --- lib/paperclip/lazy_thumbnail.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/paperclip/lazy_thumbnail.rb b/lib/paperclip/lazy_thumbnail.rb index ea675a5bf..542c17fb2 100644 --- a/lib/paperclip/lazy_thumbnail.rb +++ b/lib/paperclip/lazy_thumbnail.rb @@ -20,7 +20,7 @@ module Paperclip private def needs_convert? - needs_different_geometry? || needs_different_format? + needs_different_geometry? || needs_different_format? || needs_metadata_stripping? end def needs_different_geometry? @@ -31,5 +31,9 @@ module Paperclip def needs_different_format? @format.present? && @current_format != @format end + + def needs_metadata_stripping? + @attachment.instance.respond_to?(:local?) && @attachment.instance.local? + end end end From 10f7278e9ac99fd43060b30b0e091b3f565f1b91 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 28 Sep 2018 17:02:53 +0200 Subject: [PATCH 02/11] Fix class autoloading issue in ActivityPub::Activity::Create (#8820) --- app/lib/activitypub/activity/create.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index f40e1fa3e..978289788 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -92,7 +92,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity return if tag['href'].blank? account = account_from_uri(tag['href']) - account = FetchRemoteAccountService.new.call(tag['href'], id: false) if account.nil? + account = ::FetchRemoteAccountService.new.call(tag['href'], id: false) if account.nil? return if account.nil? account.mentions.create(status: status) end From 38a48a627c290072d6c6c1e99952cecb59d39f51 Mon Sep 17 00:00:00 2001 From: Yamagishi Kazutoshi Date: Sun, 30 Sep 2018 07:05:59 +0900 Subject: [PATCH 03/11] Fix that Rails.cache information could not be sent via StatsD (#8831) --- config/initializers/statsd.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/statsd.rb b/config/initializers/statsd.rb index 45702ac94..ce83fd9de 100644 --- a/config/initializers/statsd.rb +++ b/config/initializers/statsd.rb @@ -9,7 +9,7 @@ if ENV['STATSD_ADDR'].present? ::NSA.inform_statsd(statsd) do |informant| informant.collect(:action_controller, :web) informant.collect(:active_record, :db) - informant.collect(:cache, :cache) + informant.collect(:active_support_cache, :cache) informant.collect(:sidekiq, :sidekiq) end end From c2f31d908ec8764ab3a91e3c6edb8f8d8309d503 Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 3 Oct 2018 23:44:13 +0200 Subject: [PATCH 04/11] Fix handling of ActivityPub activities lacking some attributes (#8864) --- app/lib/activitypub/activity/accept.rb | 2 +- app/lib/activitypub/activity/delete.rb | 2 ++ app/lib/activitypub/activity/reject.rb | 2 +- app/lib/activitypub/activity/undo.rb | 2 ++ 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/lib/activitypub/activity/accept.rb b/app/lib/activitypub/activity/accept.rb index 7e60b2c00..348ee0d1c 100644 --- a/app/lib/activitypub/activity/accept.rb +++ b/app/lib/activitypub/activity/accept.rb @@ -26,7 +26,7 @@ class ActivityPub::Activity::Accept < ActivityPub::Activity end def relay - @relay ||= Relay.find_by(follow_activity_id: object_uri) + @relay ||= Relay.find_by(follow_activity_id: object_uri) unless object_uri.nil? end def relay_follow? diff --git a/app/lib/activitypub/activity/delete.rb b/app/lib/activitypub/activity/delete.rb index 3474d55d9..457047ac0 100644 --- a/app/lib/activitypub/activity/delete.rb +++ b/app/lib/activitypub/activity/delete.rb @@ -17,6 +17,8 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity end def delete_note + return if object_uri.nil? + @status = Status.find_by(uri: object_uri, account: @account) @status ||= Status.find_by(uri: @object['atomUri'], account: @account) if @object.is_a?(Hash) && @object['atomUri'].present? diff --git a/app/lib/activitypub/activity/reject.rb b/app/lib/activitypub/activity/reject.rb index d81b157de..dba21fb9a 100644 --- a/app/lib/activitypub/activity/reject.rb +++ b/app/lib/activitypub/activity/reject.rb @@ -28,7 +28,7 @@ class ActivityPub::Activity::Reject < ActivityPub::Activity end def relay - @relay ||= Relay.find_by(follow_activity_id: object_uri) + @relay ||= Relay.find_by(follow_activity_id: object_uri) unless object_uri.nil? end def relay_follow? diff --git a/app/lib/activitypub/activity/undo.rb b/app/lib/activitypub/activity/undo.rb index 64c2be7d9..599823c6e 100644 --- a/app/lib/activitypub/activity/undo.rb +++ b/app/lib/activitypub/activity/undo.rb @@ -19,6 +19,8 @@ class ActivityPub::Activity::Undo < ActivityPub::Activity private def undo_announce + return if object_uri.nil? + status = Status.find_by(uri: object_uri, account: @account) status ||= Status.find_by(uri: @object['atomUri'], account: @account) if @object.is_a?(Hash) && @object['atomUri'].present? From 0d844c078044dafcfea14df9700bd6d338b6ce6a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" Date: Fri, 5 Oct 2018 04:07:44 +0200 Subject: [PATCH 05/11] [Security] Bump nokogiri from 1.8.4 to 1.8.5 (#8881) Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.8.4 to 1.8.5. **This update includes security fixes.** - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/master/CHANGELOG.md) - [Commits](https://github.com/sparklemotion/nokogiri/compare/v1.8.4...v1.8.5) Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 502d08efe..ad5ade8ee 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -347,7 +347,7 @@ GEM net-ssh (>= 2.6.5) net-ssh (4.2.0) nio4r (2.3.1) - nokogiri (1.8.4) + nokogiri (1.8.5) mini_portile2 (~> 2.3.0) nokogumbo (1.5.0) nokogiri From a1b904441ebed76a77daaad8bdd217f0d6dda047 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" Date: Fri, 5 Oct 2018 18:58:13 +0200 Subject: [PATCH 06/11] Bump puma from 3.11.4 to 3.12.0 (#8883) Bumps [puma](https://github.com/puma/puma) from 3.11.4 to 3.12.0. - [Release notes](https://github.com/puma/puma/releases) - [Changelog](https://github.com/puma/puma/blob/master/History.md) - [Commits](https://github.com/puma/puma/compare/v3.11.4...v3.12.0) Signed-off-by: dependabot[bot] --- Gemfile | 2 +- Gemfile.lock | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index bdac00b48..fc61a268f 100644 --- a/Gemfile +++ b/Gemfile @@ -5,7 +5,7 @@ ruby '>= 2.3.0', '< 2.6.0' gem 'pkg-config', '~> 1.3' -gem 'puma', '~> 3.11' +gem 'puma', '~> 3.12' gem 'rails', '~> 5.2.1' gem 'thor', '~> 0.20' diff --git a/Gemfile.lock b/Gemfile.lock index ad5ade8ee..bbbcc3423 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -412,7 +412,7 @@ GEM pry-rails (0.3.6) pry (>= 0.10.4) public_suffix (3.0.2) - puma (3.11.4) + puma (3.12.0) pundit (1.1.0) activesupport (>= 3.0.0) rack (2.0.5) @@ -721,7 +721,7 @@ DEPENDENCIES private_address_check (~> 0.4.1) pry-byebug (~> 3.6) pry-rails (~> 0.3) - puma (~> 3.11) + puma (~> 3.12) pundit (~> 1.1) rack-attack (~> 5.2) rack-cors (~> 1.0) @@ -765,4 +765,4 @@ RUBY VERSION ruby 2.5.0p0 BUNDLED WITH - 1.16.3 + 1.16.5 From 485dc7d5594b21add2ee0825c5940d053f8989ca Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 7 Oct 2018 04:40:37 +0200 Subject: [PATCH 07/11] Add fallback for PostgreSQL without upsert in CopyStatusStats (#8903) Fix #8590 --- .../20180812173710_copy_status_stats.rb | 50 +++++++++++++++---- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/db/migrate/20180812173710_copy_status_stats.rb b/db/migrate/20180812173710_copy_status_stats.rb index 850aa9c13..ff10c18d9 100644 --- a/db/migrate/20180812173710_copy_status_stats.rb +++ b/db/migrate/20180812173710_copy_status_stats.rb @@ -3,15 +3,10 @@ class CopyStatusStats < ActiveRecord::Migration[5.2] def up safety_assured do - Status.unscoped.select('id').find_in_batches(batch_size: 5_000) do |statuses| - execute <<-SQL.squish - INSERT INTO status_stats (status_id, reblogs_count, favourites_count, created_at, updated_at) - SELECT id, reblogs_count, favourites_count, created_at, updated_at - FROM statuses - WHERE id IN (#{statuses.map(&:id).join(', ')}) - ON CONFLICT (status_id) DO UPDATE - SET reblogs_count = EXCLUDED.reblogs_count, favourites_count = EXCLUDED.favourites_count - SQL + if supports_upsert? + up_fast + else + up_slow end end end @@ -19,4 +14,41 @@ class CopyStatusStats < ActiveRecord::Migration[5.2] def down # Nothing end + + private + + def supports_upsert? + version = select_one("SELECT current_setting('server_version_num') AS v")['v'].to_i + version >= 90500 + end + + def up_fast + say 'Upsert is available, importing counters using the fast method' + + Status.unscoped.select('id').find_in_batches(batch_size: 5_000) do |statuses| + execute <<-SQL.squish + INSERT INTO status_stats (status_id, reblogs_count, favourites_count, created_at, updated_at) + SELECT id, reblogs_count, favourites_count, created_at, updated_at + FROM statuses + WHERE id IN (#{statuses.map(&:id).join(', ')}) + ON CONFLICT (status_id) DO UPDATE + SET reblogs_count = EXCLUDED.reblogs_count, favourites_count = EXCLUDED.favourites_count + SQL + end + end + + def up_slow + say 'Upsert is not available in PostgreSQL below 9.5, falling back to slow import of counters' + + # We cannot use bulk INSERT or overarching transactions here because of possible + # uniqueness violations that we need to skip over + Status.unscoped.select('id, reblogs_count, favourites_count, created_at, updated_at').find_each do |status| + begin + params = [[nil, status.id], [nil, status.reblogs_count], [nil, status.favourites_count], [nil, status.created_at], [nil, status.updated_at]] + exec_insert('INSERT INTO status_stats (status_id, reblogs_count, favourites_count, created_at, updated_at) VALUES ($1, $2, $3, $4, $5)', nil, params) + rescue ActiveRecord::RecordNotUnique + next + end + end + end end From 6984396b1157df3754c5d2c85bcd02d0a7003df5 Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 7 Oct 2018 19:45:40 +0200 Subject: [PATCH 08/11] Ensure only toots from the reported users are reported (#8916) --- app/controllers/api/v1/reports_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/v1/reports_controller.rb b/app/controllers/api/v1/reports_controller.rb index a954101cb..2aac5e61e 100644 --- a/app/controllers/api/v1/reports_controller.rb +++ b/app/controllers/api/v1/reports_controller.rb @@ -27,7 +27,7 @@ class Api::V1::ReportsController < Api::BaseController private def reported_status_ids - Status.find(status_ids).pluck(:id) + reported_account.statuses.find(status_ids).pluck(:id) end def status_ids From 65662b384742ba733dd590fd7eccb5abf8c6d0c8 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 7 Oct 2018 20:13:54 +0200 Subject: [PATCH 09/11] Bump version to 2.5.1 --- CHANGELOG.md | 10 ++++++++++ lib/mastodon/version.rb | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 000000000..220cb6591 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,10 @@ +## 2.5.1 + +- Fix some local images not having their EXIF metadata stripped on upload (#8714) +- Fix class autoloading issue in ActivityPub Create handler (#8820) +- Fix cache statistics not being sent via statsd when statsd enabled (#8831) +- Fix being able to enable a disabled relay via ActivityPub Accept handler (#8864) +- Bump nokogiri from 1.8.4 to 1.8.5 (#8881) +- Bump puma from 3.11.4 to 3.12.0 (#8883) +- Fix database migrations for PostgreSQL below 9.5 (#8903) +- Fix being able to report statuses not belonging to the reported account (#8916) diff --git a/lib/mastodon/version.rb b/lib/mastodon/version.rb index 0e41a9a6d..e6d3af35d 100644 --- a/lib/mastodon/version.rb +++ b/lib/mastodon/version.rb @@ -13,7 +13,7 @@ module Mastodon end def patch - 0 + 1 end def pre From 1787704e1c283410481b44cd0128dac8b52e087d Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 12 Oct 2018 00:15:55 +0200 Subject: [PATCH 10/11] Improve signature verification safeguards (#8959) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Downcase signed_headers string before building the signed string The HTTP Signatures draft does not mandate the “headers” field to be downcased, but mandates the header field names to be downcased in the signed string, which means that prior to this patch, Mastodon could fail to process signatures from some compliant clients. It also means that it would not actually check the Digest of non-compliant clients that wouldn't use a lowercased Digest field name. Thankfully, I don't know of any such client. * Revert "Remove dead code (#8919)" This reverts commit a00ce8c92c06f42109aad5cfe65d46862cf037bb. * Restore time window checking, change it to 12 hours By checking the Date header, we can prevent replaying old vulnerable signatures. The focus is to prevent replaying old vulnerable requests from software that has been fixed in the meantime, so a somewhat long window should be fine and accounts for timezone misconfiguration. * Escape users' URLs when formatting them Fixes possible HTML injection * Escape all string interpolations in Formatter class Slightly improve performance by reducing class allocations from repeated Formatter#encode calls * Fix code style issues --- .../concerns/signature_verification.rb | 12 +++++++--- app/lib/formatter.rb | 16 ++++++++----- .../concerns/signature_verification_spec.rb | 24 +++++++++++++++++++ 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb index 4d77fa432..e5d5e2ca6 100644 --- a/app/controllers/concerns/signature_verification.rb +++ b/app/controllers/concerns/signature_verification.rb @@ -22,6 +22,12 @@ module SignatureVerification return end + if request.headers['Date'].present? && !matches_time_window? + @signature_verification_failure_reason = 'Signed request date outside acceptable time window' + @signed_request_account = nil + return + end + raw_signature = request.headers['Signature'] signature_params = {} @@ -76,7 +82,7 @@ module SignatureVerification def build_signed_string(signed_headers) signed_headers = 'date' if signed_headers.blank? - signed_headers.split(' ').map do |signed_header| + signed_headers.downcase.split(' ').map do |signed_header| if signed_header == Request::REQUEST_TARGET "#{Request::REQUEST_TARGET}: #{request.method.downcase} #{request.path}" elsif signed_header == 'digest' @@ -89,12 +95,12 @@ module SignatureVerification def matches_time_window? begin - time_sent = DateTime.httpdate(request.headers['Date']) + time_sent = Time.httpdate(request.headers['Date']) rescue ArgumentError return false end - (Time.now.utc - time_sent).abs <= 30 + (Time.now.utc - time_sent).abs <= 12.hours end def body_digest diff --git a/app/lib/formatter.rb b/app/lib/formatter.rb index 8b694536c..35d5a09b7 100644 --- a/app/lib/formatter.rb +++ b/app/lib/formatter.rb @@ -90,8 +90,12 @@ class Formatter private + def html_entities + @html_entities ||= HTMLEntities.new + end + def encode(html) - HTMLEntities.new.encode(html) + html_entities.encode(html) end def encode_and_link_urls(html, accounts = nil, options = {}) @@ -143,7 +147,7 @@ class Formatter emoji = emoji_map[shortcode] if emoji - replacement = "\":#{shortcode}:\"" + replacement = "\":#{encode(shortcode)}:\"" before_html = shortname_start_index.positive? ? html[0..shortname_start_index - 1] : '' html = before_html + replacement + html[i + 1..-1] i += replacement.size - (shortcode.size + 2) - 1 @@ -212,7 +216,7 @@ class Formatter return link_to_account(acct) unless linkable_accounts account = linkable_accounts.find { |item| TagManager.instance.same_acct?(item.acct, acct) } - account ? mention_html(account) : "@#{acct}" + account ? mention_html(account) : "@#{encode(acct)}" end def link_to_account(acct) @@ -221,7 +225,7 @@ class Formatter domain = nil if TagManager.instance.local_domain?(domain) account = EntityCache.instance.mention(username, domain) - account ? mention_html(account) : "@#{acct}" + account ? mention_html(account) : "@#{encode(acct)}" end def link_to_hashtag(entity) @@ -239,10 +243,10 @@ class Formatter end def hashtag_html(tag) - "##{tag}" + "##{encode(tag)}" end def mention_html(account) - "@#{account.username}" + "@#{encode(account.username)}" end end diff --git a/spec/controllers/concerns/signature_verification_spec.rb b/spec/controllers/concerns/signature_verification_spec.rb index 3daf1fc4e..720690097 100644 --- a/spec/controllers/concerns/signature_verification_spec.rb +++ b/spec/controllers/concerns/signature_verification_spec.rb @@ -73,6 +73,30 @@ describe ApplicationController, type: :controller do end end + context 'with request older than a day' do + before do + get :success + + fake_request = Request.new(:get, request.url) + fake_request.add_headers({ 'Date' => 2.days.ago.utc.httpdate }) + fake_request.on_behalf_of(author) + + request.headers.merge!(fake_request.headers) + end + + describe '#signed_request?' do + it 'returns true' do + expect(controller.signed_request?).to be true + end + end + + describe '#signed_request_account' do + it 'returns nil' do + expect(controller.signed_request_account).to be_nil + end + end + end + context 'with body' do before do post :success, body: 'Hello world' From e8a4ba49cfda3b7461fe349266a6ea479e667bf5 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 12 Oct 2018 00:22:38 +0200 Subject: [PATCH 11/11] Bump version to 2.5.2 --- CHANGELOG.md | 4 ++++ lib/mastodon/version.rb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 220cb6591..75a51fc7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.5.2 + +- Fix XSS vulnerability (#8959) + ## 2.5.1 - Fix some local images not having their EXIF metadata stripped on upload (#8714) diff --git a/lib/mastodon/version.rb b/lib/mastodon/version.rb index e6d3af35d..a49e7f102 100644 --- a/lib/mastodon/version.rb +++ b/lib/mastodon/version.rb @@ -13,7 +13,7 @@ module Mastodon end def patch - 1 + 2 end def pre