Compare commits
	
		
			4 Commits
		
	
	
		
			features/4
			...
			v3.1.5
		
	
	| Author | SHA1 | Date | |
|---|---|---|---|
|  | 661f3f26b0 | ||
|  | 2d2e3651ee | ||
|  | 951e997b26 | ||
|  | fa3f78e4bf | 
| @@ -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 | ||||
|  | ||||
|   | ||||
| @@ -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 | ||||
|  | ||||
|   | ||||
| @@ -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 | ||||
|  | ||||
|   | ||||
| @@ -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 | ||||
|  | ||||
|   | ||||
| @@ -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 | ||||
|   | ||||
| @@ -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| | ||||
|     req.remote_ip if req.post? && req.path =~ PROTECTED_PATHS_REGEX | ||||
|   throttle('throttle_sign_up_attempts/ip', limit: 25, period: 5.minutes) do |req| | ||||
|     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| | ||||
|   | ||||
| @@ -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 | ||||
|   | ||||
| @@ -13,7 +13,7 @@ module Mastodon | ||||
|     end | ||||
|  | ||||
|     def patch | ||||
|       4 | ||||
|       5 | ||||
|     end | ||||
|  | ||||
|     def flags | ||||
|   | ||||
| @@ -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) | ||||
|   | ||||
							
								
								
									
										42
									
								
								spec/controllers/media_proxy_controller_spec.rb
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										42
									
								
								spec/controllers/media_proxy_controller_spec.rb
									
									
									
									
									
										Normal file
									
								
							| @@ -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 | ||||
		Reference in New Issue
	
	Block a user