Merge branch 'security-github-ssrf-redirect' into 'master'

Do not allow localhost url redirection in GitHub Integration

See merge request gitlab/gitlabhq!3188

changelogs/unreleased/security-github-ssrf-redirect.yml

+---
+title: Do not allow localhost url redirection in GitHub Integration
+merge_request:
+author:
+type: security

config/initializers/octokit.rb

+Octokit.middleware.insert_after Octokit::Middleware::FollowRedirects, Gitlab::Octokit::Middleware

lib/gitlab/github_import/client.rb

@@ -40,7 +40,7 @@ module Gitlab
       #            otherwise hitting the rate limit will result in a thread
       #            being blocked in a `sleep()` call for up to an hour.
       def initialize(token, per_page: 100, parallel: true)
-        @octokit = Octokit::Client.new(
+        @octokit = ::Octokit::Client.new(
           access_token: token,
           per_page: per_page,
           api_endpoint: api_endpoint
@@ -139,7 +139,7 @@ module Gitlab
 
         begin
           yield
-        rescue Octokit::TooManyRequests
+        rescue ::Octokit::TooManyRequests
           raise_or_wait_for_rate_limit
 
           # This retry will only happen when running in sequential mode as we'll

lib/gitlab/legacy_github_import/client.rb

@@ -101,7 +101,7 @@ module Gitlab
       # GitHub Rate Limit API returns 404 when the rate limit is
       # disabled. In this case we just want to return gracefully
       # instead of spitting out an error.
-      rescue Octokit::NotFound
+      rescue ::Octokit::NotFound
         nil
       end
 

lib/gitlab/octokit/middleware.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Octokit
+    class Middleware
+      def initialize(app)
+        @app = app
+      end
+
+      def call(env)
+        Gitlab::UrlBlocker.validate!(env[:url], { allow_localhost: allow_local_requests?, allow_local_network: allow_local_requests? })
+
+        @app.call(env)
+      end
+
+      private
+
+      def allow_local_requests?
+        Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
+      end
+    end
+  end
+end

spec/lib/gitlab/octokit/middleware_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Octokit::Middleware do
+  let(:app) { double(:app) }
+  let(:middleware) { described_class.new(app) }
+
+  shared_examples 'Public URL' do
+    it 'does not raise an error' do
+      expect(app).to receive(:call).with(env)
+
+      expect { middleware.call(env) }.not_to raise_error
+    end
+  end
+
+  shared_examples 'Local URL' do
+    it 'raises an error' do
+      expect { middleware.call(env) }.to raise_error(Gitlab::UrlBlocker::BlockedUrlError)
+    end
+  end
+
+  describe '#call' do
+    context 'when the URL is a public URL' do
+      let(:env) { { url: 'https://public-url.com' } }
+
+      it_behaves_like 'Public URL'
+    end
+
+    context 'when the URL is a localhost adresss' do
+      let(:env) { { url: 'http://127.0.0.1' } }
+
+      context 'when localhost requests are not allowed' do
+        before do
+          stub_application_setting(allow_local_requests_from_hooks_and_services: false)
+        end
+
+        it_behaves_like 'Local URL'
+      end
+
+      context 'when localhost requests are allowed' do
+        before do
+          stub_application_setting(allow_local_requests_from_hooks_and_services: true)
+        end
+
+        it_behaves_like 'Public URL'
+      end
+    end
+
+    context 'when the URL is a local network address' do
+      let(:env) { { url: 'http://172.16.0.0' } }
+
+      context 'when local network requests are not allowed' do
+        before do
+          stub_application_setting(allow_local_requests_from_hooks_and_services: false)
+        end
+
+        it_behaves_like 'Local URL'
+      end
+
+      context 'when local network requests are allowed' do
+        before do
+          stub_application_setting(allow_local_requests_from_hooks_and_services: true)
+        end
+
+        it_behaves_like 'Public URL'
+      end
+    end
+  end
+end