diff --git a/.codeclimate.yml b/.codeclimate.yml index b74b36e..7027358 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -9,6 +9,9 @@ engines: enabled: true rubocop: enabled: true + exclude_fingerprints: + # Using #=== intentionally to do subnet masking + - d1afe90be49c43e85a76bfa58f637804 ratings: paths: - "**.rb" diff --git a/lib/cc/fixed_resolv.rb b/lib/cc/fixed_resolv.rb new file mode 100644 index 0000000..7ce21bc --- /dev/null +++ b/lib/cc/fixed_resolv.rb @@ -0,0 +1,29 @@ +require "resolv-replace" + +module CC + class FixedResolv < Resolv::DNS + def self.enable! + new.tap do |instance| + Resolv::DefaultResolver.replace_resolvers([instance]) + end + end + + def initialize + @addresses = {} + end + + def setaddress(name, address) + addresses[name] = address + end + + def each_address(name) + if addresses.key?(name) + yield addresses.fetch(name) + end + end + + private + + attr_reader :addresses + end +end diff --git a/lib/cc/service/http.rb b/lib/cc/service/http.rb index 4f7f27f..f0d163c 100644 --- a/lib/cc/service/http.rb +++ b/lib/cc/service/http.rb @@ -1,5 +1,6 @@ require "active_support/concern" require "cc/service/response_check" +require "cc/service/safe_webhook" module CC::Service::HTTP extend ActiveSupport::Concern @@ -38,10 +39,8 @@ def service_post_with_redirects(url, body = nil, headers = nil, &block) end def raw_get(url = nil, params = nil, headers = nil) - http.get do |req| - req.url(url) if url + http_method(:get, url, nil, headers) do |req| req.params.update(params) if params - req.headers.update(headers) if headers yield req if block_given? end end @@ -54,6 +53,8 @@ def raw_post(url = nil, body = nil, headers = nil) def http_method(method, url = nil, body = nil, headers = nil) block = Proc.new if block_given? + CC::Service::SafeWebhook.ensure_safe!(url) + http.send(method) do |req| req.url(url) if url req.headers.update(headers) if headers diff --git a/lib/cc/service/safe_webhook.rb b/lib/cc/service/safe_webhook.rb new file mode 100644 index 0000000..de0490b --- /dev/null +++ b/lib/cc/service/safe_webhook.rb @@ -0,0 +1,69 @@ +require "ipaddr" +require "uri" + +require "cc/fixed_resolv" + +module CC + class Service + class SafeWebhook + InternalWebhookError = Class.new(StandardError) + + PRIVATE_ADDRESS_SUBNETS = [ + IPAddr.new("10.0.0.0/8"), + IPAddr.new("172.16.0.0/12"), + IPAddr.new("192.168.0.0/16"), + IPAddr.new("fd00::/8"), + IPAddr.new("127.0.0.1"), + IPAddr.new("0:0:0:0:0:0:0:1"), + ].freeze + + def self.ensure_safe!(url) + instance = new(url) + instance.ensure_safe! + end + + def self.getaddress(host) + @dns ||= Resolv::DNS.new + @dns.getaddress(host) + end + + def self.setaddress(host, address) + @fixed_resolv ||= CC::FixedResolv.enable! + @fixed_resolv.setaddress(host, address) + end + + def initialize(url) + @url = url + end + + def ensure_safe! + uri = URI.parse(url) + + if !allow_internal_webhooks? && internal?(uri.host) + raise InternalWebhookError, "#{url.inspect} maps to an internal address" + end + end + + private + + attr_reader :url + + def internal?(host) + address = self.class.getaddress(host) + + self.class.setaddress(host, address) + + PRIVATE_ADDRESS_SUBNETS.any? do |subnet| + subnet === IPAddr.new(address.to_s) + end + rescue Resolv::ResolvError + true # localhost + end + + def allow_internal_webhooks? + var = ENV["CODECLIMATE_ALLOW_INTERNAL_WEBHOOKS"] || "" + var == "1" || var == "true" + end + end + end +end diff --git a/spec/cc/service/safe_webhook_spec.rb b/spec/cc/service/safe_webhook_spec.rb new file mode 100644 index 0000000..b7094da --- /dev/null +++ b/spec/cc/service/safe_webhook_spec.rb @@ -0,0 +1,41 @@ +require "spec_helper" + +class CC::Service + describe SafeWebhook do + describe ".ensure_safe!" do + it "does not allow internal URLs" do + %w[ 127.0.0.1 192.168.0.1 10.0.1.18 ].each do |address| + stub_resolv("github.com", address) + + expect do + SafeWebhook.ensure_safe!("https://github.com/api/v1/user") + end.to raise_error(SafeWebhook::InternalWebhookError) + end + end + + it "allows internal URLs when configured to do so" do + allow(ENV).to receive(:[]). + with("CODECLIMATE_ALLOW_INTERNAL_WEBHOOKS"). + and_return("1") + + stub_resolv("github.com", "10.0.1.18") + + SafeWebhook.ensure_safe!("https://github.com/api/v1/user") + end + + it "allows non-internal URLs" do + stub_resolv("github.com", "1.1.1.2") + + SafeWebhook.ensure_safe!("https://github.com/api/v1/user") + end + + it "ensures future dns queries get the same answer" do + stub_resolv("github.com", "1.1.1.3") + + SafeWebhook.ensure_safe!("https://github.com/api/v1/user") + + expect(Resolv.getaddress("github.com").to_s).to eq "1.1.1.3" + end + end + end +end diff --git a/spec/cc/service_spec.rb b/spec/cc/service_spec.rb index 6cffebb..50fa135 100644 --- a/spec/cc/service_spec.rb +++ b/spec/cc/service_spec.rb @@ -32,42 +32,42 @@ it "post success" do stub_http("/my/test/url", [200, {}, '{"ok": true, "thing": "123"}']) - response = service_post("/my/test/url", { token: "1234" }.to_json, {}) do |inner_response| + response = service_post("http://example.com/my/test/url", { token: "1234" }.to_json, {}) do |inner_response| body = JSON.parse(inner_response.body) { thing: body["thing"] } end expect(response[:ok]).to eq(true) expect(response[:params]).to eq('{"token":"1234"}') - expect(response[:endpoint_url]).to eq("/my/test/url") + expect(response[:endpoint_url]).to eq("http://example.com/my/test/url") expect(response[:status]).to eq(200) end it "post redirect success" do - stub_http("/my/test/url", [307, { "Location" => "/my/redirect/url" }, '{"ok": false, "redirect": true}']) + stub_http("/my/test/url", [307, { "Location" => "http://example.com/my/redirect/url" }, '{"ok": false, "redirect": true}']) stub_http("/my/redirect/url", [200, {}, '{"ok": true, "thing": "123"}']) - response = service_post_with_redirects("/my/test/url", { token: "1234" }.to_json, {}) do |inner_response| + response = service_post_with_redirects("http://example.com/my/test/url", { token: "1234" }.to_json, {}) do |inner_response| body = JSON.parse(inner_response.body) { thing: body["thing"] } end expect(response[:ok]).to eq(true) expect(response[:params]).to eq('{"token":"1234"}') - expect(response[:endpoint_url]).to eq("/my/test/url") + expect(response[:endpoint_url]).to eq("http://example.com/my/test/url") expect(response[:status]).to eq(200) end it "post http failure" do stub_http("/my/wrong/url", [404, {}, ""]) - expect { service_post("/my/wrong/url", { token: "1234" }.to_json, {}) }.to raise_error(CC::Service::HTTPError) + expect { service_post("http://example.com/my/wrong/url", { token: "1234" }.to_json, {}) }.to raise_error(CC::Service::HTTPError) end it "post some other failure" do stub_http("/my/wrong/url") { raise ArgumentError, "lol" } - expect { service_post("/my/wrong/url", { token: "1234" }.to_json, {}) }.to raise_error(ArgumentError) + expect { service_post("http://example.com/my/wrong/url", { token: "1234" }.to_json, {}) }.to raise_error(ArgumentError) end it "services" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 14e39a6..773cb4e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -36,4 +36,8 @@ # This setting enables warnings. It's recommended, but in some cases may # be too noisy due to issues in dependencies. config.warnings = true + config.before do + # Disable actual DNS resolution during specs by default + stub_resolv(anything, "1.1.1.1") + end end diff --git a/spec/support/resolv_helpers.rb b/spec/support/resolv_helpers.rb new file mode 100644 index 0000000..b035ab3 --- /dev/null +++ b/spec/support/resolv_helpers.rb @@ -0,0 +1,10 @@ +module ResolvHelpers + def stub_resolv(name, address) + allow(CC::Service::SafeWebhook).to receive(:getaddress). + with(name).and_return(Resolv::IPv4.create(address)) + end +end + +RSpec.configure do |conf| + conf.include(ResolvHelpers) +end