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.
## [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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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|

View file

@ -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

View file

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

View file

@ -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)

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