Merge branch 'security-fogbugz-importer-dns-rebind-ssrf' into 'master'

Fix fogbugz importer DNS Rebind SSRF

See merge request gitlab-org/security/gitlab!1668

lib/gitlab/fogbugz_import.rb

+# frozen_string_literal: true
+
+require 'fogbugz'
+
+module Gitlab
+  module FogbugzImport
+    # Custom adapter to validate the URL before each request
+    # This way we avoid DNS rebinds or other unsafe requests
+    ::Fogbugz.adapter[:http] = HttpAdapter
+  end
+end

lib/gitlab/fogbugz_import/client.rb

 # frozen_string_literal: true
 
-require 'fogbugz'
-
 module Gitlab
   module FogbugzImport
     class Client

lib/gitlab/fogbugz_import/http_adapter.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module FogbugzImport
+    class HttpAdapter
+      def initialize(options = {})
+        @root_url = options[:uri]
+      end
+
+      def request(action, options = {})
+        uri = Gitlab::Utils.append_path(@root_url, 'api.asp')
+
+        params = { 'cmd' => action }.merge(options.fetch(:params, {}))
+
+        response = Gitlab::HTTP.post(uri, body: params)
+
+        response.body
+      end
+    end
+  end
+end

spec/lib/gitlab/fogbugz_import/importer_spec.rb

@@ -4,23 +4,11 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::FogbugzImport::Importer do
   let(:project) { create(:project_empty_repo) }
-  let(:importer) { described_class.new(project) }
-  let(:repo) do
-    instance_double(Gitlab::FogbugzImport::Repository,
-      safe_name: 'vim',
-      path: 'vim',
-      raw_data: '')
-  end
-
-  let(:import_data) { { 'repo' => repo } }
-  let(:credentials) do
-    {
-      'fb_session' => {
-        'uri' => 'https://testing.fogbugz.com',
-        'token' => 'token'
-      }
-    }
-  end
+  let(:fogbugz_project) { { 'ixProject' => project.id, 'sProject' => 'vim' } }
+  let(:import_data) { { 'repo' => fogbugz_project } }
+  let(:base_url) { 'https://testing.fogbugz.com' }
+  let(:token) { 'token' }
+  let(:credentials) { { 'fb_session' => { 'uri' => base_url, 'token' => token } } }
 
   let(:closed_bug) do
     {
@@ -46,18 +34,22 @@ RSpec.describe Gitlab::FogbugzImport::Importer do
 
   let(:fogbugz_bugs) { [opened_bug, closed_bug] }
 
+  subject(:importer) { described_class.new(project) }
+
   before do
     project.create_import_data(data: import_data, credentials: credentials)
-    allow_any_instance_of(::Fogbugz::Interface).to receive(:command).with(:listCategories).and_return([])
-    allow_any_instance_of(Gitlab::FogbugzImport::Client).to receive(:cases).and_return(fogbugz_bugs)
+
+    stub_fogbugz('listProjects', projects: { project: [fogbugz_project], count: 1 })
+    stub_fogbugz('listCategories', categories: { category: [], count: 0 })
+    stub_fogbugz('search', cases: { case: fogbugz_bugs, count: fogbugz_bugs.size })
   end
 
   it 'imports bugs' do
-    expect { importer.execute }.to change { Issue.count }.by(2)
+    expect { subject.execute }.to change { Issue.count }.by(2)
   end
 
   it 'imports opened bugs' do
-    importer.execute
+    subject.execute
 
     issue = Issue.where(project_id: project.id).find_by_title(opened_bug[:sTitle])
 
@@ -65,10 +57,54 @@ RSpec.describe Gitlab::FogbugzImport::Importer do
   end
 
   it 'imports closed bugs' do
-    importer.execute
+    subject.execute
 
     issue = Issue.where(project_id: project.id).find_by_title(closed_bug[:sTitle])
 
     expect(issue.state_id).to eq(Issue.available_states[:closed])
   end
+
+  context 'verify url' do
+    context 'when host is localhost' do
+      let(:base_url) { 'https://localhost:3000' }
+
+      it 'does not allow localhost requests' do
+        expect { subject.execute }
+          .to raise_error(
+            ::Gitlab::HTTP::BlockedUrlError,
+            "URL 'https://localhost:3000/api.asp' is blocked: Requests to localhost are not allowed"
+          )
+      end
+    end
+
+    context 'when host is on local network' do
+      let(:base_url) { 'http://192.168.0.1' }
+
+      it 'does not allow localhost requests' do
+        expect { subject.execute }
+          .to raise_error(
+            ::Gitlab::HTTP::BlockedUrlError,
+            "URL 'http://192.168.0.1/api.asp' is blocked: Requests to the local network are not allowed"
+          )
+      end
+    end
+
+    context 'when host is ftp protocol' do
+      let(:base_url) { 'ftp://testing' }
+
+      it 'only accept http and https requests' do
+        expect { subject.execute }
+          .to raise_error(
+            HTTParty::UnsupportedURIScheme,
+            "'ftp://testing/api.asp' Must be HTTP, HTTPS or Generic"
+          )
+      end
+    end
+  end
+
+  def stub_fogbugz(command, response)
+    stub_request(:post, "#{base_url}/api.asp")
+      .with(body: hash_including({ 'cmd' => command, 'token' => token }))
+      .to_return(status: 200, body: response.to_xml(root: :response))
+  end
 end