diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index d5fb1d0dfd8..9a7165ad3bf 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -1701,7 +1701,7 @@ class Attachment < ActiveRecord::Base
# Pass an existing attachment in opts[:attachment] to use that, rather than
# creating a new attachment.
def self.clone_url_as_attachment(url, opts = {})
- _, uri = CanvasHttp.validate_url(url)
+ _, uri = CanvasHttp.validate_url(url, check_host: true)
CanvasHttp.get(url) do |http_response|
if http_response.code.to_i == 200
diff --git a/config/initializers/canvas_http.rb b/config/initializers/canvas_http.rb
index e8c29fd060d..5ee984bfc01 100644
--- a/config/initializers/canvas_http.rb
+++ b/config/initializers/canvas_http.rb
@@ -16,4 +16,5 @@
# with this program. If not, see .
CanvasHttp.open_timeout = -> { Setting.get('http_open_timeout', 5).to_f }
-CanvasHttp.read_timeout = -> { Setting.get('http_read_timeout', 30).to_f }
\ No newline at end of file
+CanvasHttp.read_timeout = -> { Setting.get('http_read_timeout', 30).to_f }
+CanvasHttp.blocked_ip_filters = -> { Setting.get('http_blocked_ip_ranges', '127.0.0.1/8').split(/,/) }
diff --git a/gems/canvas_http/lib/canvas_http.rb b/gems/canvas_http/lib/canvas_http.rb
index 5d494492966..2060f060ded 100644
--- a/gems/canvas_http/lib/canvas_http.rb
+++ b/gems/canvas_http/lib/canvas_http.rb
@@ -16,6 +16,8 @@
# with this program. If not, see .
require 'uri'
+require 'ipaddr'
+require 'resolv'
module CanvasHttp
class Error < ::StandardError; end
@@ -28,6 +30,7 @@ module CanvasHttp
end
end
class RelativeUriError < ArgumentError; end
+ class InsecureUriError < ArgumentError; end
def self.put(*args, &block)
CanvasHttp.request(Net::HTTP::Put, *args, &block)
@@ -66,7 +69,7 @@ module CanvasHttp
loop do
raise(TooManyRedirectsError) if redirect_limit <= 0
- _, uri = CanvasHttp.validate_url(url_str, host: last_host, scheme: last_scheme) # uses the last host and scheme for relative redirects
+ _, uri = CanvasHttp.validate_url(url_str, host: last_host, scheme: last_scheme, check_host: true) # uses the last host and scheme for relative redirects
http = CanvasHttp.connection_for_uri(uri)
multipart_query = nil
@@ -113,7 +116,7 @@ module CanvasHttp
end
# returns [normalized_url_string, URI] if valid, raises otherwise
- def self.validate_url(value, host: nil, scheme: nil, allowed_schemes: %w{http https})
+ def self.validate_url(value, host: nil, scheme: nil, allowed_schemes: %w{http https}, check_host: false)
value = value.strip
raise ArgumentError if value.empty?
uri = URI.parse(value)
@@ -130,10 +133,22 @@ module CanvasHttp
end
raise ArgumentError if !allowed_schemes.nil? && !allowed_schemes.include?(uri.scheme.downcase)
raise(RelativeUriError) if uri.host.nil? || uri.host.strip.empty?
+ raise InsecureUriError if check_host && self.insecure_host?(uri.host)
return value, uri
end
+ def self.insecure_host?(host)
+ return unless filters = self.blocked_ip_filters
+ addrs = Resolv.getaddresses(host).map { |ip| ::IPAddr.new(ip) rescue nil}.compact
+ return true unless addrs.any?
+
+ filters.any? do |filter|
+ addr_range = ::IPAddr.new(filter) rescue nil
+ addr_range && addrs.any?{|addr| addr_range.include?(addr)}
+ end
+ end
+
# returns a Net::HTTP connection object for the given URI object
def self.connection_for_uri(uri)
http = Net::HTTP.new(uri.host, uri.port)
@@ -151,8 +166,12 @@ module CanvasHttp
@read_timeout.respond_to?(:call) ? @read_timeout.call : @read_timeout || 30
end
+ def self.blocked_ip_filters
+ @blocked_ip_filters.respond_to?(:call) ? @blocked_ip_filters.call : @blocked_ip_filters
+ end
+
class << self
- attr_writer :open_timeout, :read_timeout
+ attr_writer :open_timeout, :read_timeout, :blocked_ip_filters
end
# returns a tempfile with a filename based on the uri (same extension, if
diff --git a/gems/canvas_http/spec/canvas_http_spec.rb b/gems/canvas_http/spec/canvas_http_spec.rb
index 9b94401a693..ab3998e81f4 100644
--- a/gems/canvas_http/spec/canvas_http_spec.rb
+++ b/gems/canvas_http/spec/canvas_http_spec.rb
@@ -41,7 +41,6 @@ describe "CanvasHttp" do
to_return(status: 200)
expect(CanvasHttp.post(url, body: body, content_type: content_type).code).to eq "200"
end
-
end
describe ".get" do
@@ -118,6 +117,25 @@ describe "CanvasHttp" do
expect(res.body).to eq("Hello")
end
+ it "should check host before running" do
+ res = nil
+ stub_request(:get, "http://www.example.com/a/b").
+ to_return(body: "Hello", headers: { 'Content-Length' => 5 })
+ expect(CanvasHttp).to receive(:insecure_host?).with("www.example.com").and_return(true)
+ expect{ CanvasHttp.get("http://www.example.com/a/b") }.to raise_error(CanvasHttp::InsecureUriError)
+ end
+ end
+
+ describe '#insecure_host?' do
+ it "should check for insecure hosts" do
+ CanvasHttp.blocked_ip_filters = -> { ['127.0.0.1/8', '42.42.42.42/16']}
+ expect(CanvasHttp.insecure_host?('www.example.com')).to eq false
+ expect(CanvasHttp.insecure_host?('localhost')).to eq true
+ expect(CanvasHttp.insecure_host?('127.0.0.1')).to eq true
+ expect(CanvasHttp.insecure_host?('42.42.42.42')).to eq true
+ expect(CanvasHttp.insecure_host?('42.42.1.1')).to eq true
+ expect(CanvasHttp.insecure_host?('42.1.1.1')).to eq false
+ end
end
describe ".tempfile_for_url" do
diff --git a/lib/canvas/migration/archive.rb b/lib/canvas/migration/archive.rb
index d6a707337e7..46014f8e45a 100644
--- a/lib/canvas/migration/archive.rb
+++ b/lib/canvas/migration/archive.rb
@@ -81,7 +81,7 @@ module Canvas::Migration
if @settings[:export_archive_path]
File.open(@settings[:export_archive_path], 'rb')
elsif @settings[:course_archive_download_url].present?
- _, uri = CanvasHttp.validate_url(@settings[:course_archive_download_url])
+ _, uri = CanvasHttp.validate_url(@settings[:course_archive_download_url], check_host: true)
CanvasHttp.get(@settings[:course_archive_download_url]) do |http_response|
raise CanvasHttp::InvalidResponseCodeError.new(http_response.code.to_i) unless http_response.code.to_i == 200
tmpfile = CanvasHttp.tempfile_for_uri(uri)
diff --git a/spec/lib/course_link_validator_spec.rb b/spec/lib/course_link_validator_spec.rb
index 6d7c43333c0..89d40ab7011 100644
--- a/spec/lib/course_link_validator_spec.rb
+++ b/spec/lib/course_link_validator_spec.rb
@@ -117,6 +117,31 @@ describe CourseLinkValidator do
expect(issues).to be_empty
end
+ describe "insecure hosts" do
+ def test_url(url)
+ course_factory
+ topic = @course.discussion_topics.create!(:message => %{kekeke}, :title => "title")
+
+ expect(CanvasHttp).to_not receive(:connection_for_uri) # don't try to continue after failing validation
+ CourseLinkValidator.queue_course(@course)
+ run_jobs
+
+ issues = CourseLinkValidator.current_progress(@course).results[:issues]
+ expect(issues.first[:invalid_links].first[:reason]).to eq :unreachable
+ end
+
+ it "should not try to access local ips" do
+ test_url("http://localhost:3000/haxxed")
+ test_url("http://127.0.0.1/haxxedagain")
+ end
+
+ it "should be able to set the ip filter" do
+ Setting.set('http_blocked_ip_ranges', '42.42.42.42/8,24.24.24.24')
+ test_url("http://42.42.0.1/haxxedtheplanet")
+ test_url("http://24.24.24.24/haxxedforever")
+ end
+ end
+
it "should check for deleted/unpublished objects" do
course_factory
active = @course.assignments.create!(:title => "blah")
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 7bc6773ed7e..c22c8669c0d 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -43,6 +43,7 @@ WebMock.enable!
module WebMock::API
include WebMock::Matchers
def self.included(other)
+ other.before { allow(CanvasHttp).to receive(:insecure_host?).and_return(false) }
other.after { WebMock.reset! }
end
end