filter out localhost and local ips from canvashttp

test plan:
* use the API to queue a content migration for a course
 with a parameter 'file_url' set to a local url
 (i.e. 'localhost:3000/500.html')
* it should not download the file and allow the file
 to be downloaded on the content migrations page
 for the course

* create a link to a local file
 in a piece of course content (i.e. a page)
* run the course link validator
* it should not actually check if the link exists locally
 or not, but rather always flag the link

closes #SEC-606 SEC-607

Change-Id: I671c017ec93d88446df77d716725fff8874622bc
Reviewed-on: https://gerrit.instructure.com/125118
Tested-by: Jenkins
Reviewed-by: Brad Horrocks <bhorrocks@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Heath Hales <hhales@instructure.com>
Product-Review: James Williams  <jamesw@instructure.com>
This commit is contained in:
James Williams 2017-09-06 14:07:25 -06:00
parent fd2ed0a2bd
commit 3855fc4af1
7 changed files with 71 additions and 7 deletions

View File

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

View File

@ -17,3 +17,4 @@
CanvasHttp.open_timeout = -> { Setting.get('http_open_timeout', 5).to_f }
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(/,/) }

View File

@ -16,6 +16,8 @@
# with this program. If not, see <http://www.gnu.org/licenses/>.
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

View File

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

View File

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

View File

@ -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 => %{<a href="#{url}">kekeke</a>}, :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")

View File

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