Merge tag 'v3.1.5' into hometown-dev

CHANGELOG.md

@@ -3,6 +3,13 @@ Changelog
 
 All notable changes to this project will be documented in this file.
 
+## [v3.1.5] - 2020-07-07
+### Security
+
+- Fix media attachment enumeration ([ThibG](https://github.com/tootsuite/mastodon/pull/14254))
+- Change rate limits for various paths ([Gargron](https://github.com/tootsuite/mastodon/pull/14253))
+- Fix other sessions not being logged out on password change ([Gargron](https://github.com/tootsuite/mastodon/pull/14252))
+
 ## [v3.1.4] - 2020-05-14
 ### Added
 

app/controllers/auth/passwords_controller.rb

@@ -8,7 +8,10 @@ class Auth::PasswordsController < Devise::PasswordsController
 
   def update
     super do |resource|
-      resource.session_activations.destroy_all if resource.errors.empty?
+      if resource.errors.empty?
+        resource.session_activations.destroy_all
+        resource.forget_me!
+      end
     end
   end
 

app/controllers/auth/registrations_controller.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class Auth::RegistrationsController < Devise::RegistrationsController
+  include Devise::Controllers::Rememberable
+
   layout :determine_layout
 
   before_action :set_invite, only: [:new, :create]
@@ -24,7 +26,11 @@ class Auth::RegistrationsController < Devise::RegistrationsController
 
   def update
     super do |resource|
-      resource.clear_other_sessions(current_session.session_id) if resource.saved_change_to_encrypted_password?
+      if resource.saved_change_to_encrypted_password?
+        resource.clear_other_sessions(current_session.session_id)
+        resource.forget_me!
+        remember_me(resource)
+      end
     end
   end
 

app/controllers/home_controller.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 class HomeController < ApplicationController
+  before_action :redirect_unauthenticated_to_permalinks!
   before_action :authenticate_user!
   before_action :set_referrer_policy_header
 
@@ -10,7 +11,7 @@ class HomeController < ApplicationController
 
   private
 
-  def authenticate_user!
+  def redirect_unauthenticated_to_permalinks!
     return if user_signed_in?
 
     matches = request.path.match(/\A\/web\/(statuses|accounts)\/([\d]+)\z/)
@@ -35,6 +36,7 @@ class HomeController < ApplicationController
     end
 
     matches = request.path.match(%r{\A/web/timelines/tag/(?<tag>.+)\z})
+
     redirect_to(matches ? tag_path(CGI.unescape(matches[:tag])) : default_redirect_path)
   end
 

app/controllers/media_proxy_controller.rb

@@ -2,6 +2,7 @@
 
 class MediaProxyController < ApplicationController
   include RoutingHelper
+  include Authorization
 
   skip_before_action :store_current_location
   skip_before_action :require_functional!
@@ -10,12 +11,14 @@ class MediaProxyController < ApplicationController
 
   rescue_from ActiveRecord::RecordInvalid, with: :not_found
   rescue_from Mastodon::UnexpectedResponseError, with: :not_found
+  rescue_from Mastodon::NotPermittedError, with: :not_found
   rescue_from HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError, with: :internal_server_error
 
   def show
     RedisLock.acquire(lock_options) do |lock|
       if lock.acquired?
-        @media_attachment = MediaAttachment.remote.find(params[:id])
+        @media_attachment = MediaAttachment.remote.attached.find(params[:id])
+        authorize @media_attachment.status, :show?
         redownload! if @media_attachment.needs_redownload? && !reject_media?
       else
         raise Mastodon::RaceConditionError

config/initializers/rack_attack.rb

@@ -38,15 +38,6 @@ class Rack::Attack
     end
   end
 
-  PROTECTED_PATHS = %w(
-    /auth/sign_in
-    /auth
-    /auth/password
-    /auth/confirmation
-  ).freeze
-
-  PROTECTED_PATHS_REGEX = Regexp.union(PROTECTED_PATHS.map { |path| /\A#{Regexp.escape(path)}/ })
-
   Rack::Attack.safelist('allow from localhost') do |req|
     req.remote_ip == '127.0.0.1' || req.remote_ip == '::1'
   end
@@ -86,8 +77,32 @@ class Rack::Attack
     req.authenticated_user_id if (req.post? && req.path =~ API_DELETE_REBLOG_REGEX) || (req.delete? && req.path =~ API_DELETE_STATUS_REGEX)
   end
 
-  throttle('protected_paths', limit: 25, period: 5.minutes) do |req|
+  throttle('throttle_sign_up_attempts/ip', limit: 25, period: 5.minutes) do |req|
-    req.remote_ip if req.post? && req.path =~ PROTECTED_PATHS_REGEX
+    req.remote_ip if req.post? && req.path == '/auth'
+  end
+
+  throttle('throttle_password_resets/ip', limit: 25, period: 5.minutes) do |req|
+    req.remote_ip if req.post? && req.path == '/auth/password'
+  end
+
+  throttle('throttle_password_resets/email', limit: 5, period: 30.minutes) do |req|
+    req.params.dig('user', 'email').presence if req.post? && req.path == '/auth/password'
+  end
+
+  throttle('throttle_email_confirmations/ip', limit: 25, period: 5.minutes) do |req|
+    req.remote_ip if req.post? && req.path == '/auth/confirmation'
+  end
+
+  throttle('throttle_email_confirmations/email', limit: 5, period: 30.minutes) do |req|
+    req.params.dig('user', 'email').presence if req.post? && req.path == '/auth/password'
+  end
+
+  throttle('throttle_login_attempts/ip', limit: 25, period: 5.minutes) do |req|
+    req.remote_ip if req.post? && req.path == '/auth/sign_in'
+  end
+
+  throttle('throttle_login_attempts/email', limit: 25, period: 1.hour) do |req|
+    req.session[:attempt_user_id] || req.params.dig('user', 'email').presence if req.post? && req.path == '/auth/sign_in'
   end
 
   self.throttled_response = lambda do |env|

config/initializers/rack_attack_logging.rb

@@ -2,5 +2,6 @@ ActiveSupport::Notifications.subscribe(/rack_attack/) do |_name, _start, _finish
   req = payload[:request]
 
   next unless [:throttle, :blacklist].include? req.env['rack.attack.match_type']
+
   Rails.logger.info("Rate limit hit (#{req.env['rack.attack.match_type']}): #{req.ip} #{req.request_method} #{req.fullpath}")
 end

lib/mastodon/version.rb

@@ -13,7 +13,7 @@ module Mastodon
     end
 
     def patch
-      4
+      5
     end
 
     def flags

spec/controllers/media_controller_spec.rb

@@ -28,9 +28,8 @@ describe MediaController do
     end
 
     it 'raises when not permitted to view' do
-      status = Fabricate(:status)
+      status = Fabricate(:status, visibility: :direct)
       media_attachment = Fabricate(:media_attachment, status: status)
-      allow_any_instance_of(MediaController).to receive(:authorize).and_raise(ActiveRecord::RecordNotFound)
       get :show, params: { id: media_attachment.to_param }
 
       expect(response).to have_http_status(404)

spec/controllers/media_proxy_controller_spec.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe MediaProxyController do
+  render_views
+
+  before do
+    stub_request(:get, 'http://example.com/attachment.png').to_return(request_fixture('avatar.txt'))
+  end
+
+  describe '#show' do
+    it 'redirects when attached to a status' do
+      status = Fabricate(:status)
+      media_attachment = Fabricate(:media_attachment, status: status, remote_url: 'http://example.com/attachment.png')
+      get :show, params: { id: media_attachment.id }
+
+      expect(response).to have_http_status(302)
+    end
+
+    it 'responds with missing when there is not an attached status' do
+      media_attachment = Fabricate(:media_attachment, status: nil, remote_url: 'http://example.com/attachment.png')
+      get :show, params: { id: media_attachment.id }
+
+      expect(response).to have_http_status(404)
+    end
+
+    it 'raises when id cant be found' do
+      get :show, params: { id: 'missing' }
+
+      expect(response).to have_http_status(404)
+    end
+
+    it 'raises when not permitted to view' do
+      status = Fabricate(:status, visibility: :direct)
+      media_attachment = Fabricate(:media_attachment, status: status, remote_url: 'http://example.com/attachment.png')
+      get :show, params: { id: media_attachment.id }
+
+      expect(response).to have_http_status(404)
+    end
+  end
+end