1
0
Fork 1
forked from fedi/mastodon

Compare commits

...

4 commits

Author SHA1 Message Date
Eugen Rochko 661f3f26b0 Bump version to 3.1.5 2020-07-07 15:22:47 +02:00
Thibaut Girka 2d2e3651ee Fix media attachment enumeration
Signed-off-by: Eugen Rochko <eugen@zeonfederated.com>
2020-07-07 15:13:23 +02:00
Eugen Rochko 951e997b26 Change rate limits for various paths
- Rate limit login attempts by target account
- Rate limit password resets and e-mail re-confirmations by target account
- Rate limit sign-up/login attempts, password resets, and e-mail re-confirmations by IP like before
2020-07-07 15:13:19 +02:00
Eugen Rochko fa3f78e4bf Fix other sessions not being logged out on password change
While OAuth tokens were immediately revoked, accessing the home
controller immediately generated new OAuth tokens and "revived"
the session due to a combination of using remember_me tokens and
overwriting the `authenticate_user!` method
2020-07-07 15:13:14 +02:00
10 changed files with 96 additions and 18 deletions

View file

@ -3,6 +3,13 @@ Changelog
All notable changes to this project will be documented in this file. 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 ## [v3.1.4] - 2020-05-14
### Added ### Added

View file

@ -8,7 +8,10 @@ class Auth::PasswordsController < Devise::PasswordsController
def update def update
super do |resource| 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
end end

View file

@ -1,6 +1,8 @@
# frozen_string_literal: true # frozen_string_literal: true
class Auth::RegistrationsController < Devise::RegistrationsController class Auth::RegistrationsController < Devise::RegistrationsController
include Devise::Controllers::Rememberable
layout :determine_layout layout :determine_layout
before_action :set_invite, only: [:new, :create] before_action :set_invite, only: [:new, :create]
@ -24,7 +26,11 @@ class Auth::RegistrationsController < Devise::RegistrationsController
def update def update
super do |resource| 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
end end

View file

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

View file

@ -2,6 +2,7 @@
class MediaProxyController < ApplicationController class MediaProxyController < ApplicationController
include RoutingHelper include RoutingHelper
include Authorization
skip_before_action :store_current_location skip_before_action :store_current_location
skip_before_action :require_functional! skip_before_action :require_functional!
@ -10,12 +11,14 @@ class MediaProxyController < ApplicationController
rescue_from ActiveRecord::RecordInvalid, with: :not_found rescue_from ActiveRecord::RecordInvalid, with: :not_found
rescue_from Mastodon::UnexpectedResponseError, 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 rescue_from HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError, with: :internal_server_error
def show def show
RedisLock.acquire(lock_options) do |lock| RedisLock.acquire(lock_options) do |lock|
if lock.acquired? 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? redownload! if @media_attachment.needs_redownload? && !reject_media?
else else
raise Mastodon::RaceConditionError raise Mastodon::RaceConditionError

View file

@ -38,15 +38,6 @@ class Rack::Attack
end end
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| Rack::Attack.safelist('allow from localhost') do |req|
req.remote_ip == '127.0.0.1' || req.remote_ip == '::1' req.remote_ip == '127.0.0.1' || req.remote_ip == '::1'
end 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) req.authenticated_user_id if (req.post? && req.path =~ API_DELETE_REBLOG_REGEX) || (req.delete? && req.path =~ API_DELETE_STATUS_REGEX)
end 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 end
self.throttled_response = lambda do |env| self.throttled_response = lambda do |env|

View file

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

View file

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

View file

@ -28,9 +28,8 @@ describe MediaController do
end end
it 'raises when not permitted to view' do it 'raises when not permitted to view' do
status = Fabricate(:status) status = Fabricate(:status, visibility: :direct)
media_attachment = Fabricate(:media_attachment, status: status) 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 } get :show, params: { id: media_attachment.to_param }
expect(response).to have_http_status(404) expect(response).to have_http_status(404)

View 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