From 2e8a492e8843aa958c53636b24cf4d344e7ca47d Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sun, 25 Feb 2018 03:16:11 +0900 Subject: [PATCH] Raise Mastodon::HostValidationError when host for HTTP request is private (#6410) --- Gemfile | 4 ++++ Gemfile.lock | 2 ++ app/lib/exceptions.rb | 1 + app/lib/request.rb | 19 +++++++++++++++++- app/lib/sidekiq_error_handler.rb | 11 +++++++++++ config/environments/development.rb | 6 ++++++ config/initializers/sidekiq.rb | 4 ++++ spec/lib/request_spec.rb | 31 ++++++++++++++++++++++-------- 8 files changed, 69 insertions(+), 9 deletions(-) create mode 100644 app/lib/sidekiq_error_handler.rb diff --git a/Gemfile b/Gemfile index da5fc2f38..e2b3b1971 100644 --- a/Gemfile +++ b/Gemfile @@ -96,6 +96,10 @@ group :development, :test do gem 'rspec-rails', '~> 3.7' end +group :production, :test do + gem 'private_address_check', '~> 0.4.1' +end + group :test do gem 'capybara', '~> 2.15' gem 'climate_control', '~> 0.2' diff --git a/Gemfile.lock b/Gemfile.lock index 65a0dfabf..8293977d8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -376,6 +376,7 @@ GEM premailer-rails (1.10.1) actionmailer (>= 3, < 6) premailer (~> 1.7, >= 1.7.9) + private_address_check (0.4.1) pry (0.11.3) coderay (~> 1.1.0) method_source (~> 0.9.0) @@ -683,6 +684,7 @@ DEPENDENCIES pghero (~> 1.7) pkg-config (~> 1.2) premailer-rails + private_address_check (~> 0.4.1) pry-rails (~> 0.3) puma (~> 3.10) pundit (~> 1.1) diff --git a/app/lib/exceptions.rb b/app/lib/exceptions.rb index b2489711d..95e3365c2 100644 --- a/app/lib/exceptions.rb +++ b/app/lib/exceptions.rb @@ -4,6 +4,7 @@ module Mastodon class Error < StandardError; end class NotPermittedError < Error; end class ValidationError < Error; end + class HostValidationError < ValidationError; end class RaceConditionError < Error; end class UnexpectedResponseError < Error diff --git a/app/lib/request.rb b/app/lib/request.rb index 7671f4ffc..5776b3d78 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +require 'ipaddr' +require 'socket' + class Request REQUEST_TARGET = '(request-target)' @@ -8,7 +11,7 @@ class Request def initialize(verb, url, **options) @verb = verb @url = Addressable::URI.parse(url).normalize - @options = options + @options = options.merge(socket_class: Socket) @headers = {} set_common_headers! @@ -87,4 +90,18 @@ class Request def http_client HTTP.timeout(:per_operation, timeout).follow(max_hops: 2) end + + class Socket < TCPSocket + class << self + def open(host, *args) + address = IPSocket.getaddress(host) + raise Mastodon::HostValidationError if PrivateAddressCheck.private_address? IPAddr.new(address) + super address, *args + end + + alias new open + end + end + + private_constant :Socket end diff --git a/app/lib/sidekiq_error_handler.rb b/app/lib/sidekiq_error_handler.rb new file mode 100644 index 000000000..23785cf05 --- /dev/null +++ b/app/lib/sidekiq_error_handler.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class SidekiqErrorHandler + def call(*) + yield + rescue Mastodon::HostValidationError => e + Rails.logger.error "#{e.class}: #{e.message}" + Rails.logger.error e.backtrace.join("\n") + # Do not retry + end +end diff --git a/config/environments/development.rb b/config/environments/development.rb index 59bc2c3e2..2da407c32 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -85,3 +85,9 @@ Rails.application.configure do end ActiveRecordQueryTrace.enabled = ENV.fetch('QUERY_TRACE_ENABLED') { false } + +module PrivateAddressCheck + def self.private_address?(*) + false + end +end diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index b70784d79..f875fbd95 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -9,6 +9,10 @@ end Sidekiq.configure_server do |config| config.redis = redis_params + + config.server_middleware do |chain| + chain.add SidekiqErrorHandler + end end Sidekiq.configure_client do |config| diff --git a/spec/lib/request_spec.rb b/spec/lib/request_spec.rb index 782f14b18..dc7daa52c 100644 --- a/spec/lib/request_spec.rb +++ b/spec/lib/request_spec.rb @@ -38,17 +38,32 @@ describe Request do end describe '#perform' do - before do - stub_request(:get, 'http://example.com') - subject.perform + context 'with valid host' do + before do + stub_request(:get, 'http://example.com') + subject.perform + end + + it 'executes a HTTP request' do + expect(a_request(:get, 'http://example.com')).to have_been_made.once + end + + it 'sets headers' do + expect(a_request(:get, 'http://example.com').with(headers: subject.headers)).to have_been_made + end end - it 'executes a HTTP request' do - expect(a_request(:get, 'http://example.com')).to have_been_made.once - end + context 'with private host' do + around do |example| + WebMock.disable! + example.run + WebMock.enable! + end - it 'sets headers' do - expect(a_request(:get, 'http://example.com').with(headers: subject.headers)).to have_been_made + it 'raises Mastodon::ValidationError' do + allow(IPSocket).to receive(:getaddress).with('example.com').and_return('0.0.0.0') + expect{ subject.perform }.to raise_error Mastodon::ValidationError + end end end end